Chore: create setAvatarFromService endpoint #26402
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #26402 +/- ##
===========================================
- Coverage 40.88% 39.06% -1.83%
===========================================
Files 791 748 -43
Lines 17809 18647 +838
Branches 1928 1424 -504
===========================================
+ Hits 7281 7284 +3
- Misses 10232 11141 +909
+ Partials 296 222 -74
Flags with carried forward coverage won't be shown. Click here to find out more. |
Co-authored-by: Guilherme Gazzo <guilhermegazzo@gmail.com>
Co-authored-by: Guilherme Gazzo <guilhermegazzo@gmail.com>
| @@ -226,6 +227,43 @@ API.v1.addRoute( | |||
| }, | |||
| ); | |||
|
|
|||
| API.v1.addRoute( | |||
| 'users.setAvatarFromService', | |||
There was a problem hiding this comment.
there is no need to create a new endpoint.. we already have the endpoint users.setAvatar, if it is missing this functionality we should just improve it.
| @@ -29,7 +31,8 @@ Meteor.methods({ | |||
| let user; | |||
|
|
|||
| if (userId && userId !== Meteor.userId()) { | |||
| if (!hasPermission(Meteor.userId(), 'edit-other-user-avatar')) { | |||
| // eslint-disable-next-line @typescript-eslint/no-non-null-assertion | |||
There was a problem hiding this comment.
instead of having to disable this rule, you can make your code compliant. one way to achieve that is assigning Meteor.userId() value to a variable and checking/using that variable, so something like:
const userId = Meteor.userId();
if (!userId) {
throw new Error('generic error');
}
// now you can use userId here, no need to disable a TS lint rule
if (!hasPermission(userId, 'edit-other-user-avatar')) {
// throw ...
}| before((done) => { | ||
| updateSetting('Accounts_AllowUserAvatarChange', true).then(() => { | ||
| updatePermission('edit-other-user-avatar', ['admin', 'user']).then(done); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
please use async/await (just like the lines above):
| before((done) => { | |
| updateSetting('Accounts_AllowUserAvatarChange', true).then(() => { | |
| updatePermission('edit-other-user-avatar', ['admin', 'user']).then(done); | |
| }); | |
| }); | |
| before(async () => { | |
| await updateSetting('Accounts_AllowUserAvatarChange', true); | |
| await updatePermission('edit-other-user-avatar', ['admin', 'user']); | |
| }); |
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
1 similar comment
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
Proposed changes (including videos or screenshots)
Created the 'users.setAvatarFromService' endpoint for the
apps/meteor/client/hooks/useUpdateAvatar.tsfile, and added Ajv validations.Issue(s)
Steps to test or reproduce
Further comments