This service will be undergoing maintenance at 00:00 UTC, 2017-10-23 It is expected to last about 30 minutes
Bug 1313751 - Email Notifications show 'katello', not Satellite.
Email Notifications show 'katello', not Satellite.
Status: CLOSED ERRATA
Product: Red Hat Satellite 6
Classification: Red Hat
Component: Branding (Show other bugs)
6.2.0
Unspecified Unspecified
unspecified Severity high (vote)
: Beta
: --
Assigned To: Stephen Benjamin
Roman Plevka
: Triaged
: 1324928 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2016-03-02 05:02 EST by Brad Buckingham
Modified: 2016-07-27 07:27 EDT (History)
9 users (show)

See Also:
Fixed In Version: rubygem-katello-3.0.0.15-1
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-07-27 07:27:05 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
Foreman Issue Tracker 14459 None None None 2016-04-04 10:05 EDT

  None (edit)
Description Brad Buckingham 2016-03-02 05:02:05 EST
Email Notifications show 'katello', not Satellite.  (probably same for foreman - e.g. after provisioning / puppet reports etc)
Comment 2 Stephen Benjamin 2016-03-24 12:17:59 EDT
Indeed, same for foreman.  E.g. the test mail shows "Foreman test mail"

Also, there's a setting called "Email subject prefix" which adds [foreman] to the subject.  This should add [satellite] instead.
Comment 3 Shimon Shtein 2016-03-27 04:53:55 EDT
All name changes like Foreman -> Satellite e.t.c. should be done by the foreman_theme_satellite internal translation process described in: https://gitlab.sat.lab.tlv.redhat.com/satellite6/foreman_theme_satellite#rebase-process-internal-tool-use.
Comment 4 Mike McCune 2016-04-01 00:55:54 EDT
Stephen, 

Mind tackling this one? Would be a MR against the foreman_theme_satellite
Comment 5 Stephen Benjamin 2016-04-01 08:27:12 EDT
These aren't localized strings, but rather identifiers that were renamed to Satellite in 6.1:

  https://github.com/Katello/katello/blob/master/db/seeds.d/106-mail_notifications.rb#L10

The same applies to the setting, that prefix isn't localzied anywhere.


I'm not sure how foreman_theme_satellite would handle these?
Comment 7 Shimon Shtein 2016-04-03 05:30:55 EDT
The theme does not handle identifiers by default, but since it's only an identifier I assume we don't show it to the user.

Setting's full_name does have a translation though - https://github.com/theforeman/foreman/blob/develop/app/helpers/settings_helper.rb#L42

> I'd propose we just get rid of the word "Katello" entirely from the notification upstream, and avoid the problem all together. 

+1
Comment 8 Shimon Shtein 2016-04-03 07:49:37 EDT
Found the code that actually shows strings based on notification identifiers.

To correct it, just add a config/initializers/inflections.rb with the following code to the theme:

ActiveSupport::Inflector.inflections do |inflect|
  inflect.human 'katello_host_advisory', 'Whatever we decide to show to the user'
end
Comment 9 Stephen Benjamin 2016-04-03 14:54:27 EDT
> The theme does not handle identifiers by default, but since it's only an identifier I assume we don't show it to the user.

The labels are shown to the user.  Mail notifications work like Settings used to work, where the user sees the label. 

> Found the code that actually shows strings based on notification identifiers.

Where? I wasn't aware we were humanizing the mail notification labels. Does the inflection cover what's shown in the API and CLI?


And this really totally ignores the upgrade case, which we can't do. In 6.0 and 6.1, the labels were actually satellite_XXXX.  These were seeded and in the database.  Same for every other object that was branded.  So I think with this solution we're going to end up with what appears to be duplicate mail notifications, the formerly hardcoded branded ones with the satellite_host_advisory name, and the smoke and mirrors satellite_host_advisory one.

We'll need a migration most likely for all these cases, no?
Comment 10 Shimon Shtein 2016-04-04 02:47:24 EDT
> Where? I wasn't aware we were humanizing the mail notification labels.

UI: app/views/users/_mail_notifications.html.erb: :label => f.object.mail_notification.name.humanize
API: We show the raw value ("puppet_error_state") api/v2/users/show.json.rabl that calls api/v2/mail_notifications/base.json.rabl
CLI: Based on the output from the UI, but doesn't show the notifications at all (bug).

> So I think with this solution we're going to end up with what appears to be duplicate mail notifications
The user  won't get duplicate notifications, since he is subscribed to only one of them. Although he will see two lines in the user settings screen. We can actually humanize the old name to something like "Warning, will be deprecated".

> We'll need a migration most likely for all these cases, no?
Probably yes, but the migration is minimal - just change name column in mail_notifications table, you won't loose any subscriptions e.t.c. because it's not a primary key for the table.
Comment 11 Stephen Benjamin 2016-04-04 09:51:23 EDT
Really not ideal to show the user a name in the UI and then show them a different one everywhere else.  They won't even be able to use search queries in the API because the names will be different.

I think I'd just rather ask the upstream to remove the "Katello" from the notification label entirely and get rid of this headache forever.

Upstream proposal: https://github.com/Katello/katello/pull/5935
Comment 13 sthirugn@redhat.com 2016-04-07 14:45:16 EDT
*** Bug 1324928 has been marked as a duplicate of this bug. ***
Comment 14 Tazim Kolhar 2016-04-09 08:31:27 EDT
Hi,

      please provide verification steps

thanks and regards,
tazim
Comment 15 Roman Plevka 2016-04-09 13:40:18 EDT
Haven't verified all emails yet, but this should probably go back to ASSIGNED as the test email for Foreman still shows the [foreman] prefix. According to comment #2 it should be [satellite]

(In reply to Stephen Benjamin from comment #2)
> Indeed, same for foreman.  E.g. the test mail shows "Foreman test mail"
> 
> Also, there's a setting called "Email subject prefix" which adds [foreman]
> to the subject.  This should add [satellite] instead.

also, the Sender mail is set to Foreman-noreply@redhat.com which should be changed as well
Comment 17 Stephen Benjamin 2016-04-11 10:22:37 EDT
Verification steps for when this is ON_QA again

1. Under "My Account" page and ensure on the Email tab none of the notification names say "Katello"

2. Under Administer / Settings, ensure email_reply_address says satellite-noreply@<domain>

3. Under Administer / Settings, ensure email_subject_prefix says [satellite]
Comment 19 Shimon Shtein 2016-04-14 05:19:39 EDT
Peter, can you open a separate issue for that? It's an upstream issue that is not related to branding. The branding does not change anything related to OpenSCAP.
Comment 20 Peter Ondrejka 2016-04-14 05:30:02 EDT
(In reply to Shimon Shtein from comment #19)
> Peter, can you open a separate issue for that? It's an upstream issue that
> is not related to branding. The branding does not change anything related to
> OpenSCAP.

Sure, filed https://bugzilla.redhat.com/show_bug.cgi?id=1327090
Comment 21 Roman Plevka 2016-04-20 04:38:19 EDT
VERIFIED
e-mail settings no longer default to 'katello'
Comment 22 Bryan Kearney 2016-07-27 07:27:05 EDT
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2016:1501

Note You need to log in before you can comment on or make changes to this bug.