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

Make app consistent with upgrades #464

Merged
merged 7 commits into from
Aug 17, 2020

Conversation

cdccollins
Copy link
Contributor

This makes Search Admin consistent with the other apps that have been upgraded to Rails 6

Use the Gemfile.lock to manage the versions instead which should make
it easier to upgrade as it allows bundler a wider set of versions to
resolve the dependencies.
Rails 6 uses zeitwerk for autoloading, and there is some inconsistency with
uppercase "ID" and "Id" in classnames within the app so these uppercase relevant
classnames have been added to the zeitwork.rb file.
Using a sass css compressor causes a scss file to be processed twice
(once to build, once to compress) which breaks the usage of "unquote"
to use CSS that has same function names as SCSS such as max.
@Rhian
Copy link

Rhian commented Aug 13, 2020

Couple of small optional updates (taken from http://railsdiff.org/5.0.0/6.0.3.2)

  • Update csrf metatags to <%= csp_meta_tag %> on line 3 of /app/views/layouts/application.html.erb
  • You could also get rid of /bin/bundle
  • Related to the first point, I notice there is a default GovUK Security Policy header which could be added in config/initializers/csp.rb, as per this commit: alphagov/travel-advice-publisher@91889bb

@cdccollins cdccollins requested a review from Rhian August 14, 2020 15:35
Copy link

@Rhian Rhian left a comment

Choose a reason for hiding this comment

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

Updates look good to me.

@cdccollins cdccollins merged commit ba55443 into master Aug 17, 2020
@cdccollins cdccollins deleted the make-app-consistent-with-upgrades branch August 17, 2020 09:02
kevindew added a commit that referenced this pull request Jan 24, 2023
GOV.UK hadn't intended for this app to have the GOV.UK Content Security
Policy yet, with us first planning to roll out this to frontend app. It
looks like this was added as part of an outsourced Rails update [1],
where the dev couldn't have known about our nuanced context.

As this is an app that doesn't receive a lot of developer attention I'm
disabling this as I don't want breaking changes to the CSP [2] to end up
in this app.

[1]: #464
[2]: alphagov/govuk_app_config#279
kevindew added a commit that referenced this pull request Jan 25, 2023
GOV.UK hadn't intended for this app to have the GOV.UK Content Security
Policy yet, with us first planning to roll out this to frontend app. It
looks like this was added as part of an outsourced Rails update [1],
where the dev couldn't have known about our nuanced context.

As this is an app that doesn't receive a lot of developer attention I'm
disabling this as I don't want breaking changes to the CSP [2] to end up
in this app.

[1]: #464
[2]: alphagov/govuk_app_config#279
kevindew added a commit that referenced this pull request Jan 25, 2023
GOV.UK hadn't intended for this app to have the GOV.UK Content Security
Policy yet, with us first planning to roll out this to frontend app. It
looks like this was added as part of an outsourced Rails update [1],
where the dev couldn't have known about our nuanced context.

As this is an app that doesn't receive a lot of developer attention I'm
disabling this as I don't want breaking changes to the CSP [2] to end up
in this app.

[1]: #464
[2]: alphagov/govuk_app_config#279
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.

2 participants