Use subject.favorite to get favourited status #3573

Merged
merged 2 commits into from Mar 10, 2017

Conversation

Projects
None yet
4 participants
@eatyourgreens
Member

eatyourgreens commented Mar 9, 2017

Fixes #3494 .

Describe your changes.
Uses the subject.favorite flag to set favourited status.
Removes API call to check for subject in collection.

Review Checklist

Optional

  • If it's a new component, is it in ES6? Is it clear of warnings from ESLint?
  • Have you replaced any ChangeListener or PromiseRenderer components with code that updates component state?
  • If changes are made to the classifier, does the dev classifier still work?
  • Have you added in flow type annotations?
Use subject.favorite to get favourited status
Remove API call to check for subject in collection.
@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Mar 9, 2017

@eatyourgreens, thanks for your PR! By analyzing the history of the files in this pull request, we identified @camallen and @amyrebecca to be potential reviewers.

@eatyourgreens, thanks for your PR! By analyzing the history of the files in this pull request, we identified @camallen and @amyrebecca to be potential reviewers.

@eatyourgreens eatyourgreens added the SGL label Mar 9, 2017

@eatyourgreens

This comment has been minimized.

Show comment
Hide comment
@eatyourgreens

eatyourgreens Mar 9, 2017

Member

Seems to work in the classifier interface but I'm not sure that it works on Talk, or in collections.

Member

eatyourgreens commented Mar 9, 2017

Seems to work in the classifier interface but I'm not sure that it works on Talk, or in collections.

@camallen

This comment has been minimized.

Show comment
Hide comment
@camallen

camallen Mar 9, 2017

Member

@eatyourgreens should we just drop the check and allow users to press the button no matter what. response should be ok / already in collections error that would signify to make the heart solid. Anything else we could report an error on?

Member

camallen commented Mar 9, 2017

@eatyourgreens should we just drop the check and allow users to press the button no matter what. response should be ok / already in collections error that would signify to make the heart solid. Anything else we could report an error on?

@eatyourgreens

This comment has been minimized.

Show comment
Hide comment
@eatyourgreens

eatyourgreens Mar 9, 2017

Member

@camallen that works for the classifier but on Talk, and in collections, the heart should be solid already if they've previously favourited the subject.

Member

eatyourgreens commented Mar 9, 2017

@camallen that works for the classifier but on Talk, and in collections, the heart should be solid already if they've previously favourited the subject.

@srallen

srallen approved these changes Mar 9, 2017

LGTM for the classifier. I'm not sure what the best approach is for collections, but it's something that can be thought about next week since some of us in Chicago will be doing collections work for Gravity Spy.

@eatyourgreens eatyourgreens merged commit 7a93e71 into master Mar 10, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@eatyourgreens eatyourgreens deleted the subject-favorite-flag branch Mar 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment