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

fix: Shows requested reviews for doc fixes #6022

Merged
merged 2 commits into from
Jul 25, 2023
Merged

Conversation

kivinen
Copy link
Contributor

@kivinen kivinen commented Jul 22, 2023

Adds requested reviews to the document information page. Moved the due date of the incomplete reviews out from the badge to the actual text.

Fixes #4881.

@@ -351,6 +351,9 @@
{% for review_assignment in review_assignments %}
{% include "doc/review_assignment_summary.html" with current_doc_name=doc.name current_rev=doc.rev %}
{% endfor %}
{% for review_request in review_requests %}
{% include "doc/review_request_summary.html" with current_doc_name=doc.name current_rev=doc.rev %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{% include "doc/review_request_summary.html" with current_doc_name=doc.name current_rev=doc.rev %}
{% include "doc/review_request_summary.html" with current_doc_name=doc.name current_rev=doc.rev only %}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea what that change would do so can't really comment on that.

Copy link
Member

Choose a reason for hiding this comment

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

it passes only the current_doc_name and current_rev to the review_request_summary template instead of the full context available to the template you are calling {% include from.

Copy link
Member

Choose a reason for hiding this comment

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

but I'm really confused - review_request_summary.html only uses review_request, why are you passing current_doc_name and current_rev to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it is cut & paste from the above where it was giving same parameters to the doc/review_assignment_symmary.html.

I copied the review_assignment_summary.html as a base for the review_request_summary.html and at that point I did not know whether the lines using current_doc stays in or not.

In the end I did not need them as they are used to remove the version number from the line telling which version the review was done for in case it is current revision and document.

So most likely I should simply change it to

{% include "doc/review_request_summary.html" with review_request only %}

or do I need to use review_request=review_request before only?

Copy link
Member

Choose a reason for hiding this comment

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

you need to use review_request=review_request

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, now this should be done. I also did the same for the review_assignment_summary include.

@kivinen kivinen changed the title Fix: Shows requested reviews for doc Fix: Shows requested reviews for doc fixes #4881 Jul 22, 2023
@rjsparks rjsparks changed the title Fix: Shows requested reviews for doc fixes #4881 fix: Shows requested reviews for doc fixes #4881 Jul 22, 2023
@codecov
Copy link

codecov bot commented Jul 22, 2023

Codecov Report

Merging #6022 (7f87556) into main (09f3477) will decrease coverage by 0.01%.
The diff coverage is 89.47%.

❗ Current head 7f87556 differs from pull request most recent head 29788b1. Consider uploading reports for the commit 29788b1 to get more accurate results

@@            Coverage Diff             @@
##             main    #6022      +/-   ##
==========================================
- Coverage   88.67%   88.67%   -0.01%     
==========================================
  Files         288      288              
  Lines       40001    40009       +8     
==========================================
+ Hits        35471    35476       +5     
- Misses       4530     4533       +3     
Files Changed Coverage Δ
ietf/doc/views_stats.py 74.80% <ø> (ø)
ietf/ietfauth/widgets.py 84.61% <ø> (ø)
ietf/nomcom/views.py 92.90% <33.33%> (-0.22%) ⬇️
ietf/doc/utils.py 87.15% <100.00%> (+0.01%) ⬆️
ietf/doc/views_doc.py 91.63% <100.00%> (+<0.01%) ⬆️
ietf/nomcom/templatetags/nomcom_tags.py 70.83% <100.00%> (+3.64%) ⬆️
ietf/review/utils.py 92.08% <100.00%> (+2.50%) ⬆️
ietf/submit/forms.py 79.02% <100.00%> (+0.03%) ⬆️

... and 8 files with indirect coverage changes

@rjsparks rjsparks changed the title fix: Shows requested reviews for doc fixes #4881 fix: Shows requested reviews for doc fixes Jul 22, 2023
@rjsparks rjsparks merged commit b24dd44 into ietf-tools:main Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Requested directorate reviews are not shown on the document status page until (?)
3 participants