Skip to content

fix: Add Reviews history page to review teams group page. (fixes #9190) #9191

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 28, 2025
Merged

fix: Add Reviews history page to review teams group page. (fixes #9190) #9191

merged 3 commits into from
Jul 28, 2025

Conversation

kivinen
Copy link
Contributor

@kivinen kivinen commented Jul 19, 2025

This implements the Reviews history page for the review teams.

@rjsparks rjsparks marked this pull request as draft July 22, 2025 07:35
@rjsparks
Copy link
Member

Moving this to draft per in-person discussion with Tero

@rjsparks
Copy link
Member

On the first test run, there is only one real failure (mypy). The second failure at

 ======================================================================
FAIL: test_session_attendance (ietf.meeting.tests_views.ProceedingsTests.test_session_attendance)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/__w/datatracker/datatracker/ietf/meeting/tests_views.py", line 9323, in test_session_attendance
    self.assertContains(r, person.plain_name())
  File "/usr/local/lib/python3.9/site-packages/django/test/testcases.py", line 660, in assertContains
    self.assertTrue(
AssertionError: False is not true : Couldn't find 'David O'Connor' in response

is a transient.
The rest are just consequences of the first two.

For the first error, try

HistoricalReviewAssignment = ReviewAssignment.history.model

just before using it in the code rather than importing HistoricalReviewAssignment.

@rjsparks
Copy link
Member

The new view and template are missing test coverage.

kivinen added 2 commits July 25, 2025 10:37
@kivinen
Merge branch 'ietf-tools:main' into fix-9190
2b491be
@kivinen
fix: Added test cases.
cd9827a 
Fixed html to validate, moved sort text to data-text from comments.
Added test cases. fixes ietf-tools#9190
@codecov Codecov
Copy link

codecov bot commented Jul 25, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.75%. Comparing base (f380b1a) to head (cd9827a).
⚠️ Report is 26 commits behind head on main.

Files with missing lines Patch % Lines
ietf/group/views.py 88.23% 2 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff            @@
##             main    #9191    +/-   ##
========================================
  Coverage   88.74%   88.75%            
========================================
  Files         321      320     -1     
  Lines       41853    41674   -179     
========================================
- Hits        37144    36989   -155     
+ Misses       4709     4685    -24

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kivinen kivinen marked this pull request as ready for review July 25, 2025 12:41
@jennifer-richards jennifer-richards merged commit ea8377d into ietf-tools:main Jul 28, 2025
9 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rjsparks rjsparks rjsparks approved these changes

@jennifer-richards jennifer-richards jennifer-richards approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kivinen @rjsparks @jennifer-richards