Skip to content

Conversation

@chynh
Copy link
Contributor

@chynh chynh commented Oct 30, 2020

Description

Modify resource retrieval logic so that the payload includes the current vote direction on a resource for logged in users. This would allow the front-end to display the current vote direction for a user.

I noticed that this piece of information is missing while working on a front-end issue so I jumped on it. Feel free to provide suggestions/comments.

Copy link
Member

@aaron-junot aaron-junot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it's starting to look really good. For one thing, we need to include the user_vote_direction when we get a single resource as well as multiple resources. I know that the front end doesn't currently use this functionality, but if it ever does, I don't want to have to come and fix it. Why don't we add user_votes as part of the serialization in the Resources model, and if there is no auth key given, we just serialize to None?

This would also return the user_vote_direction when you create or update a resource, and when you retrieve a resource without having to have that logic in multiple files.

I'd also like to see some unit tests that verify user_vote_direction is null when there's no auth key, that it's "None" where there is an auth key but no vote direction, and that it's "upvotes" when it's been upvoted, and "downvotes" when it's been downvoted.

And since this is a change to the API itself, we'll need to add it to the documentation (OpenAPI file).

All that being said, I like the idea and I like where you're going with it. Let's just get it in the right place and get tests and docs for it, and it should be good to merge if there are no bugs that I can find. Thanks for this!

@chynh
Copy link
Contributor Author

chynh commented Nov 4, 2020

Not sure if you want to reset db since this involves schema change in vote_information.

@aaron-junot
Copy link
Member

I notice that the /click endpoint isn't returning the vote direction correctly. I haven't dug any deeper, and I know that endpoint is not really in use on the front end, but I'd like to see that working properly before we merge because it could cause other bugs for other endpoints depending on what the root cause of it is

@chynh
Copy link
Contributor Author

chynh commented Nov 6, 2020

I notice that the /click endpoint isn't returning the vote direction correctly. I haven't dug any deeper, and I know that endpoint is not really in use on the front end, but I'd like to see that working properly before we merge because it could cause other bugs for other endpoints depending on what the root cause of it is

If it is the issue where vote direction is always returning None, that is expected as that is not an autheticated route. If you want the vote direction when logged in, I think we can have the decorator on that route similar to what we have on retrieval routes.

@aaron-junot
Copy link
Member

Ah, yes I see now. Can we go ahead and add that decorator to all routes that have a resource in the response so the API response is the same across the board?

@aaron-junot aaron-junot merged commit 202b27d into OperationCode:main Nov 7, 2020
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

Successfully merging this pull request may close these issues.

2 participants