Skip to content

fix: Use the doc name matched by fuzzy_find_documents when rendering #4862

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

Closed
larseggert wants to merge 5 commits into from
Closed

fix: Use the doc name matched by fuzzy_find_documents when rendering #4862

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a36f7d6
fix: Use the doc name matched by fuzzy_find_documents when rendering
larseggert Dec 9, 2022
ac87d98
Merge remote-tracking branch 'origin/main' into fix-4855
larseggert Dec 12, 2022
7574741
Follow suggestion by @rjsparks
larseggert Dec 12, 2022
c9a1477
Merge branch 'main' into fix-4855
larseggert Dec 13, 2022
4efa1d9
Merge branch 'main' into fix-4855
larseggert Dec 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ietf/doc/views_doc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,8 @@ def document_html(request, name, rev=None):
raise Http404("Multiple documents matched: %s" % name)

if found.matched_name.startswith('rfc') and name != found.matched_name:
Copy link
Copy Markdown
Member

@rjsparks rjsparks Dec 14, 2022

Choose a reason for hiding this comment

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

The problem you are working on is dealing with URLs like https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-07. And the issue is that the URL dispatcher is handing

    [name, rev]: '['draft-ietf-oauth-v2', '1-07']'

To the view.
What's in the PR right now won't help since (beyond it being a no-op) the change is in the logic branch gated on finding something that starts with 'rfc'.

Your original commit was closer, and the property I was hoping to get out of the redirect isn't going to happen since the globbing is getting things so wrong to begin with.

I'll take the PR back to that commit, but we need to add tests to make sure that this isn't just pushing the general problem around and breaking other things.

Copy link
Copy Markdown
Member

@rjsparks rjsparks Dec 14, 2022

Choose a reason for hiding this comment

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

Our real problem is that we have

   "rev": r"(?P<rev>[0-9]{1,2}(-[0-9]{2})?)",

in URL_REGEXPS so that we can match charter document versions that look like -00-01, which was a terrible thing to have let happen, and maybe we should refactor the URL parsers to match charters first with that rev pattern and everything else with just [0-9]{2}.

Copy link
Copy Markdown
Member

@rjsparks rjsparks Dec 14, 2022

Choose a reason for hiding this comment

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

For instance, this is 404ing, and it shouldn't:

https://datatracker.ietf.org/doc/html/draft-ietf-tsvwg-ieee-802-11

It should be showing RFC8325.

Your earlier fix causes it to show draft-ietf-tsvwg-ieee-802-11-11, so there's more to fix than just this name change.

return redirect('ietf.doc.views_doc.document_html', name=found.matched_name)
name = found.matched_name
Copy link
Copy Markdown
Member

@rjsparks rjsparks Dec 14, 2022

Choose a reason for hiding this comment

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

This does nothing - name is never used after this assignment.

return redirect('ietf.doc.views_doc.document_html', name=found.matched_name)

doc = found.documents.get()

Expand Down