Conversation
briri
commented
Jul 31, 2026
briri
commented
- Upgrade to Rails v8.1 - fixes #270
- Patch API auth to validate that user is still active
- Fix issue where deleting a contributor or related work was not automatically updating the DOI landing pages #277
| return Language.sorted_by_abbreviation if Rails.env.development? | ||
|
|
||
| Rails.cache.fetch('languages', expires_in: 1.hour) { Language.sorted_by_abbreviation } | ||
| Rails.cache.fetch('languages', expires_in: 1.hour) { Language.sorted_by_abbreviation.to_a } |
There was a problem hiding this comment.
Change in Rails 8 wants items for the cache to be in standard arrays instead of ActiveRecord collection of objects
| has_many :notes | ||
|
|
||
| after_save :notify_plan_subscribers | ||
| after_commit :notify_plan_subscribers |
There was a problem hiding this comment.
the after_save hook only applies to insert and update. This after_commit includes delete as well
|
|
||
| def self.many? | ||
| Rails.cache.fetch([model_name, 'many?'], expires_in: 1.hour) { all.many? } | ||
| Rails.cache.fetch([model_name, 'many?'], expires_in: 1.hour) { count > 1 } |
There was a problem hiding this comment.
store a true/false boolean in the cache instead of a call to the function
| @api_client = User.where(email: token[:client_id]).first | ||
| # Valid if User is active, has permission to use the API and | ||
| # the :client_secret matches the token | ||
| usr = User.where(email: token[:client_id], active: true, api_token: @client_secret).first |
There was a problem hiding this comment.
We were previously just looking up the user by email and not verifying that their account was still active.
There was a problem hiding this comment.
If the@client_secret is nil or blank, could the user still authenticate without providing a secret?
| # exec "./bin/rails", "server", *ARGV | ||
|
|
||
| # ==================================================================================== | ||
| # DMP Tool custom dev script below (commented out core Rails server command above). |
There was a problem hiding this comment.
Rails upgrades sometimes overwrite these bin/ and config/ files so I had to splice our customizations back in. I decided to add comments to help indicate what's ours versus core rails
There was a problem hiding this comment.
Let the upgrade overwrite our old customization of this file since we don't really use it and run inside of Docker now when running locally
| # Question Formats controller, currently just the one action | ||
| get 'question_formats/rda_api_address' => 'question_formats#rda_api_address' | ||
|
|
||
| resources :notes, only: %i[create update archive] do |
There was a problem hiding this comment.
Rails 8 did not like the non-standard archive here. It was redudant anyway with the member definition below
jupiter007
left a comment
jupiter007
left a comment
There was a problem hiding this comment.
Looks good.
Just had a question about what happens in authentication_service.rb if @client_secret is nil. Does the authentication still happen?
| @api_client = User.where(email: token[:client_id]).first | ||
| # Valid if User is active, has permission to use the API and | ||
| # the :client_secret matches the token | ||
| usr = User.where(email: token[:client_id], active: true, api_token: @client_secret).first |
There was a problem hiding this comment.
If the@client_secret is nil or blank, could the user still authenticate without providing a secret?
| @@ -0,0 +1,6 @@ | |||
| #!/usr/bin/env ruby | |||
There was a problem hiding this comment.
Interesting. I wonder if it catches the same vulnerabilities as dependabot?
Good question. I'd assume we want to block that, so I've added a check for nil prior to the query. |
2 participants
