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

RAILS Upgrade from 6.1.7 to 7.0.8 #345

Open
wants to merge 18 commits into
base: dev
Choose a base branch
from

Conversation

SewerusHtd
Copy link
Collaborator

Related tasks

  • No task

What's new?

  • Updated Rails from 6.1.7 to 7.0.8

Details of solved problems

Why concurrent-ruby fixed to version 1.3.4
  • issue:
lib/active_support/logger_thread_safe_level.rb:12:in `<module:LoggerThreadSafeLevel>': uninitialized constant ActiveSupport::LoggerThreadSafeLevel::Logger (NameError)
    Logger::Severity.constants.each do |severity|
  • the fix is:
gem 'concurrent-ruby', '1.3.4'
Using argon2_options
  • issue:
NoMethodError: undefined method `encryptor=' for Devise:Module
  config.encryptor = :argon2
        ^^^^^^^^^^^^
/cias-api/config/initializers/devise.rb:278:in `block in <main>'
  • the fix is:
argon2_options: { migrate_from_devise_argon2_v1: true }
Keep using Lockbox encryption
  • problem with nil values of encrypted data (like User.first.email in terminal)
  • what is the problem:
    • in Rails 6.1 code encrypts :email, :first_name, :last_name, :uid is related to Lockbox gem
    • in Rails 7.0 the same code is related to new Active Record Encryption, not Lockbox gem
  • the simplest solution: using has_encrypted instead of encrypts
  • it's this change: 19a90af
Setting session store
  • issue:
ActionDispatch::Request::Session::DisabledSessionError (Your application has sessions disabled. To write to the session you must first configure a session store)
  • the solution:
config.session_store :cookie_store, key: '_interslice_session'
config.middleware.use ActionDispatch::Cookies
config.middleware.use config.session_store, config.session_options
Fixing reading json schemas
  • issue:
Empty input (after ) at line 1, column 2 [parse.c:1064] in '/Users/sewerynpanek/Projects/cias-api/db/schema/question/settings.json
  • the solution is to use File.read(...) when reading model schemas
  • the same solution here: Oj.load_file(ENV['GOOGLE_APPLICATION_CREDENTIALS'])
Do not define finalizer
  • issue:
      NoMethodError:
        undefined method `finalize' for Rack::Test::UploadedFile:Class

            ObjectSpace.define_finalizer(uploaded_file, uploaded_file.class.finalize(tempfile))
  • more details: rack/rack-test@4fa741f
  • the solution is to skip defining finalizer here since this class has it already implemented
  • it's here: 284ede2
Defining `default_url_options[:host]` for test environment
  • issue:
Missing host to link to! Please provide the :host parameter, set default_url_options[:host], or set :only_path to true
  • the solution:
Rails.application.routes.default_url_options[:host] = 'test.host'
Start using allow_other_host when needed
  • issue:
ActionController::Redirecting::UnsafeRedirectError
  • the solution is to set:
allow_other_host: true
Adding faraday-multipart gem
  • issue:
Undefined method multipart in faraday requests
  • the solution is add missing gem responsible for this functionality:
gem 'faraday-multipart'
Setting default template environment variables
  • issue: this code
    @language = language || ENV.fetch('TEXT_TO_SPEECH_LANGUAGE', 'en-US')
    @voice_type = voice_type || ENV.fetch('TEXT_TO_SPEECH_VOICE', 'en-US-Standard-C')

was setting empty strings in @language and @voice_type instance variables (even if there's default value given in ENV.fetch method)

  • the solution is to set template envs TEXT_TO_SPEECH_LANGUAGE and TEXT_TO_SPEECH_VOICE:
TEXT_TO_SPEECH_LANGUAGE=en-US
TEXT_TO_SPEECH_VOICE=en-US-Standard-C

@SewerusHtd SewerusHtd self-assigned this Jan 24, 2025
@krzepecki
Copy link
Collaborator

Setting session store - did you try to simply remove :timeoutable devise extension? JWT tokens have its own mechanism for invalidating, so technically it should not have connection with :timeoutable. See here: waiting-for-dev/devise-jwt#235 (comment)

@SewerusHtd SewerusHtd force-pushed the CIAS30-Rails-upgrade-from-6-1-7-to-7-0-8 branch from b72d24d to 426e721 Compare January 27, 2025 10:46
@SewerusHtd
Copy link
Collaborator Author

Setting session store - did you try to simply remove :timeoutable devise extension? JWT tokens have its own mechanism for invalidating, so technically it should not have connection with :timeoutable. See here: waiting-for-dev/devise-jwt#235 (comment)

I didn't try it before. Checked now and there's the same issue:
image

@@ -65,19 +65,21 @@
it_behaves_like 'permitted user'
end

context 'when params are invalid' do
context 'when params are invalid', skip: 'behaviour not implemented' do
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's this line in the service: https://github.com/Michigan-State-University/cias-api/blob/dev/app/services/v1/organizations/dashboard_sections/update.rb#L26

Because of this line, this test fails - updating is being skipped because name is blank and no error is raised

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -84,19 +84,21 @@
it_behaves_like 'permitted user'
end

context 'when params are invalid' do
context 'when params are invalid', skip: 'behaviour not implemented' do
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's this line in the service: https://github.com/Michigan-State-University/cias-api/blob/dev/app/services/v1/organizations/update.rb#L37

Because of this line, this test fails - updating is being skipped because name is blank and no error is raised

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -91,19 +91,21 @@
it_behaves_like 'permitted user'
end

context 'when params are invalid' do
context 'when params are invalid', skip: 'behaviour not implemented' do
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's this line in the service: https://github.com/Michigan-State-University/cias-api/blob/dev/app/services/v1/health_clinics/update.rb#L25

Because of this line, this test fails - updating is being skipped because name is blank and no error is raised

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -92,19 +92,19 @@
end
end

context 'when params are invalid' do
context 'when params are invalid', skip: 'behaviour not implemented' do
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's this line in the service: https://github.com/Michigan-State-University/cias-api/blob/dev/app/services/v1/health_systems/update.rb#L28

Because of this line, this test fails - updating is being skipped because name is blank and no error is raised

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

let(:message_delivery) { instance_double(ActionMailer::MessageDelivery) }
let(:intervention) { create(:intervention, status: status) }
let(:status) { :draft }
let(:user) { create(:user, :confirmed, :admin) }

context 'intervention is draft' do
it 'dose not schedule send email' do
expect(InterventionMailer).not_to receive(:inform_to_an_email)
it 'does not schedule send email', skip: 'behaviour not implemented' do
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This method is here: https://github.com/Michigan-State-University/cias-api/blob/dev/app/models/intervention.rb#L141

Status (draft) is nowhere checked. We can add return if draft? on the top of this method, but is it expected?

@@ -101,8 +108,9 @@
context "user is #{role}" do
let(:user) { create(:user, :confirmed, role) }

it 'dose not schedule send email' do
expect(SessionMailer).not_to receive(:inform_to_an_email)
it 'does not schedule send email', skip: 'behaviour not implemented' do
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This method is here: https://github.com/Michigan-State-University/cias-api/blob/dev/app/models/intervention.rb#L141

The guest and preview_session roles are nowhere checked

@SewerusHtd SewerusHtd force-pushed the CIAS30-Rails-upgrade-from-6-1-7-to-7-0-8 branch 2 times, most recently from 868ba35 to e596851 Compare January 27, 2025 12:02
@SewerusHtd SewerusHtd force-pushed the CIAS30-Rails-upgrade-from-6-1-7-to-7-0-8 branch from e596851 to cd16d33 Compare January 27, 2025 12:17
@SewerusHtd SewerusHtd marked this pull request as ready for review January 27, 2025 12:51
@SewerusHtd SewerusHtd requested a review from krzepecki January 27, 2025 12:51
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