Cast vote for idea - Githubissues

OpenUnited / platform

Other

41 stars 66 forks source link

Cast vote for idea #258

Closed sachintom999 closed 4 months ago

sachintom999 commented 4 months ago

Added capability for a logged in user to vote / unvote for an idea
Without logging in, the vote button is not visible
on page refresh, the latest vote count is fetched from the DB, Also, the vote icon indicates the logged in user had already voted or not

Changes

added a new column voted_users in Idea model to store list of voted users. Removed vote_count
added a new view cast_vote_for_idea and corresponding url
on click of Vote, it checks whether the logged in user is voted or not , then add/remove the user to voted_users and update the new count in the frontend

Testing

Case : logged in user

13may

Case: user without authentication

@adrianmcphee , I will raise a separate PR for unit tests for the above functionality

adrianmcphee commented 4 months ago

@endalkh should we perhaps merge this as is, or are your suggestions important enough that they should be addressed?

sachintom999 commented 4 months ago

@endalkh should we perhaps merge this as is, or are your suggestions important enough that they should be addressed?

@endalkh - Thanks for the review comments.

@adrianmcphee, As per our discussion, though adding the field voted_users is simpler, on further reading, your idea of adding a new model (called IdeaVote ) is more robust and prevents data corruption in certain cases. I am implementing that approach. All the comments above will be addressed when I push the latest changes. I will do that in some time.

sachintom999 commented 4 months ago

@adrianmcphee @endalkh , the pipeline is failing because of conflicting migrations

I tried pulling the latest main branch and merging into this feature branch and for me , it says No conflicts detected to merge.

Could you please try at your end and help me here. Thanks

endalkh commented 4 months ago

@sachintom999 can you please pull the last changes and run migration merge

'python manage-py nakemigrations --merge'

sachintom999 commented 4 months ago

@endalkh I already did that , but it says No conflicts detected to merge

sachintom999 commented 4 months ago

@endalkh , sorry. My fork was not upto date. Hence the above issues. Fixed all issues now.