MAKENTNU / web

The website of the student organization MAKE NTNU, built with Django.
https://makentnu.no
MIT License
9 stars 5 forks source link

Remove Discord username validation #717

Closed Gunvor4 closed 3 months ago

Gunvor4 commented 4 months ago

Proposed changes

Removed the regex validating Discord usernames, since Discord somewhat recently changed their username format (see https://discord.com/blog/usernames and https://support.discord.com/hc/en-us/articles/12620128861463-New-Usernames-Display-Names). Also removed the help_text explaining the old username format.

Areas to review closely

Checklist

(If any of the points are not relevant, mark them as checked)

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.15%. Comparing base (54d6901) to head (74b3839). Report is 4 commits behind head on dev.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/MAKENTNU/web/pull/717/graphs/tree.svg?width=650&height=150&src=pr&token=EL6fslS1y2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MAKENTNU)](https://app.codecov.io/gh/MAKENTNU/web/pull/717?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MAKENTNU) ```diff @@ Coverage Diff @@ ## dev #717 +/- ## ========================================== - Coverage 88.15% 88.15% -0.01% ========================================== Files 152 152 Lines 6188 6186 -2 ========================================== - Hits 5455 5453 -2 Misses 733 733 ``` | [Files](https://app.codecov.io/gh/MAKENTNU/web/pull/717?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MAKENTNU) | Coverage Δ | | |---|---|---| | [src/internal/models.py](https://app.codecov.io/gh/MAKENTNU/web/pull/717?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MAKENTNU#diff-c3JjL2ludGVybmFsL21vZGVscy5weQ==) | `93.78% <100.00%> (ø)` | | | [src/internal/validators.py](https://app.codecov.io/gh/MAKENTNU/web/pull/717?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MAKENTNU#diff-c3JjL2ludGVybmFsL3ZhbGlkYXRvcnMucHk=) | `84.21% <ø> (-1.51%)` | :arrow_down: |
elisakiv commented 4 months ago

The help text also need to be updated in the member model, the discord field in the Django admin panel still asks for the username to include the hashtag and the numbers

ddabble commented 4 months ago

Did you remember to run makemigrations? These changes to the model will necessarily create a new migration :p None of the changes affect the database schema in any way, though, so I think it's better to simply remove help_text (and validators, re: my comment above) from the internal migrations 0015 and 0024 - which are the only ones that include those arguments of the discord_username field 🙂

We can also remove the translation strings for the previous help_text (and discord_username_validator) from django.po; recompiling django.mo shouldn't be necessary, though it won't hurt :)

Gunvor4 commented 4 months ago

Did you remember to run makemigrations? These changes to the model will necessarily create a new migration :p None of the changes affect the database schema in any way, though, so I think it's better to simply remove help_text (and validators, re: my comment above) from the internal migrations 0015 and 0024 - which are the only ones that include those arguments of the discord_username field 🙂

We can also remove the translation strings for the previous help_text (and discord_username_validator) from django.po; recompiling django.mo shouldn't be necessary, though it won't hurt :)

Done and done :-)