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

Bugfix/fix trailing slashes by removing whitenoise #359

Merged
merged 10 commits into from
May 14, 2024

Conversation

252afh
Copy link
Contributor

@252afh 252afh commented May 14, 2024

Context

Whitenoise seems to be caching the pages at a single url and not respecting the trailing_slash setting. We don't really need whitenoise locally so we can just remove it.

Changes proposed in this pull request

  • Removed whitenoise from settings
  • Removed artifact static urls from urls.py
  • Replaced function in documents.html that calls the file status text with the correct one

Guidance to review

  • Log in and try go to /documents and see that you get redirected to /documents/

Relevant links

Things to check

  • I have added any new ENV vars in all deployed environments
  • I have tested any code added or changed
  • I have run integration tests

{% endcompress %}
{% else %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@@ -9,7 +9,7 @@
<div class="govuk-grid-column-two-thirds">
<h1 class="govuk-heading-xl">Ask any question of documents in your Redbox</h1>
<p class="govuk-body-l">Use Artificial Intelligence (AI) to get insights from your personal document set. You can use up to, and including, Official Sensitive documents.</p>
<p class="govuk-body">If you have an account, please <a class="govuk-link" href="/sign-in">sign in</a> to get started</p>
<p class="govuk-body">If you have an account, please <a class="govuk-link" href="{{url('sign-in')}}">sign in</a> to get started</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've not seen this function syntax in jinja templates before. What is it and is it safe going forward?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a django/jinja2 syntax for constructing urls from the url slug. This is the better way of doing it, you can't get links that go nowhere doing it this way

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it the same or different to this?

Comment on lines +1 to +4
# SASS Compilation (if not using django-compressor)

`npm install -g sass`
`sass style.scss style.css -w`
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like a bad idea to have, at this point, a third nested level of README.

@wpfl-dbt
Copy link
Collaborator

Log in and try go to /documents and see that you get redirected to /documents/

This seems to work the opposite way around -- I go to /documents/ and get /documents. Is this a typo or a problem?

Copy link
Collaborator

@gecBurton gecBurton left a comment

Choose a reason for hiding this comment

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

LGTM

@gecBurton gecBurton merged commit b79170e into main May 14, 2024
9 checks passed
@gecBurton gecBurton deleted the bugfix/fix-trailing-slashes-by-removing-whitenoise branch May 14, 2024 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants