-
Notifications
You must be signed in to change notification settings - Fork 73
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
Add approval workflow for paid registrar requests #3659
Add approval workflow for paid registrar requests #3659
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3659 +/- ##
===========================================
- Coverage 69.64% 69.11% -0.53%
===========================================
Files 54 54
Lines 7339 7467 +128
===========================================
+ Hits 5111 5161 +50
- Misses 2228 2306 +78 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! This is going to be such a helpful feature!!
A couple thoughts related to searching:
-
great idea to re-use the user list markup
-
I think if no search results are found, it's a little confusing, it looks like the form maybe didn't submit
-
I think the experience of submitting the search form could be frustrating for keyboard users and screen reader users, since focus will reset to the top of the page upon form submission. Since this is an internal, admin-only page, and since this page isn't currently set up to have any javascript... it's probably not worth the effort to address now. But, if we ever get around to redesigning this part of the app, good for us to think about.
LGTM, left a few remarks and questions
perma_web/perma/templates/user_management/approve_pending_registrar.html
Outdated
Show resolved
Hide resolved
''' | ||
Does the firm signup form submit as expected? Success cases. | ||
''' | ||
firm_organization_form = self.create_firm_organization_form() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to officially apologize for these tests.
Thanks for the review @rebeccacremona! I fixed the issues you raised as well as a couple other small bugs I discovered. Going ahead and merging this. |
This builds upon our work in #3595 and #3631 to add an approval workflow for sign-up requests by paid registrars (a.k.a. "firms"). An approval workflow already exists for library sign-up requests; this branch generalizes the library approval form so it supports firms as well.
Before
After