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?
Related issues
Updated by Tom Clegg 6 months ago
- Related to Idea #20846: Support Ubuntu 22.04 LTS added
Updated by Brett Smith 6 months 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 4 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.