Implementation of updating user avatar. #1549
Conversation
| val workerParams = UploadAvatarWorker.Params(sessionId, newAvatarUri, fileName) | ||
| val workerData = WorkerParamsFactory.toData(workerParams) | ||
|
|
||
| val uploadAvatarWork = workManagerProvider.matrixOneTimeWorkRequestBuilder<UploadAvatarWorker>() |
There was a problem hiding this comment.
I don't think we need to make uploading avatar uses a worker. It's alright if it fails, we'll get an error and can retry later. What do you think about that @bmarty ?
There was a problem hiding this comment.
I think this code is more complicated that I'd expect it to be. We should use the uploadService facilities. I have not checked in the code if it is possible.
| * @param newAvatarUri the new avatar uri of the user | ||
| * @param fileName the fileName of selected image | ||
| */ | ||
| fun updateAvatar(userId: String, newAvatarUri: Uri, fileName: String, matrixCallback: MatrixCallback<Unit>): Cancelable |
There was a problem hiding this comment.
Can we update avatar of other users? otherwise, we should not offer the userId param and inject @userid instead
There was a problem hiding this comment.
The Matrix API takes a userId, that's why it's exposed here... But yes in the app, it will be only possible to change avatar of the current user.
| val uri = image.contentUri | ||
| UCrop.of(uri, destinationFile.toUri()) | ||
| .withOptions( | ||
| UCrop.Options() |
There was a problem hiding this comment.
Didn't check, but if Ucrop options are the same than for attachment, we should extract this in a helper
There was a problem hiding this comment.
Maybe not the same that for attachment. We should limit to a square image, or if possible a round image. But yes it should be extracted and not copy-paste multiple times
| val result = WorkerParamsFactory.fromData<UploadAvatarWorker.OutputParams>(info.outputData) | ||
| cancelableBag.add( | ||
| setAvatarUrlTask | ||
| .configureWith(SetAvatarUrlTask.Params(userId = userId, newAvatarUrl = result?.imageUrl!!)) { |
There was a problem hiding this comment.
I would put the !! to result instead
| // ============================================================================================================== | ||
| // contacts management | ||
| // ============================================================================================================== | ||
| // ============================================================================================================== |
There was a problem hiding this comment.
Please revert those fomartting change
Fixes #1054