Show pending training review message on project landing page - #2656
Show pending training review message on project landing page#2656Chrystinne wants to merge 1 commit into
Conversation
lukepayyapilli
left a comment
There was a problem hiding this comment.
Thanks for this, @Chrystinne! This is a genuinely helpful UX improvement, and telling users their training is under review instead of asking them to submit again will save a lot of confusion (and probably some support emails). Moving the "You may submit your training here" text into a proper <li> is a nice cleanup too.
I found one behavioral issue and a few things I'd like to see addressed before merging:
1. Multi-training projects can hide unmet requirements (blocker)
has_training_under_review is true if any required training is in REVIEW, and the template then replaces the whole requirements list with the "under review" message. For a project that requires two trainings, a user who has submitted training A but never started training B will see only "have your submitted training report approved." The list of required trainings and the submit link disappear, so they have no way to discover training B until after A is approved.
I think the condition should be "all trainings the user is still missing are under review" rather than "at least one is." Alternatively, showing per-training status in the list would be even clearer.
2. Reuse the existing get_review() queryset method (blocker)
TrainingQuerySet already has a get_review() method (user/managers.py) that encapsulates what "under review" means, including the DOCUMENT/URL type filtering. Using Training.objects.get_review().filter(...) here would avoid hand-rolling the status filter and importing TrainingStatus into views.py, and it keeps that domain rule in one place.
3. Extract the duplicated template block (blocker)
The new conditional block is identical in both the self-managed and non-self-managed sections, so the template now has two 13-line copies that must be edited in lockstep. Pulling the training requirements block into an {% include %} partial would make future edits much safer. Happy to have this done as part of this PR since it's the change that grew the duplication.
4. Please add a test for the new branch (blocker)
A test covering a user with a training under review (and ideally the multi-training case from point 1) would lock in the behavior and would have caught the issue above. Two small assertions should do it.
Minor, non-blocking suggestions:
- The wording "Have your submitted training report approved" reads as a task the user can't act on. Something like "Your training report is under review. You can check its status on the Certification page" might land better.
- The view now runs two similar queries against the user's trainings for this project. Fetching them once and answering both questions from that queryset would trim a query on a hot page.
Thanks again, the underlying change is a good one and I'm looking forward to seeing it land!
This PR updates the restricted-access message shown in the Files section of a project landing page when the user has already submitted the required training report and it is still under review.
This avoids confusing users about whether they still need to submit a training report. It also adds a link to the Certification page, where users can check the status of their training submissions.