🚧(app-regie) use mail domain slug for mail-domains navigation and requests #253
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
|
What's the idea behing this change? Can you provide more details in the description of the pull request with our usual format : Purpose / Proposal? |
daproclaima
force-pushed
the
252-cannot-access-mail-domain-with-id
branch
from
85498b8 to
9447e7b
Compare
daproclaima
changed the title
mjeammet
force-pushed
the
252-cannot-access-mail-domain-with-id
branch
2 times, most recently
from
88db5f2 to
493b641
Compare
|
cannot lint locally, I don't understand why 😢 EDIT : OK, fixed it with a delicate chmod 777. You're free to merge, @daproclaima |
mjeammet
force-pushed
the
252-cannot-access-mail-domain-with-id
branch
from
bc6d427 to
dc53ecc
Compare
mjeammet
approved these changes
Jun 21, 2024
AntoLC
approved these changes
Jun 25, 2024
Comment on lines
38
to
41
| def get_slug(self, instance): | ||
| """Return slug from the domain's name.""" | ||
| return instance.get_slug() | ||
|
|
There was a problem hiding this comment.
@mjeammet Why do you need to override slug ?
When you save your slug, you already use slugify I can see.
https://github.com/numerique-gouv/people/blob/dc53ecc3bc2ca5d11451ecff8af035fdf3f85230/src/backend/mailbox_manager/models.py#L32-L33
There was a problem hiding this comment.
Actually you're right, I don't think we need that. I removed it.
mjeammet
force-pushed
the
252-cannot-access-mail-domain-with-id
branch
from
dc53ecc to
c1f5798
Compare
Stop using mailDomain.id in frontend navigation and mail-domains/ requests. Instead, uses mailDomain slug.
add missing field to MailDomain serializer after commit b4bafb6
Updates tests to use mail domain slug instead of id.
- slug readonly on admin - fix test to expect slug in payload, when retrieving a domain
daproclaima
force-pushed
the
252-cannot-access-mail-domain-with-id
branch
from
c1f5798 to
b324df5
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.
Stop using mailDomain.id in frontend navigation and requests to the backend. Instead, use mailDomain.url as a slug.
A meaningful URL, easy to memorize for the user consulting a specific mail domain view.
With this change, instead of providing a UUID in the frontend application URL, we use the slug of a mail domain.
As a result, a URL that used to have the format
http://localhost:3000/mail-domains/6bbd8081-563a-4c5a-ae02-2785d18f2480will look likehttp://localhost:3000/mail-domains/domainfr.Changes in the code
Each navigation button supposed to redirect to the view of a specific mail domain will embed the slug of the mail domain instead of its id. Also, every http request sent to the django API on the api/mail-domains/ path related to a specific domain and its mailboxes will use the mail domain slug instead of its id (as introduced in commit b4bafb6).