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

Rename between users #4758

Merged
merged 7 commits into from
Feb 12, 2024
Merged

Rename between users #4758

merged 7 commits into from
Feb 12, 2024

Conversation

brong
Copy link
Member

@brong brong commented Dec 2, 2023

We found an issue in Fastmail production where a user had done:

  1. rename user.A.foo user.B.foo

followed by

  1. rename user.B.foo user.A.foo

Which had caused conversations to be all broken because we weren't correctly detecting this case in the mailbox_rename_nocopy (uuid paths) case.

This PR adds a test for the case, then code to make it work (and yes, I did tdd this!)

@brong brong requested a review from ksmurchison December 2, 2023 11:14
Copy link
Contributor

@ksmurchison ksmurchison left a comment

Choose a reason for hiding this comment

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

The code looks sane to me, but its failing CI

@brong brong force-pushed the rename-between-users branch from cb08357 to 9752dd1 Compare December 12, 2023 11:49
@brong brong requested a review from ksmurchison December 12, 2023 11:49
@brong
Copy link
Member Author

brong commented Dec 12, 2023

CI is fully passing now on my machine (including JMAP-Tester, I added that to my machine). The issue was setting the foldermodseq for the newmbentry too early, so it didn't get included in changes correctly.

@brong brong requested a review from elliefm December 12, 2023 11:51
Copy link
Contributor

@ksmurchison ksmurchison left a comment

Choose a reason for hiding this comment

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

This all looks sane to me, and if it works, it works

@brong brong added backport-to-3.8 for PRs that are to be backported to 3.8 backport-to-3.10 for PRs that are to be backported to 3.10 labels Dec 22, 2023
@brong
Copy link
Member Author

brong commented Dec 22, 2023

@elliefm I've added backport tags to this - it fixes an otherwise bad bug that came in with uuid mailboxes

@elliefm elliefm added the backport-to-3.6 for PRs that are to be backported to 3.6 label Dec 22, 2023
@brong brong force-pushed the rename-between-users branch from 9752dd1 to a6535bb Compare February 11, 2024 22:50
@brong brong merged commit 4d58f33 into master Feb 12, 2024
2 checks passed
@brong brong deleted the rename-between-users branch February 12, 2024 00:41
@elliefm elliefm removed the backport-to-3.8 for PRs that are to be backported to 3.8 label Feb 26, 2024
@elliefm
Copy link
Contributor

elliefm commented Mar 4, 2024

This doesn't backport well to 3.6, because the test Quota.storage_convquota_immediate relies on the userrawquota annotation, which doesn't exist yet. I guess that's why @brong didn't tag this for 3.6 initially?

If we do want to backport this to 3.6, we'll either also need #4203 and #4209 (at least), or need to drop this test

@elliefm elliefm removed the backport-to-3.6 for PRs that are to be backported to 3.6 label Mar 4, 2024
@elliefm elliefm removed the backport-to-3.10 for PRs that are to be backported to 3.10 label Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants