Bug #21169
closed
Fix deprecated ERB usage in account setup email view
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?
Updated by Tom Clegg about 1 year ago
- Related to Idea #20846: Support Ubuntu 22.04 LTS added
Updated by Brett Smith about 1 year ago
- Target version changed from Future to To be scheduled
Tom Clegg wrote:
Keeping
trim_mode: "-"and removingsafe_mode: 0makes 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.
Updated by Brett Smith 12 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 removingsafe_mode: 0makes 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.