✨(dimail) send pending mailboxes upon domain activation #635
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mjeammet
force-pushed
the
mpj/dimail/enable-pending-mailboxes
branch
3 times, most recently
from
f83a1f2 to
4e40e96
Compare
Morendil
reviewed
Jan 16, 2025
mjeammet
commented
Jan 16, 2025
sdemagny
reviewed
Jan 20, 2025
mjeammet
force-pushed
the
mpj/dimail/enable-pending-mailboxes
branch
2 times, most recently
from
470a3c2 to
3bc4024
Compare
mjeammet
force-pushed
the
mpj/dimail/enable-pending-mailboxes
branch
2 times, most recently
from
04ea13d to
f0edee4
Compare
sdemagny
requested changes
Feb 2, 2025
| @@ -23,6 +23,28 @@ | |||
| ) | |||
|
|
|||
|
|
|||
| @pytest.mark.django_db | |||
| def test_admin_action__should_not_sync_if_domain_enabled(client): | |||
| """Test admin action to check health of some domains""" | |||
There was a problem hiding this comment.
la docstring n'est pas bonne
src/backend/mailbox_manager/admin.py
Outdated
| @@ -13,9 +13,15 @@ | |||
| @admin.action(description=_("Synchronise from dimail")) | |||
| def sync_mailboxes_from_dimail(modeladmin, request, queryset): # pylint: disable=unused-argument | |||
| """Admin action to synchronize existing mailboxes from dimail to our database.""" | |||
There was a problem hiding this comment.
Peut-être rajouter qu'uniquement les boites mail des domaines enabled sont synchronisables.
|
|
||
|
|
||
| @pytest.mark.django_db | ||
| def test_admin_action__fetch_domain_status_from_dimail(client): | ||
| def test_admin_action__should_not_sync_if_domain_enabled(client): |
There was a problem hiding this comment.
C'est l'inverse c'est pour les domains not enabled.
Mais est-ce que ça ne serait pas encore mieux d'avoir à la place de _admin_action__ qui est déjà dans le nom du module, l'action en question ? Ça serait plus facile de savoir tout de suite quelle action de l'admin on teste.
Suggested change
| def test_admin_action__should_not_sync_if_domain_enabled(client): | |
| def test_sync_mailboxes__should_not_sync_if_domain_is_not_enabled(client): |
There was a problem hiding this comment.
absolument ! ouhla ça commence à sentir la fatigue, cette PR ^
mjeammet
force-pushed
the
mpj/dimail/enable-pending-mailboxes
branch
from
f0edee4 to
c1dc4ea
Compare
sdemagny
reviewed
Feb 3, 2025
| def test_sync_mailboxes__should_not_sync_if_domain_is_not_enabled( | ||
| domain_status, client | ||
| ): | ||
| """Mailboxes should not be sync'ed on non-enabled domaisn""" |
There was a problem hiding this comment.
Suggested change
| """Mailboxes should not be sync'ed on non-enabled domaisn""" | |
| """Mailboxes should not be sync'ed on non-enabled domains""" |
sdemagny
approved these changes
Feb 3, 2025
mjeammet
force-pushed
the
mpj/dimail/enable-pending-mailboxes
branch
2 times, most recently
from
b6eae39 to
01fa7b9
Compare
send creation requests to dimail for all pending mailboxes when domain goes from "pending" to "enabled".
dimail's ok response upon mailbox creation is used in several tests. All those tests now reference proper response available in fixtures.
Pending, failed and deactivated domains should not be sync'ed.
mjeammet
force-pushed
the
mpj/dimail/enable-pending-mailboxes
branch
from
01fa7b9 to
c5f8cc3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
send creation requests to dimail for all pending mailboxes
when domain goes from "pending" to "enabled".
Proposal
Description...
For next PR