Project

General

Profile

Actions

Bug #21169

closed

Fix deprecated ERB usage in account setup email view

Added by Tom Clegg 7 months ago. Updated 5 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Story points:
0.5

Description

As of #20846, testing services/api in Ruby 3 gave the following warnings.

/home/tom/arvados/services/api/app/views/user_notifier/account_is_setup.text.erb:5: warning: Passing safe_level with the 2nd argument of ERB.new is de\
precated. Do not use it, and specify other arguments as keyword arguments.
/home/tom/arvados/services/api/app/views/user_notifier/account_is_setup.text.erb:5: warning: Passing trim_mode with the 3rd argument of ERB.new is dep\
recated. Use keyword argument like ERB.new(str, trim_mode: ...) instead.

However, if we do the obvious thing:

diff --git a/services/api/app/views/user_notifier/account_is_setup.text.erb b/services/api/app/views/user_notifier/account_is_setup.text.erb
index 352ee7754e..e6349922fa 100644
--- a/services/api/app/views/user_notifier/account_is_setup.text.erb
+++ b/services/api/app/views/user_notifier/account_is_setup.text.erb
@@ -2,4 +2,4 @@

 SPDX-License-Identifier: AGPL-3.0 %>

-<%= ERB.new(Rails.configuration.Users.UserSetupMailText, 0, "-").result(binding) %>
+<%= ERB.new(Rails.configuration.Users.UserSetupMailText, safe_level: 0, trim_mode: "-").result(binding) %>

The result is:

UserNotifierTest#test_account_is_setup = 0.54 s = E

Error:                                                                                                                                                 
UserNotifierTest#test_account_is_setup:                                                                                                                
ActionView::Template::Error: unknown keyword: :safe_level                                                                                              
    app/views/user_notifier/account_is_setup.text.erb:5:in `new'                                                                                       
    app/views/user_notifier/account_is_setup.text.erb:5                                                                                                
    app/mailers/user_notifier.rb:14:in `account_is_setup'                                                                                              
    test/unit/user_notifier_test.rb:36:in `block in <class:UserNotifierTest>'                                                                          

Keeping trim_mode: "-" and removing safe_mode: 0 makes the errors and warnings go away, but what are the other implications of removing that?


Related issues

Related to Arvados - Idea #20846: Support Ubuntu 22.04 LTSResolvedBrett Smith10/30/2023Actions
Actions #1

Updated by Tom Clegg 7 months ago

  • Description updated (diff)
Actions #2

Updated by Tom Clegg 7 months ago

  • Related to Idea #20846: Support Ubuntu 22.04 LTS added
Actions #3

Updated by Brett Smith 7 months ago

  • Target version changed from Future to To be scheduled

Tom Clegg wrote:

Keeping trim_mode: "-" and removing safe_mode: 0 makes the errors and warnings go away, but what are the other implications of removing that?

I'm pretty sure this is what we're supposed to do. It sounds like safe_mode used to set the Ruby $SAFE variable while doing work. This variable is strictly global state in more recent Ruby, and setting it inside procs like ERB was doing is now a noop.

This is also clear if you look at the signature of ERB::new:

new(str, safe_level=NOT_GIVEN, legacy_trim_mode=NOT_GIVEN, legacy_eoutvar=NOT_GIVEN, trim_mode: nil, eoutvar: '_erbout')

It's also consistent with the warning text: "*Do not use* [safe_mode], and specify other arguments as keyword arguments."

Still not 100% sure what $SAFE itself does, but whatever we were relying on it for has been removed from the Ruby itself, so there's nothing for ERB to do here.

Actions #4

Updated by Brett Smith 5 months ago

  • Target version changed from To be scheduled to Development 2024-01-03 sprint
  • Status changed from New to Resolved

Brett Smith wrote in #note-3:

Tom Clegg wrote:

Keeping trim_mode: "-" and removing safe_mode: 0 makes the errors and warnings go away, but what are the other implications of removing that?

I'm pretty sure this is what we're supposed to do.

Peter is doing this in #21059, fba452e1092543f9bbc1fd6b2c87ae9288134b15.

Actions #5

Updated by Brett Smith 5 months ago

  • Assigned To set to Peter Amstutz
Actions

Also available in: Atom PDF