feat: image service integrated with feature flag #2003
Conversation
…-Squad/website-backend into feat/image-service-live
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -60,7 +60,8 @@ | |||
| userValidator.validateImageVerificationQuery, | |||
| users.verifyUserImage | |||
| ); | |||
| router.get("/picture/:id", authenticate, authorizeRoles([SUPERUSER]), users.getUserImageForVerification); | |||
| router.get("/picture/all", authenticate, authorizeRoles([SUPERUSER]), users.getAllUsersPhotoVerificationRequests); | |||
Check failure
Code scanning / CodeQL
Missing rate limiting High
| router.patch( | ||
| "/avatar/verify/:id", | ||
| authenticate, | ||
| authorizeRoles([SUPERUSER]), | ||
| checkIsVerifiedDiscord, | ||
| updateDiscordImageForVerification | ||
| ); | ||
| router.patch( | ||
| "/avatar/update/:discordId", | ||
| authenticate, |
Check failure
Code scanning / CodeQL
Missing rate limiting High
ivinayakg
force-pushed
the
feat/image-service-live
branch
from
7818242 to
ec1480f
Compare
ivinayakg
changed the title
| * @return {Promise<{Object}|{Object}>} | ||
| * @throws {Error} - If error occurs while fetching user's image verification entry | ||
| */ | ||
| const getAllUsersPhotoVerificationRequests = async (username = null) => { |
There was a problem hiding this comment.
is there any confusion or problem this may cause?
There was a problem hiding this comment.
So when I read the the function name, what i understand is get all users photo verification requests, is it doing the same?
…-Squad/website-backend into feat/image-service-live
models/users.js
Outdated
| url: profileImageUrl, | ||
| publicId: profileImageID, | ||
| approved: false, | ||
| date: admin.firestore.Timestamp.fromDate(new Date()), |
There was a problem hiding this comment.
Lets not use this formate, lets change it to the standard one which we are following at other place
models/users.js
Outdated
| url: profileImageUrl, | ||
| publicId: profileImageID, | ||
| approved: false, | ||
| date: admin.firestore.Timestamp.fromDate(new Date()), |
There was a problem hiding this comment.
what is date here?
Did you mean createdAt?
There was a problem hiding this comment.
again, this was already present there, therefore I opted for using the current ones only. But I can make the change to createdAt, as that will make more sense.
There was a problem hiding this comment.
Or, I'll rather use updatedAt as the one single picture entry in the photo-verification object can be updated.
| await documentRef.update({ status: photoVerificationRequestStatus.APPROVED }); | ||
| await updateUserPicture( | ||
| { url: photoVerificationObject.profile.url, publicId: photoVerificationObject.profile.publicId }, | ||
| userId | ||
| ); |
There was a problem hiding this comment.
Do both the call needs to happen in a series?
There was a problem hiding this comment.
yes, first the photo-verification-object needs to be updated, then the user picture needs to be updated too.
| const verificationImagesSnapshotQuery = photoVerificationModel.where( | ||
| "status", | ||
| "==", | ||
| photoVerificationRequestStatus.PENDING | ||
| ); | ||
|
|
There was a problem hiding this comment.
NIT: why not declare/initialize it near to the line where its used?
| }); | ||
|
|
||
| const photoVerificationObject = await Promise.all(photoVerificationPromises); | ||
| return photoVerificationObject; |
There was a problem hiding this comment.
Are results being paginated?
There was a problem hiding this comment.
no for now the results are not paginated, but I can add this into in optimization ticket, wdyt?
routes/discordactions.js
Outdated
| router.patch( | ||
| "/avatar/verify/:id", | ||
| authenticate, | ||
| authorizeRoles([SUPERUSER]), | ||
| checkIsVerifiedDiscord, | ||
| updateDiscordImageForVerification | ||
| ); | ||
| // here "id" is the user's discord id | ||
| router.patch( | ||
| "/avatar/photo-verification-update/:id", |
There was a problem hiding this comment.
Is it following route naming conventions?
Date: 13/04/2024
Developer Name: Vinayak Goyal
Issue Ticket Number
closes #1924
Description
Test Coverage