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

Custom registration form error messages #975

Merged
merged 7 commits into from
Mar 3, 2022
Merged

Custom registration form error messages #975

merged 7 commits into from
Mar 3, 2022

Conversation

i-oden
Copy link
Member

@i-oden i-oden commented Mar 2, 2022

Added custom messages. Currently not showing up, for them to actually be visible we need to add novalidate to the html form.

  • Tests passing
  • Black formatting
  • [ - ] Migrations for any changes to the database schema
  • Rebase/merge the dev branch
  • Note in the CHANGELOG

@i-oden i-oden requested a review from alneberg March 2, 2022 11:42
@i-oden i-oden self-assigned this Mar 2, 2022
@i-oden i-oden added the bug Something isn't working label Mar 2, 2022
@i-oden i-oden requested review from talavis and aanil March 2, 2022 11:44
@codecov
Copy link

codecov bot commented Mar 2, 2022

Codecov Report

Merging #975 (b08e53b) into dev (1773e20) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #975      +/-   ##
==========================================
- Coverage   87.84%   87.81%   -0.03%     
==========================================
  Files          26       26              
  Lines        2903     2898       -5     
==========================================
- Hits         2550     2545       -5     
  Misses        353      353              
Impacted Files Coverage Δ
dds_web/forms.py 97.29% <100.00%> (-0.08%) ⬇️
dds_web/web/user.py 83.33% <100.00%> (-0.27%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1773e20...b08e53b. Read the comment docs.

@i-oden i-oden changed the title Reg form messages Custom registration form error messages Mar 2, 2022
@ewels
Copy link
Contributor

ewels commented Mar 2, 2022

Nice! Much of this validation we can also do in the front-end if we want, both with native browser controls and maybe some simple javascript. Can be nice to get immediate validation errors before submitting the form.

Any chance the things like username / password lengths etc could be put in some kind of config variable to make them easier to reuse?

@i-oden
Copy link
Member Author

i-oden commented Mar 2, 2022

@ewels Should be possible to create a custom field so I'll have a look now!

@ewels
Copy link
Contributor

ewels commented Mar 2, 2022

Sorry, not in the output (these work as html attributes so can just be printed into the HTML template), I more just mean in the Python so that we don't have to keep 3 lots of validation parameters in sync..

@i-oden
Copy link
Member Author

i-oden commented Mar 2, 2022

@ewels Yeah started looking into that too but I think I'll create a card for this for now. But definitely a good point, should add some config for username, password etc.

@i-oden
Copy link
Member Author

i-oden commented Mar 2, 2022

@ewels #978

<!-- Unit -->
{% if unit %}
Unit name:
{{ unit }}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@aanil aanil left a comment

Choose a reason for hiding this comment

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

👍 Looks ok!

@i-oden i-oden merged commit 49ababc into dev Mar 3, 2022
@i-oden i-oden deleted the reg-form-messages branch March 3, 2022 11:32
@i-oden i-oden mentioned this pull request Mar 3, 2022
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants