Rails upgrade 7.0.8 to 7.1.0 by sarvaiyanidhi · Pull Request #5478 · rubyforgood/casa

sarvaiyanidhi · 2024-01-08T22:08:19Z

What github issue is this PR for, if any?

Resolves #5462

What changed, and why?

Upgrade Rails to newer version

How will this affect user permissions?

Volunteer permissions: Not Effected
Supervisor permissions: Not Effected
Admin permissions: Not Effected

How is this tested? (please write tests!) 💖💪

Have made changes in existing test to pass as per new configuration

Screenshots please :)

NA

…_path= option

…ons to false is deprecated. Set to :none instead

…ed_id for a new record issue

sarvaiyanidhi · 2024-01-09T00:34:05Z

          <% if current_organization.court_report_template.attached? %>
-            <%= link_to 'Download Current Template', rails_blob_path(current_organization.court_report_template, only_path: true), class: "btn btn-info" %>
+          <% ActiveStorage::Current.url_options = { host: request.base_url } %>
+           <%= link_to 'Download Current Template', current_organization.court_report_template.url(only_path: true), class: "btn btn-info" %>


Had to remove rails_blob_path and update with url code to handle ActionView::Template::Error: Cannot get a signed_id for a new record error.

Referred this open issue - https://stackoverflow.com/questions/77201333/cannot-get-a-signed-id-for-a-new-record-with-url-for-after-attaching-pictur

…preview paths

sarvaiyanidhi · 2024-01-09T01:09:11Z

Have kept this file for just review. Later I will delete and update application.rb to load 7.1 defaults

sarvaiyanidhi · 2024-01-09T01:12:04Z

    it "shows casa orgs" do
-      page = request.parsed_body
+      # Code changes to fix response as earlier HTML String instead of Nokogiri::HTML5::Document object as received in Rails 7.1.0 to pass expectation
+      page = request.parsed_body.to_html


Had to update code as request.parsed_body in Rails 7.1 was returning Nokogiri::HTML5::Document object which was failing spec

makes sense

sarvaiyanidhi · 2024-01-09T01:17:56Z

Getting same below error for all failing 337 specs

can't modify frozen Array: ["/home/runner/work/casa/casa/lib", #Pathname:/home/runner/work/casa/casa/lib/mailers/previews, "/home/runner/work/casa/casa/test/mailers/previews"]

But I am not able to replicate same error locally by running bin/rails spec and all tests are passing locally.

Trying to figure out issue, but can anyone try locally by pulling branch and check if encountering same issue.

sarvaiyanidhi · 2024-01-10T16:51:50Z

  # config.action_view.annotate_rendered_view_with_filenames = true
+
+  # Raise error when a before_action's only/except options reference missing actions
+  config.action_controller.raise_on_missing_callback_actions = false


By default this was set to true but I have changed it to false as it was failing specs for verify_authorized action

sarvaiyanidhi · 2024-01-10T19:45:18Z

Hi @FireLemons Can you please review code and also help with issue I am facing with rspec failure as I am not able to replicate same issue locally and able to pass all specs.

compwron

Seems reasonable!

schoork · 2024-01-15T20:52:58Z

    # config.eager_load_paths << Rails.root.join("extras")

-    config.action_mailer.preview_path ||= defined?(Rails.root) ? Rails.root.join("lib", "mailers", "previews") : nil
+    config.action_mailer.preview_paths << (defined?(Rails.root) ? Rails.root.join("lib", "mailers", "previews") : nil)


I'm wondering if the difference between ||= and << is what's causing the failing rspec test in github @sarvaiyanidhi

In Rails 7.1.0 there is now support for multiple preview paths so I am appending as per new configuration.

https://edgeguides.rubyonrails.org/upgrading_ruby_on_rails.html#support-multiple-preview-paths-for-actionmailer-preview

In one of the earlier commit, I still reverted to earlier code base to check if specs pass with it but encountered same issue so not sure if this is the problem 1d639db

schoork · 2024-01-15T21:12:24Z

Getting same below error for all failing 337 specs

can't modify frozen Array: ["/home/runner/work/casa/casa/lib", #Pathname:/home/runner/work/casa/casa/lib/mailers/previews, "/home/runner/work/casa/casa/test/mailers/previews"]

But I am not able to replicate same error locally by running bin/rails spec and all tests are passing locally.

Trying to figure out issue, but can anyone try locally by pulling branch and check if encountering same issue.

I can confirm they pass locally.

sarvaiyanidhi · 2024-01-17T03:08:16Z

Getting same below error for all failing 337 specs
can't modify frozen Array: ["/home/runner/work/casa/casa/lib", #Pathname:/home/runner/work/casa/casa/lib/mailers/previews, "/home/runner/work/casa/casa/test/mailers/previews"]
But I am not able to replicate same error locally by running bin/rails spec and all tests are passing locally.
Trying to figure out issue, but can anyone try locally by pulling branch and check if encountering same issue.

I can confirm they pass locally.

Thank you for the confirmation @schoork

compwron · 2024-01-26T19:19:08Z

This is sooooo close to being mergeable... how can we get it over the line?

compwron · 2024-01-26T19:21:01Z

What if we merge and then fix-forward? I don't have time to own it right now but it might be a possibility this weekend...

sarvaiyanidhi · 2024-01-31T03:32:08Z

This is sooooo close to being mergeable... how can we get it over the line?

Yes, let me try to fix spec issue again. I am trying to replicate issue locally so that I can figure out solution.

sarvaiyanidhi · 2024-01-31T18:49:31Z

Hi @compwron @schoork Finally spec issue is fixed. It was small fix but took some time to evaluate the exact cause.
Basically I had to add mailers folder in config.autoload_lib(ignore: %w[assets tasks mailers]) so that it won't get eager loaded as it is not necessary.

Also, this file config/initializers/new_framework_defaults_7_1.rb is just kept for you to review changes applicable in upgrade and can be removed before merging code in main branch once changes are confirmed.

Please review PR and let me know if I need to do any more changes in code. Thanks

compwron · 2024-01-31T18:58:33Z

Very cool!! Will review

compwron · 2024-01-31T18:59:37Z

  # by default. You can change it below and use your own secret key.
  # config.secret_key = 'a42b150b56c158d6e064aa257c8434390c921811007f42c5aa16c7455b78e3777087c5a4bcf18bfb0886fdd9296dec34eaa02fc0b47bed58417347eba971ee2d'

+  # Fixes rspec triggering Rails.application.secrets deprecation warning for Rails 7.1.0 upgrade


schoork · 2024-01-31T19:11:42Z

Programmer of the month award! 🎉🏆🥇

sarvaiyanidhi added 9 commits January 3, 2024 14:12

Update Rails version in Gemfile from 7.0.8 to 7.1.0

123c200

rails app:update and overrides code

6c2f97e

fixture singular path warning fix, Nokogiri parsed_body method fix

23befdf

Fix for deprecation warning for Rails.application.secrets and preview…

c7b9f24

…_path= option

Configuration changes related to Rails 7.1.0

78bd955

Changes for Nokogiri parsed_body to html to match expectation

67204da

Changes for DEPRECATION WARNING: Setting action_dispatch.show_excepti…

33edac1

…ons to false is deprecated. Set to :none instead

Fixed failing spec

49f45e8

Code changes to handle ActionView::Template::Error: Cannot get a sign…

097686a

…ed_id for a new record issue

sarvaiyanidhi requested review from FireLemons, compwron and littleforest as code owners January 8, 2024 22:08

sarvaiyanidhi marked this pull request as draft January 8, 2024 22:08

github-actions Bot added dependencies Touches dependency files ruby Touches Ruby code zzz_archived: Tests no-skin-tone erb Touches ERB templates labels Jan 8, 2024

sarvaiyanidhi added 2 commits January 8, 2024 17:20

Ruby Standard fix

7749518

Ruby Standard fix

c6d07d8

sarvaiyanidhi commented Jan 9, 2024

View reviewed changes

Config changes to handle deprecation warining of supporting multiple …

d267aa9

…preview paths

sarvaiyanidhi commented Jan 9, 2024

View reviewed changes

sarvaiyanidhi added 4 commits January 8, 2024 20:46

Fixing rspec issue

63e833a

Reverted deprecation warning changes for spec failing issue

1d639db

Changes for ActionMailer deprecation warning

7577090

Ruby Standard Fix

d7b4b1f

sarvaiyanidhi commented Jan 10, 2024

View reviewed changes

Updated code to load defaults 7.1 and fix file load issue

5506197

Ruby Lint issue fix

6e57ee4

compwron marked this pull request as ready for review January 13, 2024 03:32

compwron approved these changes Jan 13, 2024

View reviewed changes

schoork reviewed Jan 15, 2024

View reviewed changes

Merged main branch and fixed failing spec

6d9e514

sarvaiyanidhi added 3 commits January 30, 2024 22:50

Reverted preview_path to old code to check failing spec issue

1a05d0d

Reverted to actual code as specs were still failing with old code

2f7ee4d

Fix for failing specs

52b5a59

compwron mentioned this pull request Jan 31, 2024

By default this was set to true but I have changed it to false as it was failing specs for verify_authorized action #5502

Closed

compwron reviewed Jan 31, 2024

View reviewed changes

compwron approved these changes Jan 31, 2024

View reviewed changes

compwron merged commit f62f955 into rubyforgood:main Jan 31, 2024

JoshDevHub mentioned this pull request Feb 17, 2024

Remove unused actions in controller filters #5535

Merged

compwron added 🧪 Tests Tests and removed zzz_archived: Tests no-skin-tone labels May 3, 2026

Uh oh!

Conversation

sarvaiyanidhi commented Jan 8, 2024

What github issue is this PR for, if any?

What changed, and why?

How will this affect user permissions?

How is this tested? (please write tests!) 💖💪

Screenshots please :)

Uh oh!

sarvaiyanidhi Jan 9, 2024

Choose a reason for hiding this comment

Uh oh!

sarvaiyanidhi Jan 9, 2024

Choose a reason for hiding this comment

Uh oh!

sarvaiyanidhi Jan 9, 2024

Choose a reason for hiding this comment

Uh oh!

compwron Jan 13, 2024

Choose a reason for hiding this comment

Uh oh!

sarvaiyanidhi commented Jan 9, 2024

Uh oh!

sarvaiyanidhi Jan 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarvaiyanidhi commented Jan 10, 2024

Uh oh!

compwron left a comment

Choose a reason for hiding this comment

Uh oh!

schoork Jan 15, 2024

Choose a reason for hiding this comment

Uh oh!

sarvaiyanidhi Jan 17, 2024

Choose a reason for hiding this comment

Uh oh!

schoork commented Jan 15, 2024

Uh oh!

sarvaiyanidhi commented Jan 17, 2024

Uh oh!

compwron commented Jan 26, 2024

Uh oh!

compwron commented Jan 26, 2024

Uh oh!

sarvaiyanidhi commented Jan 31, 2024

Uh oh!

sarvaiyanidhi commented Jan 31, 2024

Uh oh!

compwron commented Jan 31, 2024

Uh oh!

compwron Jan 31, 2024

Choose a reason for hiding this comment

Uh oh!

schoork commented Jan 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sarvaiyanidhi Jan 10, 2024 •

edited

Loading