Kalina 199 3 - #297
Conversation
Finished all testing. Changed one of the getTagsByUser to getTagsByUserID because there were two methods named the same thing and sending an id through this method would go to the one that used emails instead of the one that used IDs.
ethanszeto
left a comment
ethanszeto
left a comment
There was a problem hiding this comment.
Not bad! Some refactoring could be done, but the important part is that you wrote code well-within the patterns we have, which is amazing.
| * @param {Request} req | ||
| * @param {Response} res | ||
| */ | ||
| static async filter(req, res) { |
There was a problem hiding this comment.
(n-b) filter is pretty generic for a method name, maybe introduce some more specificity so its easy to understand for other developers what this function does
| // get tags by color and email | ||
| if (color && userEmail) { | ||
| // get the tags by color | ||
| const photoTagByColor = await PhotoTagAccessor.getTagsByColor(color); | ||
| // get the user's id based on their email | ||
| const decodedEmail = decodeURIComponent(userEmail); | ||
| const userId = await Utils.getUserIdByEmail(decodedEmail) | ||
| // get the phototags by the id | ||
| const photoTagsById = await PhotoTagAccessor.getTagsByUserID(userId); | ||
| // filter the photo tags to find the tags by color and email | ||
| const photoTagsByColorAndEmail = photoTagByColor.filter(photoTag => | ||
| photoTagsById.some(tag => tag._id.toString() === photoTag._id.toString()) | ||
| ); | ||
| photoTags = photoTagsByColorAndEmail; | ||
| } | ||
| // else get tags by color | ||
| else if (color) { | ||
| photoTags = await PhotoTagAccessor.getTagsByColor(color); | ||
| } | ||
| // else get tags by userEmail | ||
| else if (userEmail) { | ||
| const decodedEmail = decodeURIComponent(userEmail); | ||
| const userId = await Utils.getUserIdByEmail(decodedEmail); | ||
| photoTags = await PhotoTagAccessor.getTagsByUserID(userId); | ||
| } |
There was a problem hiding this comment.
This is a good standard implementation of the cases you can get to try and filter through photo tags. However, there is a terser way to accomplish this sort of functionality. I would encourage you to look at Arushi's search article by options for an idea on how to make this more slick:
nusci-backend/app/controllers/articleController.js
Lines 248 to 273 in e6e9557
| static async getTagsByUserID(userID) { | ||
| await Connection.open(); | ||
| const tags = await PhotoTag.find({ user: new mongoose.Types.ObjectId(userID) }); | ||
| const tags = await PhotoTag.find({ creatingUser: new mongoose.Types.ObjectId(userID) }); |
| * @param {String} user email - The email of the user | ||
| * @returns tags | ||
| */ | ||
| static async getTagsByUser(userEmail) { |
There was a problem hiding this comment.
Perhaps naming this method something else would make it easier to identify how to use it vs the method above
|
Please also merge |
2 participants
Pull Request
Brief Summary
Questions / Considerations for the Future
API Changes
/photo-tag/filterendpoint in photoTagRoutes.Database Changes
New Tests
Closes #199