Open exalate-issue-sync[bot] opened 1 week ago
Sasha Dresden commented: So I did some research on this and there are several things I have found out.
It is possible to override this, but we don’t do so very often. Looking at our code base and the /api/docs/ the only place we are doing that is for the oidc calls. I’m not quite sure why we don’t have trailing slashes for them. Perhaps they prefer no trailing slash? Or just whoever set up these routes didn’t add them. But I tried making the mock_oidc calls with a trailing slash and they all work. I did not test with the non-mocked oidc calls, as that would require a push to dev, but I imagine it would behave the same.
An easy way to make sure redirects don’t happen is to turn off APPEND_SLASH in django. This feature is what is handling the redirects for when the front-end doesn’t have the correct trailing slash. It does a redirect with the trailing slash. This is robust as it allows people to make mistakes by forgetting to type the trailing slash and it will still work properly, but it does come with a performance overhead. Also, I noticed it doesn’t always work. I tried deleting a contact without the trailing slash and APPEND_SLASH gave an error as it wasn’t able to redirect it. {noformat}RuntimeError: You called this URL via DELETE, but the URL doesn't end in a slash and you have APPEND_SLASH set. Django can't redirect to the slash URL while maintaining DELETE data. Change your form to point to localhost:8080/api/v1/contacts/665c6c35-29fe-4f6c-b390-96d318627f95/ (note the trailing slash), or set APPEND_SLASH=False in your Django settings.{noformat}
I see two possible solutions. # Turn off APPEND_SLASH in django and update all the front end end points to be properly formatted. # Risks: # Potential for missing endpoints. *#* New endpoints need to be properly formatted or the call will fail. # Benefits: # Forces developers to properly format their api calls during development. # Use an interceptor on the front end to handle automatically adding the trailing slash before the request is sent. # Risks: # While we don’t have any endpoints that don’t have trailing slashes currently, we might in the future, and this would prevent those links from working. # Wanted to point out that the oidc links still works since those are handled as href links which interceptors do not intercept. # Benefits: *#* No risk of missing endpoint fixes on the front end. #** Technically we don’t have to turn off APPEND_SLASH in django, but I would still recommend it. Especially if we would rather have failures over expensive redirects and would still catch anything that somehow slipped through the the interceptor.
The second option is a lot easier and less time consuming to implement and has lower risk now, whereas the first option will require a comprehensive sweep of the front end, so it’ll take a lot longer to implement but we could run into issues in the future if we choose to break the django default of always having a trailing slash.
Sasha Dresden commented: Wanted to move this to Code Review but needed something for the PR field so I put the PR I put in for [https://fecgov.atlassian.net/browse/FECFILE-1785|https://fecgov.atlassian.net/browse/FECFILE-1785|smart-link] which implements the front-end interceptor to show how a demo of Option 2. As mentioned in the write-up, Option 1 would require a lot more time investment.
Identify potential solutions to avoid committing front end code that requests the api with urls that are correct except for missing a
/
at the end. These are easy to miss because the api redirects and the app behaves correctly. Consider automated solutions and change of process solutions.Upon review, follow up tickets will be made for any solutions we decide to act on.
QA Notes
null
DEV Notes
Note: time box to “2” points
Design
null
See full ticket and images here: FECFILE-1843