Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't shadow-replace Attachments with the 1st one when forwarding a Message #1923

Merged
merged 8 commits into from
Jun 20, 2024

Conversation

KevinBoulongne
Copy link
Contributor

@KevinBoulongne KevinBoulongne commented Jun 18, 2024

Depends on #1929

We've got an issue where, when forwarding an email, we replace all of its Attachments with the 1st one that we find.

It was caused by a wrong usage of uploadLocalUri, that we supposed couldn't be null.
The truth was that, when forwarding emails, Attachments don't have any uploadLocalUri.

@KevinBoulongne KevinBoulongne added the bug Something isn't working label Jun 18, 2024
@KevinBoulongne KevinBoulongne changed the title Don't replace Attachments with the 1st one when forwarding a Message Don't shadow-replace Attachments with the 1st one when forwarding a Message Jun 18, 2024
Copy link
Contributor

@FabianDevel FabianDevel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add like 2 lines to avoid doing the clear and the addAll if nothing change

If we forward a draft with a lot of attachment, that is a waste to clean the realm attachments, then add the same attachments back in realm

@KevinBoulongne
Copy link
Contributor Author

KevinBoulongne commented Jun 18, 2024

We could add like 2 lines to avoid doing the clear and the addAll if nothing change

If we forward a draft with a lot of attachment, that is a waste to clean the realm attachments, then add the same attachments back in realm

We are not sure the Attachments in the attachmentsLiveData and in the draft.attachments are already the same list to begin with, so I'm not sure we can do that.

Edit : finally, we dit it.

@KevinBoulongne KevinBoulongne requested review from FabianDevel and a team June 18, 2024 13:00
@KevinBoulongne KevinBoulongne added the rebase Add this label to rebase the PR label Jun 19, 2024
@github-actions github-actions bot removed the rebase Add this label to rebase the PR label Jun 19, 2024
@KevinBoulongne KevinBoulongne changed the base branch from master to uiAttachments-liveData-access June 20, 2024 08:42
@github-actions github-actions bot added the dependent This MR depends on another PR label Jun 20, 2024
Base automatically changed from uiAttachments-liveData-access to master June 20, 2024 11:44
@github-actions github-actions bot removed the dependent This MR depends on another PR label Jun 20, 2024
Copy link

@KevinBoulongne KevinBoulongne added the rebase Add this label to rebase the PR label Jun 20, 2024
@github-actions github-actions bot removed the rebase Add this label to rebase the PR label Jun 20, 2024
Copy link

sonarcloud bot commented Jun 20, 2024

@KevinBoulongne KevinBoulongne merged commit dd700a0 into master Jun 20, 2024
4 checks passed
@KevinBoulongne KevinBoulongne deleted the fix-forward-attachments branch June 20, 2024 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants