Skip to content

Commit 30c7450

Browse files
Peter Giacomo Lombardocursoragent
andcommitted
Address Copilot review feedback on notify-by-email errors.
Use literal gettext calls in human_attribute_name so strings are extractable, and reword owner-only validation to "allowed only for owners." Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 3ce9199 commit 30c7450

2 files changed

Lines changed: 12 additions & 11 deletions

File tree

app/models/concerns/pwpush/notifiable_by_email.rb

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,6 @@ module NotifiableByEmail
55
extend ActiveSupport::Concern
66
MAX_NOTIFY_BY_EMAILS = 5
77

8-
NOTIFY_BY_EMAIL_HUMAN_ATTRIBUTE_NAMES = {
9-
notify_emails_to: "Recipient emails",
10-
notify_emails_to_locale: "Notification language"
11-
}.freeze
12-
138
included do
149
attr_accessor :notify_emails_to, :notify_emails_to_locale, :notify_emails_to_required, :notify_by_email_creator, :notify_by_email_skip_limit_validation, :notify_by_email_recipients, :notify_by_email_locale
1510

@@ -34,8 +29,14 @@ def notify_by_email_available?
3429

3530
class_methods do
3631
def human_attribute_name(attribute, options = {})
37-
name = NOTIFY_BY_EMAIL_HUMAN_ATTRIBUTE_NAMES[attribute.to_sym]
38-
name ? _(name) : super
32+
case attribute.to_sym
33+
when :notify_emails_to
34+
_("Recipient emails")
35+
when :notify_emails_to_locale
36+
_("Notification language")
37+
else
38+
super
39+
end
3940
end
4041
end
4142

@@ -74,8 +75,8 @@ def validate_notify_by_email_availability
7475
end
7576

7677
unless notify_by_email_creator == user
77-
errors.add(:notify_emails_to, _("are allowed for only owners")) if notify_emails_to.present?
78-
errors.add(:notify_emails_to_locale, _("is allowed for only owners")) if notify_emails_to_locale.present?
78+
errors.add(:notify_emails_to, _("are allowed only for owners")) if notify_emails_to.present?
79+
errors.add(:notify_emails_to_locale, _("is allowed only for owners")) if notify_emails_to_locale.present?
7980

8081
return
8182
end

test/models/concerns/pwpush/notifiable_by_email_test.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ class Pwpush::NotifiableByEmailTest < ActiveSupport::TestCase
8585

8686
assert @other_user != @user
8787
assert_not @push.valid?
88-
assert_includes @push.errors[:notify_emails_to], "are allowed for only owners"
89-
assert_includes @push.errors[:notify_emails_to_locale], "is allowed for only owners"
88+
assert_includes @push.errors[:notify_emails_to], "are allowed only for owners"
89+
assert_includes @push.errors[:notify_emails_to_locale], "is allowed only for owners"
9090
end
9191

9292
# Test notify_by_email_limit validation

0 commit comments

Comments
 (0)