Project Tags

None

Referenced Files

None

Subscribers

mollekopf

Restricted Project

Details

Reviewers

mollekopf

Group Reviewers

Restricted Project

Commits

rKfa5008536cae: Delete an existing user mailbox in IMAP::createUser()

Diff Detail

Repository

rK kolab

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

machniak requested review of this revision.Jul 3 2025, 1:40 PM

machniak created this revision.

Harbormaster completed remote builds in B54459: Diff 15624.Jul 3 2025, 1:40 PM

mollekopf subscribed.Jul 4 2025, 6:48 AM

mollekopf added inline comments.

src/app/Backends/IMAP.php
38–41	I think we should fail here instead of automatically deleting. This is just a backstop to make sure we don't leak a mailbox, so I'd rather not automatically delete at this point because: Something went wrong earlier and we should be investigating this case. We don't want to risk accidentally deleting an active malibox because we erroneously end up in this code-path.

mollekopf requested changes to this revision.Jul 4 2025, 6:48 AM

This revision now requires changes to proceed.Jul 4 2025, 6:48 AM

Throw an exception if mailbox already exists (added a way to disable this)

Harbormaster completed remote builds in B54468: Diff 15633.Jul 4 2025, 10:07 AM

Remove redundant code

Harbormaster completed remote builds in B54471: Diff 15636.Jul 4 2025, 10:14 AM

Use saveQuietly()

Harbormaster completed remote builds in B54477: Diff 15642.Jul 4 2025, 10:40 AM

mollekopf accepted this revision.Jul 11 2025, 11:15 AM

mollekopf added inline comments.

src/app/Backends/IMAP.php
38–41	Why is it possible to skip this check? For testing? If there is no specific reason I think we should remove this check.
src/app/Jobs/User/CreateJob.php
106	Doesn't this automatically fail on exception?

This revision is now accepted and ready to land.Jul 11 2025, 11:15 AM

mollekopf requested changes to this revision.Jul 11 2025, 11:15 AM

This revision now requires changes to proceed.Jul 11 2025, 11:15 AM

machniak added inline comments.Jul 11 2025, 11:18 AM

src/app/Backends/IMAP.php
38–41	I described this in the ticket. For my purposes I want to be able to run many times `php artisan migrate:refresh --seed` on an existing system.
src/app/Jobs/User/CreateJob.php
106	It automatically fails job on "mailbox already exists" case, any other exception will kill the job and it will be re-tried again.

mollekopf accepted this revision.Jul 15 2025, 12:40 PM

mollekopf added inline comments.

src/app/Jobs/User/CreateJob.php
106	I think this would deserve a comment, but makes sense.

This revision is now accepted and ready to land.Jul 15 2025, 12:40 PM

Closed by commit rKfa5008536cae: Delete an existing user mailbox in IMAP::createUser(). · Explain WhyJul 15 2025, 12:46 PM

This revision was automatically updated to reflect the committed changes.

machniak added a commit: rKfa5008536cae: Delete an existing user mailbox in IMAP::createUser().

Delete an existing user mailbox in IMAP::createUser()
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 15723

src/app/Backends/IMAP.php

src/app/Backends/IMAP/Exceptions/MailboxExistsException.php

src/app/Jobs/User/CreateJob.php

src/phpunit.xml

src/tests/Feature/Backends/IMAPTest.php

src/tests/Feature/Jobs/User/CreateTest.php

Delete an existing user mailbox in IMAP::createUser()ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 15723

src/app/Backends/IMAP.php

src/app/Backends/IMAP/Exceptions/MailboxExistsException.php

src/app/Jobs/User/CreateJob.php

src/phpunit.xml

src/tests/Feature/Backends/IMAPTest.php

src/tests/Feature/Jobs/User/CreateTest.php

Delete an existing user mailbox in IMAP::createUser()
ClosedPublic
Actions

Revision Contents
Changeset List