Optimize JWT token size by extracting user avatars to separate endpoint#36
Optimize JWT token size by extracting user avatars to separate endpoint#36GunnarLieb wants to merge 1 commit intocoldbox-modules:developmentfrom
Conversation
- Remove avatar from JWT token payload and create dedicated /api/v1/users/:id/avatar endpoint. Reduces token size. Backend validates and processes images.
jclausen
left a comment
jclausen
left a comment
There was a problem hiding this comment.
I'm a bit confused on how this PR is optimizing the token size since we already removed that from the custom claims in a previous pull request? We're also resizing the image twice and duplicating a bunch of code.
Some changes requested, as well as some clarification on the need for this PR.
| allowLogin : true | ||
| } | ||
| }, | ||
| defaultAvatar : "data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8QEBEQCgwSExIQEw8QEBD/2wBDAQMDAwQDBAgEBAgQCwkLEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBD/wAARCADIAMgDASIAAhEBAxEB/8QAHQABAAIDAQEBAQAAAAAAAAAAAAcIBAUGAgEDCf/EAEYQAAEDAwICBAsHAgMGBwAAAAEAAgMEBQYHERIhMVFhcQgTFRYiQVWRobHRFDI1c4GS8P8" |
There was a problem hiding this comment.
Let's use the store to provide the default avatar, rather than hard-coding it in to two different places. ( here and above )
| fetchUsers(){ | ||
| usersAPI.list( { "sortOrder" : "lastName DESC, firstName DESC" }, this.authToken ) | ||
| .then( result => this.usersData = result.data ) | ||
| .then( result => { |
There was a problem hiding this comment.
I would just modify line 100 of UserService to pass includes="avatar" to the getMemento() call. Then there's no need for a separate AJAX call for each user. Same with the user API single user method.
| errors : [] | ||
| } | ||
| }, | ||
| defaultAvatar : "data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8QEBEQCgwSExIQEw8QEBD/2wBDAQMDAwQDBAgEBAgQCwkLEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBD/wAARCADIAMgDASIAAhEBAxEB/8QAHQABAAIDAQEBAQAAAAAAAAAAAAcIBAUGAgEDCf/EAEYQAAEDAwICBAsHAgMGBwAAAAEAAgMEBQYHERIhMVFhcQgTFRYiQVSRobHRFDI1c4GS8P8" |
There was a problem hiding this comment.
Same note as above. Just make this a store property and bring it in that way.
| return this; | ||
| } | ||
|
|
||
| function processAvatar(){ |
There was a problem hiding this comment.
I'm a little confused on the need for this, since it's already being resized in the javascript code prior to the upload. Can you clarify?
|
Thanks for reviewing and the remarks, I agree, will create a new PR |
2 participants
create dedicated /api/v1/users/:id/avatar endpoint. Reduces token size. Backend validates and processes images.