-
Notifications
You must be signed in to change notification settings - Fork 1
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
Mae/groupsvoteroute #40
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job on this route so far! I left a few comments on some improvements we could make.
api/optimeet/groups/urls.py
Outdated
path("<int:group_id>/recs", views.get_recommendation) | ||
path("<int:group_id>/recs", views.get_recommendation), | ||
path("<int:group_id>/votes", views.votes, name='votes'), | ||
path("<int:group_id>/votescreate", views.create_vote, name='create_vote'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of making a new route for getting or creating, we want to leverage HTTP verbs like GET and POST. We can reuse the same /votes
route and have GET requests go to one function and POST requests go to another.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job so far! There's a few more things I'd like to address
api/optimeet/groups/models.py
Outdated
class Votes(models.Model): | ||
group_id = models.ForeignKey(Group, related_name='group_votes', on_delete=models.CASCADE) | ||
user_id = models.CharField(max_length=64, blank=True) | ||
results_of_voted = models.CharField(max_length=64, blank=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would results_of_voted
be used? Since this is a single vote that a particular user is making in a group we wouldn't keep track of multiple votes. Also remember what we're voting for: recommendations, so we should add the recommendation id this vote is for as a foreign key.
Fixed votes view issue fixes issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
No description provided.