Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

/taste toggles taste count when hit with the same device_guid param. #14

Open
benlachman opened this issue Jun 25, 2015 · 10 comments
Open
Assignees

Comments

@benlachman
Copy link
Member

It should simple ignore any duplicates vs. toggling. Same goes for favoriting.

@pagan8
Copy link
Contributor

pagan8 commented Jun 25, 2015

Are you sure? I think Ricky deliberately put this behavior in so that when someone hit the like or tastes button again it would undo their request.... I guess this isn't needed though if the iOS app doesn't track state of the button (eg. liked / not liked).... I can change it but Ricky should chime in first :)

@benlachman
Copy link
Member Author

My feeling is that that logic should be in the client and only validation logic (eg no tastes by the same person twice) be in the backend. Thoughts @rickychilcott ?

@rickychilcott
Copy link
Member

So... what happens if they want to "undo" their favorite or like? Do we still ignore or provide another mechanism? That's why we implemented the toggle

@pagan8
Copy link
Contributor

pagan8 commented Jun 25, 2015

I agree with Ricky, and the current implementation prevents someone from liking or tasting something twice. By keeping the logic on the backend, you also eliminate the problem of someone clearing the app's cache / data or uninstalling / reinstalling the app to like something multiple times ;)

@rickychilcott
Copy link
Member

👍 and it's done, so we don't have to spend more time on it :)

@benlachman would it be helpful if we passed back the status for a given device_guid (i.e. status would equal 0 for not tasted and 1 for tasted).

The problem with the current implementation is that it's possible for the app state to get out of sync with the toggl mechanism

@benlachman
Copy link
Member Author

Well it's done the opposite way on the app side. So it's a change either way. My feeling that if we meant to build a mechanism for syncing state to the backend we're a bit behind. The client's state isn't reflected in the backend, it functions more as a data source currently. Based on my memory of the original discussion we just wanted to avoid duplicates in the taste count metadata.

@buswellj
Copy link
Contributor

Change the app then.. Never going to be dups the way the back end is setup
:)

On Thu, Jun 25, 2015, 18:42 Ben Lachman [email protected] wrote:

Well it's done the opposite way on the app side. So it's a change either
way. My feeling that if we meant to build a mechanism for syncing state to
the backend we're a bit behind. The client's state isn't reflected in the
backend, it functions more as a data source currently. Based on my memory
of the original discussion we just wanted to avoid duplicates in the taste
count metadata.


Reply to this email directly or view it on GitHub
#14 (comment)
.

@benlachman
Copy link
Member Author

Not going to happen. There's no reason to muddy the fact that we're not and have never planned to store app state on the service side in this version of the app. Plus, I'm the only code contributor on the app side, there are too many more important issues to make happen.

@pagan8
Copy link
Contributor

pagan8 commented Jun 27, 2015

So can we close this as working as designed? :)

@benlachman
Copy link
Member Author

Ha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants