Project

General

Profile

Actions
Copy link

Bug #21169

open

Fix deprecated ERB usage in account setup email view

Added by Tom Clegg 29 days ago. Updated 28 days ago.

Status:
New
Priority:
Normal
Assigned To:
-
Category:
API
Target version:
To be scheduled
Start date:
Due date:
% Done:

0%

Estimated time:
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 - Story #20846: Support Ubuntu 22.04 LTSIn ProgressBrett Smith10/30/2023

Actions
Actions
Copy link
 #1

Updated by Tom Clegg 29 days ago

  • Description updated (diff)
Actions
Copy link
 #2

Updated by Tom Clegg 29 days ago

Actions
Copy link
 #3

Updated by Brett Smith 28 days ago

  • Target version changed from To be groomed 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
Copy link

Also available in: Atom PDF