Bug 1313751

Summary: Email Notifications show 'katello', not Satellite.
Product: Red Hat Satellite Reporter: Brad Buckingham <bbuckingham>
Component: BrandingAssignee: Stephen Benjamin <stbenjam>
Status: CLOSED ERRATA QA Contact: Roman Plevka <rplevka>
Severity: high Docs Contact:
Priority: unspecified    
Version: 6.2.0CC: bbuckingham, cwelton, ehelms, mmccune, pondrejk, rplevka, sshtein, stbenjam, sthirugn
Target Milestone: UnspecifiedKeywords: Triaged
Target Release: Unused   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
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 11:27:05 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Brad Buckingham 2016-03-02 10:02:05 UTC
Email Notifications show 'katello', not Satellite.  (probably same for foreman - e.g. after provisioning / puppet reports etc)

Comment 2 Stephen Benjamin 2016-03-24 16:17:59 UTC
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 08:53:55 UTC
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 04:55:54 UTC
Stephen, 

Mind tackling this one? Would be a MR against the foreman_theme_satellite

Comment 5 Stephen Benjamin 2016-04-01 12:27:12 UTC
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 09:30:55 UTC
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 11:49:37 UTC
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 18:54:27 UTC
> 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 06:47:24 UTC
> 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 13:51:23 UTC
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 18:45:16 UTC
*** Bug 1324928 has been marked as a duplicate of this bug. ***

Comment 14 Tazim Kolhar 2016-04-09 12:31:27 UTC
Hi,

      please provide verification steps

thanks and regards,
tazim

Comment 15 Roman Plevka 2016-04-09 17:40:18 UTC
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 which should be changed as well

Comment 17 Stephen Benjamin 2016-04-11 14:22:37 UTC
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 09:19:39 UTC
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 09:30:02 UTC
(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 08:38:19 UTC
VERIFIED
e-mail settings no longer default to 'katello'

Comment 22 Bryan Kearney 2016-07-27 11:27:05 UTC
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