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

Added descriptions to group selections in Wagtail admin #3636

Merged
merged 19 commits into from
Dec 13, 2023

Conversation

wes-otf
Copy link
Contributor

@wes-otf wes-otf commented Oct 31, 2023

Fixes #3541.

This PR adds descriptive help text to the Wagtail admin edit/create user "Groups" prompt. This was done by creating custom Wagtail forms and overwriting the existing group field, along with adding another field to the groups.py.

The definitions provided by @fourthletter were shortened just a bit for the sake of appearance. If anyone believes they got too truncated I'm fine with updating them.

Screenshots

Screenshot 2023-10-31 at 5 07 29 PM

Screenshot 2023-10-31 at 5 07 15 PM

Cheers!

@wes-otf
Copy link
Contributor Author

wes-otf commented Nov 1, 2023

The CI gods did not favor me today. Couldn't get the test to fail locally but will continue work tomorrow.

@wes-otf wes-otf changed the title Added descriptions to group selections in Wagtail admin WIP: Added descriptions to group selections in Wagtail admin Nov 1, 2023
@fourthletter
Copy link
Contributor

@wes-otf - I forgot to add definitions for staff admin and community reviewer

Staff Admin view and edit all submissions, submit reviews, send determinations, and set up applications. Can disable 2FA, archive submissions, and other components not accessible to other roles.

Community Reviewer is an applicant with access to other applications for the Request with community review (peer review) workflow.

@frjo
Copy link
Contributor

frjo commented Nov 1, 2023

@wes-otf Try running:

pytest --ds=hypha.settings.test --store-durations

and then commit the updated .test_durations file to the PR. This sometimes resolves these kind of issues.

@frjo
Copy link
Contributor

frjo commented Nov 1, 2023

Please also test this on top of the Django 4 and Wagtail 5 PRs. Or we just wait until they have been merged it.

@frjo
Copy link
Contributor

frjo commented Nov 1, 2023

Staff Admin view and edit all submissions, submit reviews, send determinations, and set up applications. Can disable 2FA, archive submissions, and other components not accessible to other roles.

This is not correct. Most of these are reliant on permission settings in Wagtail. These are completely up to each organisation to set to whatever they like.

The only thing I know for sure are linked to "Staf admin" is "View message log".

Important also to point out that "Staff admin" always should be used together with "Staff".

@wes-otf
Copy link
Contributor Author

wes-otf commented Nov 1, 2023

@frjo So should abilities given by wagtail permissions should not be included in the description? Would this effect any of the given descriptions?

These are completely up to each organisation to set to whatever they like.

With the addition of translations in groups.py, wouldn't different orgs be able to customize what that description is?

@wes-otf
Copy link
Contributor Author

wes-otf commented Nov 1, 2023

Just tested with both the Wagtail 5 branch & Django 4 branches, both worked totally fine. Only thing I saw is that our edit/create user templates use the Wagtail shared template field_as_li.html a few times and Wagtail has said this will be removed in a future release. I could swap these out proactively if that would be worthwhile.

Otherwise this should be all set once confirmation is given on those final group descriptions! Tests looked to be failing because of a migration that wasn't made so that is also included here.

@wes-otf wes-otf changed the title WIP: Added descriptions to group selections in Wagtail admin Added descriptions to group selections in Wagtail admin Nov 2, 2023
@wes-otf wes-otf marked this pull request as ready for review November 2, 2023 15:01
@wes-otf
Copy link
Contributor Author

wes-otf commented Nov 2, 2023

The latest updates that are ready for review:

Screenshot 2023-11-02 at 11 02 33 AM

@frjo
Copy link
Contributor

frjo commented Nov 27, 2023

@wes-otf We merge in some other things so there are a merge conflict here.

@frjo
Copy link
Contributor

frjo commented Nov 27, 2023

Would be good to implement this on the "/admin/groups/" view as well.

@wes-otf
Copy link
Contributor Author

wes-otf commented Nov 27, 2023

Roger that, adding that in now.

@frjo frjo added Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Patch Mini change, used in release drafter labels Nov 28, 2023
@wes-otf
Copy link
Contributor Author

wes-otf commented Nov 29, 2023

This should be all set! Here's what the new Wagtail admin group list looks like:

Screenshot 2023-11-29 at 3 16 24 PM

@wes-otf
Copy link
Contributor Author

wes-otf commented Nov 30, 2023

Alright I believe it's actually set now. It looks like the changes in #3664 were overriding the wagtail admin user creation view so I took a stab at making it more programmatic with less code duplication. Saving users still works with the consent checkbox enabled.

@frjo @theskumar let me know if you guys have any more thoughts otherwise it's ready for test

@frjo frjo added Status: Needs testing Tickets that need testing/qa and removed Status: Needs testing Tickets that need testing/qa labels Dec 5, 2023
@frjo
Copy link
Contributor

frjo commented Dec 5, 2023

Needs a new rebase to solve merge conflicts.

After that we put on test just to confirm and then I think it is ready to go.

@wes-otf
Copy link
Contributor Author

wes-otf commented Dec 5, 2023

All set here

@frjo frjo added the Status: Needs testing Tickets that need testing/qa label Dec 7, 2023
@frjo frjo merged commit 6622ca4 into main Dec 13, 2023
10 checks passed
@frjo frjo deleted the enhancement/add-desc-to-user-roles branch February 12, 2024 11:46
wes-otf added a commit that referenced this pull request May 7, 2024
Fixes #3541.

This PR adds descriptive help text to the Wagtail admin edit/create user
"Groups" prompt. This was done by creating custom Wagtail forms and
overwriting the existing group field, along with adding another field to
the
[groups.py](https://github.com/HyphaApp/hypha/blob/enhancement/add-desc-to-user-roles/hypha/apply/users/groups.py#L40).

The
[definitions](#3541 (comment))
provided by @fourthletter were shortened just a bit for the sake of
appearance. If anyone believes they got too truncated I'm fine with
updating them.
wes-otf added a commit that referenced this pull request May 8, 2024
Fixes #3541.

This PR adds descriptive help text to the Wagtail admin edit/create user
"Groups" prompt. This was done by creating custom Wagtail forms and
overwriting the existing group field, along with adding another field to
the
[groups.py](https://github.com/HyphaApp/hypha/blob/enhancement/add-desc-to-user-roles/hypha/apply/users/groups.py#L40).

The
[definitions](#3541 (comment))
provided by @fourthletter were shortened just a bit for the sake of
appearance. If anyone believes they got too truncated I'm fine with
updating them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs testing Tickets that need testing/qa Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Patch Mini change, used in release drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a definition next to each user role in wagtail admin
4 participants