cfpb / regulations-site

(DEPRECATED) Web interface for viewing U.S. federal regulations and other regulatory information
Other
28 stars 43 forks source link

provide fallback when fetching subterps #837

Closed grapesmoker closed 6 years ago

grapesmoker commented 7 years ago

This PR provides a fallback for fetching subterps (interpretations to subparts). The problem here is that when regsite gets a request to fetch a subterp, it doesn't just forward that URL to regcore; it actually munges the URL somewhat to infer what the corresponding endpoint in regcore is. I have no idea why they aren't the same, but that's how it is. This causes some real problem when you want to grab something like a subterp for subpart B, let's say, which has label e.g. 1005-Subpart-B-Interp, but registe is telling regcore to look up 1005-Interp. Anyway, what this PR does is try the "old" way, but if that fails, try just fetching that label directly, i.e. if fetching a subterp by hitting 1005-Interp on regcore returns nothing, try getting the original label.

The other change modifies the way that relevant sections are extracted from the appendix interps.

Testing:

Everything that worked before should continue working as it previously has. This PR is necessary to get the subterps working on the reg E changes, but all tests should pass as before.

chosak commented 7 years ago

Could you please give an example of a URL from Reg E that fails without this PR? I tried running the production release (2.1.13) against the RegML in your regulations-xml#29 PR, and I'm able to hit URLs like http://localhost:8000/eregulations/1005-Appendices-Interp/2016-24506#1005-A-Interp-4-iii-B locally.

grapesmoker commented 7 years ago

What happens when you navigate to, say, http://localhost:8000/1005-C/2011-31725, and then try and navigate to a subterp? Does that work? Because it doesn't for me.

grapesmoker commented 7 years ago

Also, there's a conflict here with the change you made to the section regexp in urls.py. The problem there is that if you change that URL, it hits an entirely different view, fetching a section instead of a subterp.

The problem we have to deal with is that within Reg E, there are two different ways of structuring the interps, because the entire structure of the reg changed at notice 2012-1728. What happened was that a previously empty implicit subpart was actually split into two explicit subparts, with subterps that are attached to the specific subparts. We need to be able to support the viewing of both the current regulation, and the old regulation, and that's the problem that I'm trying to fix. I actually think that we should probably roll back that change to the section regex in favor of this change.

chosak commented 7 years ago

What happens when you navigate to

Ah, you're right - if I try to hit http://localhost:8000/eregulations/partial/1005-Subpart-Interp/2011-31725, I get a 404.

it hits an entirely different view

You're right! I didn't realize this. The way the regexes are written in urls.py it wasn't immediately obvious to me how that change would cause problems. Even so, I'm still a bit unclear as to what the proper fix to the various 500s would be, then.

Take for example internal platform issue #2323, which raises this exception:

NoReverseMatch: Reverse for 'chrome_section_diff_view' with arguments '(u'1005-Appendices-Interp', u'2016-24506', u'2016-24503_20181001')' and keyword arguments '{}' not found. 1 pattern(s) tried: ['eregulations/diff/(?P[\d]+[-][\w]+)/(?P[-\d\w]+)/(?P[-\d\w]+)$']

Am I correct in interpreting that as trying to create a sidebar diff link for the subterp section ("1005-Appendices-Interp")? In urls.py I see chrome_section_diff_view but there's no separate chrome_subterp_diff_view, which would mean that this one view (and its pattern) would have to be able to generate diff links for subterps. That's why I loosened that regex pattern, so that it could properly handle those links.

Is it the case that your Reg E content fixes (and TOC reorg) somehow modify the reg content so that these links aren't generated anymore? If not, and it's still appropriate to generate a diff link for this, how else could we do it? Some kind of new "section or subterp" regex that gets passed to that particular diff view?