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

Uncrispify AccountForm in useradmin #3168

Merged
merged 3 commits into from
Nov 15, 2024

Conversation

johannaengland
Copy link
Contributor

@johannaengland johannaengland commented Nov 7, 2024

Closes #3135.

This is quite a bit more complicated of a refactor. So I would suggest reading it commit by commit.

The form can look four different ways:

  1. Creating a new user: login, name, password fields are empty and read&write, submit field says "Create account"
  2. The default user - only the name can be changed, login and password fields are greyed out/read-only
  3. Normal user - similar to a new user, all fields are read&write, but have current information in them when loading and submit field says "Save changes
  4. Externally managed user (e.g. ldap) - only login and name are visible, but login read-only, has an additional blue info box "External authenticator: {{ authenticator }}"

Urls (when using a dump of the sikt-vk):
http://localhost/useradmin/account/new/ (1. New user)
http://localhost/useradmin/account/0/ (2. Default user)
http://localhost/useradmin/account/1/ (3. Normal user)
http://localhost/useradmin/account/1093/ (4. External user)

As already in the code suggested I split these forms up into two different forms: AccountForm for use cases 1-3 and ExternalAccountForm for use case 4.

@podliashanyk and I made the decision to move the blue "External authenticator" box to the top of the fieldset, that made it much easier to override the template and makes it look more cohesive

Screenshots of that change:
Previously:
IMG_20241107_121309

Now:
IMG_20241107_121341

Copy link

github-actions bot commented Nov 7, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 992 0 10.99s
✅ PYTHON ruff 987 0 0.11s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

@johannaengland johannaengland force-pushed the refactor/uncrispify-account-form-useradmin branch from f39d574 to 4d3a367 Compare November 7, 2024 11:28
Copy link

github-actions bot commented Nov 7, 2024

Test results

    9 files      9 suites   8m 36s ⏱️
2 139 tests 2 139 ✅ 0 💤 0 ❌
4 017 runs  4 017 ✅ 0 💤 0 ❌

Results for commit 1091e15.

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.39%. Comparing base (42c7d52) to head (075c26b).
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3168      +/-   ##
==========================================
- Coverage   60.40%   60.39%   -0.01%     
==========================================
  Files         605      605              
  Lines       43698    43690       -8     
  Branches       48       48              
==========================================
- Hits        26395    26386       -9     
- Misses      17291    17292       +1     
  Partials       12       12              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@johannaengland johannaengland force-pushed the refactor/uncrispify-account-form-useradmin branch 2 times, most recently from 4d8eea0 to 2af3c24 Compare November 14, 2024 08:28
Copy link
Contributor

@podliashanyk podliashanyk left a comment

Choose a reason for hiding this comment

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

Code LGTM 👍
See a suggestion below

Copy link
Contributor

@stveit stveit left a comment

Choose a reason for hiding this comment

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

some small comments

python/nav/web/useradmin/views.py Outdated Show resolved Hide resolved
python/nav/web/useradmin/forms.py Show resolved Hide resolved
@johannaengland johannaengland force-pushed the refactor/uncrispify-account-form-useradmin branch from 1091e15 to add0dff Compare November 15, 2024 12:00
External means managed by ldap for example and internal is a user made by nav
The information box about externally managed accounts is moved into HTML templates
@johannaengland johannaengland force-pushed the refactor/uncrispify-account-form-useradmin branch from add0dff to 075c26b Compare November 15, 2024 12:07
Copy link

sonarcloud bot commented Nov 15, 2024

@johannaengland johannaengland merged commit 705c151 into master Nov 15, 2024
10 of 11 checks passed
@johannaengland johannaengland deleted the refactor/uncrispify-account-form-useradmin branch November 15, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uncrispify AccountForm in useradmin
3 participants