-
Notifications
You must be signed in to change notification settings - Fork 0
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
Get reviews for user #20
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 work! just some minor suggestions
|
||
var review models.Review | ||
|
||
var mediaType, comment, userID, mediaID, rating *string |
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.
i dont think u need these temp vars. u can bind directly to fields of review struct that u define on line 28
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.
see this from the docs
…platnm into get_reviews_for_user
…itory field TODO: User model needs to be updated to reflect 'user' table to enable checking if a user_id is present in 'user' but not in 'reviews.' Current User model reflects 'users' table. Code is commented out in ...\users\review.go from lines 27-38 that should account for this case.
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.
really great work!
few small suggestions, also the create review PR was just merged so you'll have to pull that in and might have conflicts as a result - hopefully it's not too bad, also happy to help with that during our meeting today
@@ -0,0 +1,44 @@ | |||
package users |
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.
make sure this lives in package reviews
and a corresponding reviews folder!
also, following a more modular handler structure where each handler method gets its own file is a good way for us to avoid having massive files - see this PR's create_review file as an example
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.
Fixed!
db *pgxpool.Pool | ||
} | ||
|
||
func (r *ReviewRepository) UserExists(ctx context.Context, id string) (bool, error) { |
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.
we should prob move this function to the UserRepository in user.go
, since it's not dealing with review-specific functionality and since your handler is now holding onto both a ReviewRepository and a UserRepository
func (r *ReviewRepository) UserExists(ctx context.Context, id string) (bool, error) { | |
func (r *UserRepository) UserExists(ctx context.Context, id string) (bool, error) { |
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.
This is fixed!
backend/internal/storage/storage.go
Outdated
|
||
type ReviewRepository interface { | ||
GetReviewsByUserID(ctx context.Context, id string) ([]*models.Review, error) | ||
UserExists(ctx context.Context, id string) (bool, error) |
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.
move to UserRepository
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.
This is also fixed
|
||
id := c.Params("id") | ||
|
||
exists, err := h.reviewRepository.UserExists(c.Context(), id) |
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.
see prev comments about moving UserExists to UserRepository, and then you can update it here too so that you're using the additional repository in this handler
exists, err := h.reviewRepository.UserExists(c.Context(), id) | |
exists, err := h.userRepository.UserExists(c.Context(), id) |
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.
Fixed
rows, err := r.db.Query(ctx, "SELECT * FROM review WHERE user_id = $1", id) | ||
|
||
if !rows.Next() { | ||
return []*models.Review{}, errs.NotFound("Review", "valid id", id) |
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.
appreciate the thorough error checking! here though, I think it's actually okay and not an error if there's a valid userId with no reviews
return []*models.Review{}, errs.NotFound("Review", "valid id", id) | |
return []*models.Review{}, nil |
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.
Fixed
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.
🤩
commit 722541a Author: Dessy Dusichka <[email protected]> Date: Mon Sep 23 21:09:01 2024 -0400 Adding docs on making schema changes, updating contributing docs (#26) * docs updates * remove unnecessary info commit 403be01 Author: Luisa <[email protected]> Date: Mon Sep 23 19:23:39 2024 -0400 Get reviews for user (#20) Me when reviews are retrieved by user ids commit ec0ab7a Merge: f33718a 22b4ed7 Author: gaikwadsid <[email protected]> Date: Mon Sep 23 17:18:14 2024 -0400 Merge pull request #24 from GenerateNU/rest_of_schema Rest of schema commit f33718a Author: aahil nishad <[email protected]> Date: Mon Sep 23 16:21:22 2024 -0400 Create new review endpoint & review types. (#27) * feat: add create review endpoint * refactor: fix error handling for create review * refactor: make handlers more modular * fix: update query to handle edge cases * fix: check userid is valid when creating review * fix: make pr changes * revert update to og script --------- Co-authored-by: aaronkimbrooktec <[email protected]> Co-authored-by: ddusichka <[email protected]> commit 7ae380f Author: Dessy Dusichka <[email protected]> Date: Mon Sep 23 00:10:47 2024 -0400 Add nix scripts to lint and run frontend (#28) * add scripts for frontend * allow console for now and remove test action * make prettier happy * one more try commit 22b4ed7 Author: gaikwadsid <[email protected]> Date: Sun Sep 22 18:57:24 2024 -0400 Example for new script commit e99e4ea Author: gaikwadsid <[email protected]> Date: Sun Sep 22 00:09:46 2024 -0400 Connected Frontend to Backend (#23) * Connected Frontend to Backend * Fixed lint? * Fix lint * Fix lint * Fix lint * Fix lint * Fix lint * Fix lint? * Fix lint * Fix lint * prettier fix? * lint fix * fix lint. * Deleted .env file --------- Co-authored-by: ddusichka <[email protected]> commit e783ffe Author: ddusichka <[email protected]> Date: Sun Sep 22 00:04:59 2024 -0400 pause frontend tests commit 551b292 Author: gaikwadsid <[email protected]> Date: Sat Sep 21 21:49:17 2024 -0400 Fixed syntax errrors commit df13718 Author: Dessy Dusichka <[email protected]> Date: Sat Sep 21 14:48:00 2024 -0400 Update pull_request_template.md commit 4f7e72c Merge: 9816b83 70dd930 Author: Dessy Dusichka <[email protected]> Date: Sat Sep 21 11:17:01 2024 -0400 Merge pull request #22 from GenerateNU/env-help-bump Move Task scripts to Nix scripts commit cefc909 Author: gaikwadsid <[email protected]> Date: Fri Sep 20 22:17:35 2024 -0400 Added data into the tables commit 9816b83 Merge: 09b17a6 5cfb658 Author: Dessy Dusichka <[email protected]> Date: Fri Sep 20 13:27:13 2024 -0400 Merge pull request #18 from GenerateNU/spotify-oauth-v2 feat: spotify oauth commit 70dd930 Author: Jackson Terrill <[email protected]> Date: Fri Sep 20 00:49:03 2024 -0400 Move Task scripts to Nix scripts commit b663684 Author: gaikwadsid <[email protected]> Date: Thu Sep 19 18:38:06 2024 -0400 Added rest of schema commit 5cfb658 Merge: 540e431 09b17a6 Author: aaronkimbrooktec <[email protected]> Date: Thu Sep 19 17:41:00 2024 -0400 Merge remote-tracking branch 'origin' into spotify-oauth-v2 commit 540e431 Author: aaronkimbrooktec <[email protected]> Date: Thu Sep 19 17:40:53 2024 -0400 refactor: make oauth sessionvalue store available to all providers commit 09b17a6 Author: gaikwadsid <[email protected]> Date: Thu Sep 19 17:22:18 2024 -0400 Update pull_request_template.md commit f98e84a Author: gaikwadsid <[email protected]> Date: Thu Sep 19 17:20:53 2024 -0400 Update pull_request_template.md commit d8d6996 Author: aaronkimbrooktec <[email protected]> Date: Thu Sep 19 16:15:53 2024 -0400 refactor: rename and reorganize some things commit 1fe560a Author: garrettladley <[email protected]> Date: Thu Sep 19 11:47:29 2024 -0400 feat: back to sessions commit 5752469 Author: ddusichka <[email protected]> Date: Thu Sep 19 10:48:35 2024 -0400 make favicon error go away commit 1264a6b Author: garrettladley <[email protected]> Date: Thu Sep 19 09:50:06 2024 -0400 feat: don't keep track of challenge on our end, clean up, and TODO resolved commit 01f9218 Merge: eab21a4 ead556c Author: Garrett Ladley <[email protected]> Date: Wed Sep 18 23:45:49 2024 -0400 Merge branch 'main' into spotify-oauth-v2 commit ead556c Author: Aaron Kim <[email protected]> Date: Wed Sep 18 23:45:17 2024 -0400 feat: add central error handling (#19) commit eab21a4 Author: garrettladley <[email protected]> Date: Wed Sep 18 23:17:37 2024 -0400 nit: make stateValue fields unexported commit d9c9ddd Author: garrettladley <[email protected]> Date: Wed Sep 18 23:16:02 2024 -0400 fix: lint? commit 9f2b184 Author: garrettladley <[email protected]> Date: Wed Sep 18 23:15:12 2024 -0400 fix: lint? commit 079f0be Author: garrettladley <[email protected]> Date: Wed Sep 18 23:11:40 2024 -0400 fix: lint? commit 71ccfab Author: garrettladley <[email protected]> Date: Wed Sep 18 23:10:14 2024 -0400 fix: lint? commit 11ce351 Author: garrettladley <[email protected]> Date: Wed Sep 18 23:05:16 2024 -0400 fix: check err value of set commit f93510b Author: garrettladley <[email protected]> Date: Wed Sep 18 23:00:54 2024 -0400 feat: kv with state as the key and verifier x challenge as the value commit 72ef134 Author: aaronkimbrooktec <[email protected]> Date: Wed Sep 18 17:33:52 2024 -0400 feat: spotify oauth commit 0a567be Author: Dessy Dusichka <[email protected]> Date: Wed Sep 18 20:52:41 2024 -0400 Create pull_request_template.md commit 648ad3f Merge: 37c4fad 7c69270 Author: gaikwadsid <[email protected]> Date: Tue Sep 17 22:30:00 2024 -0400 Merge pull request #16 from GenerateNU/add-schema Setting up database tables and seed data commit 7c69270 Author: ddusichka <[email protected]> Date: Tue Sep 17 22:15:37 2024 -0400 add unique constraint commit 13c9527 Author: ddusichka <[email protected]> Date: Tue Sep 17 15:00:01 2024 -0400 update ID types and seed data commit c1feea9 Author: ddusichka <[email protected]> Date: Mon Sep 16 22:55:43 2024 -0400 setting up schema and seed data
Description
Added the endpoint to get reviews for a given user ID.
Also added the review type
Issue
Link to issue
How Has This Been Tested?
See Postman calls in the screenshots section below.
Screenshots:
User exists and there are reviews for that user:
User exists but there are no reviews for that user:
User does not exist: