-
Notifications
You must be signed in to change notification settings - Fork 109
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
Updated app to rails 7 #3426
Open
benjaminfaure
wants to merge
18
commits into
development
Choose a base branch
from
rails7
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Updated app to rails 7 #3426
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Generated by 🚫 Danger |
aaronskiba
reviewed
Jul 3, 2024
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 error shows a change in the behaviour of Template.visibility when upgrading to Rails 7:
Failures:
1) Template#upgrade_customization! sets the visibility to Organisationally visible
Failure/Error: expect(subject.visibility).to eql(Template.visibilities['organisationally_visible'])
expected: 0
got: "organisationally_visible"
(compared using eql?)
# ./spec/models/template_spec.rb:1155:in `block (3 levels) in <main>'
# /usr/share/rvm/gems/ruby-3.0.5@upstream/gems/webmock-3.23.1/lib/webmock/rspec.rb:39:in `block (2 levels) in <main>'
Rails 6
aaron@ubuntu:~/Documents/GitHub/roadmap_upstream
$ rails c
Running via Spring preloader in process 1979938
Loading development environment (Rails 6.1.7.7)
3.0.5 :001 > Template.first.visibility
Creating scope :publicly_visible. Overwriting existing method Template.publicly_visible.
Creating scope :organisationally_visible. Overwriting existing method Template.organisationally_visible.
Template Load (1.6ms) SELECT "templates".* FROM "templates" ORDER BY "templates"."id" ASC LIMIT $1 [["LIMIT", 1]]
=> 0
Rails 7
aaron@ubuntu:~/Documents/GitHub/roadmap_upstream
$ rails c
Running via Spring preloader in process 1983441
Loading development environment (Rails 7.1.3.4)
3.0.5 :001 > Template.first.visibility
Template Load (1.5ms) SELECT "templates".* FROM "templates" ORDER BY "templates"."id" ASC LIMIT $1 [["LIMIT", 1]]
=> "organisationally_visible"
Addresses the following error that was encountered when attempting to execute the GitHub Actions that correspond with the edited workflows files: https://github.com/DMPRoadmap/roadmap/actions/runs/9513143079/job/26222602325 3s ``` Run bundle exec rails db:create RAILS_ENV=test To use retry middleware with Faraday v2.0+, install `faraday-retry` gem To use multipart middleware with Faraday v2.0+, install `faraday-multipart` gem; note: this is used by the ManageGHES client for uploading licenses Copying Bootstrap glyphicons to the public directory ... Copying TinyMCE skins to the public directory ... /home/runner/work/roadmap/roadmap/config/initializers/recaptcha.rb:8:in `block in <main>': undefined method `[]' for nil:NilClass (NoMethodError) from /home/runner/work/roadmap/roadmap/vendor/bundle/ruby/3.0.0/gems/recaptcha-5.17.0/lib/recaptcha.rb:37:in `configure' from /home/runner/work/roadmap/roadmap/config/initializers/recaptcha.rb:7:in `<main>' ```
Prior to this commit, the Rails credentials were not being updated during this setup process.
Here are links to the errors being raised prior to this commit: https://github.com/DMPRoadmap/roadmap/actions/runs/9822436610/job/27119190298?pr=3435 https://github.com/DMPRoadmap/roadmap/actions/runs/9822436613/job/27119190303?pr=3435 https://github.com/DMPRoadmap/roadmap/actions/runs/9844975903/job/27179509035?pr=3435
This commit undoes some Rubocop fixes made from a prior commit ( bda5b6e ). However, it also resolves the following error that was being raised: https://github.com/DMPRoadmap/roadmap/actions/runs/9845084297/job/27179836711
Omitting the arguments results in lambda implicitly using self, which appears to be the desired behaviour here. It also resolves the Rubocop offences.
`template.visibilty` now returns a string rather than an integer. The Rails 7 upgrade actually fixes a couple of bugs within `app/views/org_admin/templates/_form.html.erb` and `app/views/org_admin/templates/_show.html.erb`. Prior to this upgrade, template.visibility would return an integer. Now that it is returning a string, the `f.object.visibility == 'organisationally_visible'` and `template.visibility == 'organisationally_visible'` checks within the aforementioned files are behaving as desired.
Prior to this commit, the default checked/unchecked values were used (i.e. "1" would be returned when checked, and "0" would be returned when unchecked). However, the box is meant to be checked when selecting 'organisationally_visible' ('for internal %{org_name} use only'), which makes the default checked/unchecked values opposite to the mapping of our enums (i.e. `{"organisationally_visible"=>0, "publicly_visible"=>1}`).
Rails 7 appears to apply stricter parsing rules. If the Content-Type is not JSON, then the body will not be parsed as JSON.
Prior to this code change, any value assigned to the `'data-method':` attribute of the `link_to` method was not being read (and instead defaulting to `GET`). This was resulting in the breaking of several `spec/features/` tests (https://github.com/DMPRoadmap/roadmap/actions/runs/9946998559/job/27478725801). The `@rails/ujs` library is meant to handle this `'data-method':` attribute.
Fix PostgreSQL GitHub Action and Tests For Rails 7 Upgrade
Added `coder:` and `type:` keywords in various places to address deprecation warnings. Example warning (before adding `type: ` keyword in `app/models/user.rb`: ```` Please pass the class as a keyword argument: serialize :prefs, type: Hash (called from <class:User> at /path/to/app/models/user.rb:73) DEPRECATION WARNING: Passing the class as positional argument is deprecated and will be removed in Rails 7.2. ```
This change addresses the following deprecation warnings: ``` DEPRECATION WARNING: Setting action_dispatch.show_exceptions to false is deprecated. Set to :none instead. (called from rescue in call at /usr/share/rvm/gems/ruby-3.0.5@upstream/gems/actionpack-7.1.3.4/lib/action_dispatch/middleware/debug_exceptions.rb:43) DEPRECATION WARNING: Setting action_dispatch.show_exceptions to false is deprecated. Set to :none instead. (called from rescue in call at /usr/share/rvm/gems/ruby-3.0.5@upstream/gems/actionpack-7.1.3.4/lib/action_dispatch/middleware/show_exceptions.rb:36) ```
Address Deprecation Warnings
Regarding some of the failing checks:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updated app to Rails 7.1