-
Notifications
You must be signed in to change notification settings - Fork 7
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
Update rails to 7.2.2 #2856
Update rails to 7.2.2 #2856
Conversation
6c75ec7
to
01b25df
Compare
Created review app at https://review.submit-social-housing-data.communities.gov.uk/2856 |
8239e3a
to
8fee431
Compare
7ff4eb4
to
2a9db93
Compare
govuk_link_to "Clear", clear_filters_path(filter_type:, path_params:) | ||
govuk_link_to "Clear", clear_filters_path(filter_type:, filter_path_params:) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Path_params kept on getting cleared, I think because of this: rails/rails#43770
enum :log_type, { lettings: "lettings", sales: "sales" } | ||
enum :rent_type_fix_status, { not_applied: "not_applied", applied: "applied", not_needed: "not_needed" } | ||
enum :failure_reason, { blank_template: "blank_template", wrong_template: "wrong_template" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our current syntax is deprecated in Rails 8
@@ -7,7 +7,6 @@ def initialize(id, hsh, page) | |||
@check_answers_card_number = 0 | |||
@max = 150 | |||
@min = 0 | |||
@hint_text = I18n.t("hints.offered") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This question is a 2023 question, but the translation for this is missing. We now get an exception because of config.i18n.raise_on_missing_translations
@@ -13,7 +13,9 @@ class MergeRequest < ApplicationRecord | |||
request_merged: "request_merged", | |||
deleted: "deleted", | |||
}.freeze | |||
enum status: STATUS | |||
|
|||
attribute :status, :string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enums now have to be backed by either a db column or an attribute
@@ -0,0 +1,17 @@ | |||
en: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know if this is the best place to put it, but as we raise all our missing translations, we have some that are only used in tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a sign we should maybe get rid of them...although actually these are coming from test form definitions, right? Probably not too bad, and can't really think of anywhere better for them to go.
if LocalAuthority.count.zero? | ||
la_path = "config/local_authorities_data/initial_local_authorities.csv" | ||
service = Imports::LocalAuthoritiesService.new(path: la_path) | ||
service.call | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enums can't be empty {}
so this fixes location/scheme tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - feels like we might want to do a fair amount of testing though looking at what's needed to change.
@@ -0,0 +1,17 @@ | |||
en: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a sign we should maybe get rid of them...although actually these are coming from test form definitions, right? Probably not too bad, and can't really think of anywhere better for them to go.
1c51427
to
8173a40
Compare
df6a44c
to
98a274f
Compare
No description provided.