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: do not prematurely dereference status change RelatedDocuments #3835

Merged
merged 3 commits into from
Apr 19, 2022

Conversation

jennifer-richards
Copy link
Member

Fixes crash when displaying RFCs with status change documents.

Also adds tests to cover this and the change to the person_link tag in #3832.

@codecov
Copy link

codecov bot commented Apr 18, 2022

Codecov Report

Merging #3835 (d9a6725) into main (ef4ad54) will increase coverage by 0.03%.
The diff coverage is 98.86%.

@@            Coverage Diff             @@
##             main    #3835      +/-   ##
==========================================
+ Coverage   87.95%   87.99%   +0.03%     
==========================================
  Files         296      297       +1     
  Lines       38795    38769      -26     
==========================================
- Hits        34124    34116       -8     
+ Misses       4671     4653      -18     
Impacted Files Coverage Δ
ietf/meeting/models.py 86.08% <ø> (ø)
ietf/meeting/urls.py 100.00% <ø> (ø)
ietf/meeting/utils.py 90.27% <ø> (-0.29%) ⬇️
ietf/name/models.py 100.00% <ø> (ø)
ietf/meeting/views.py 91.02% <96.00%> (+0.42%) ⬆️
ietf/doc/templatetags/ballot_icon.py 84.89% <100.00%> (ø)
ietf/doc/templatetags/ietf_filters.py 81.40% <100.00%> (+0.15%) ⬆️
ietf/doc/views_ballot.py 91.60% <100.00%> (ø)
ietf/doc/views_charter.py 88.72% <100.00%> (ø)
ietf/doc/views_doc.py 90.44% <100.00%> (+0.05%) ⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70596b7...d9a6725. Read the comment docs.

{% endif %}
{% if proposed_status_changes %}
<div>Proposed status changed by {{ proposed_status_changes|join:", "|urlize_related_source_list }}</div>
<div>Proposed status changed by {{ proposed_status_changes|urlize_related_source_list|join:", " }}</div>
{% endif %}
{% if rfc_aliases %}<div>Also known as {{ rfc_aliases|join:", "|urlize_ietf_docs }}</div>{% endif %}
Copy link
Member

Choose a reason for hiding this comment

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

Same bug on this line, though currently it won't be irritated as the length of rfc_aliases will be 0 or 1 until we handle bcps correctly

Copy link
Member

@rjsparks rjsparks Apr 19, 2022

Choose a reason for hiding this comment

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

eh - no - urlize_ietf_docs will work just fine on a string.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I chased that one for a bit - I think it's used a few places like this. It's a bit of a shame to convert a full-blown Document into its names just to parse those back into links that could have been generated directly from the Document, but it works. It'd be a nice thing to straighten out some time.

@rjsparks rjsparks merged commit 43952a8 into ietf-tools:main Apr 19, 2022
@jennifer-richards jennifer-richards deleted the jennifer/tag-bugs branch April 19, 2022 13:43
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 16, 2022
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.

2 participants