Bug 1313751
Summary: | Email Notifications show 'katello', not Satellite. | ||
---|---|---|---|
Product: | Red Hat Satellite | Reporter: | Brad Buckingham <bbuckingham> |
Component: | Branding | Assignee: | Stephen Benjamin <stbenjam> |
Status: | CLOSED ERRATA | QA Contact: | Roman Plevka <rplevka> |
Severity: | high | Docs Contact: | |
Priority: | unspecified | ||
Version: | 6.2.0 | CC: | bbuckingham, cwelton, ehelms, mmccune, pondrejk, rplevka, sshtein, stbenjam, sthirugn |
Target Milestone: | Unspecified | Keywords: | 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
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. 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. Stephen, Mind tackling this one? Would be a MR against the foreman_theme_satellite 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? 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 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 > 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? > 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. 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 *** Bug 1324928 has been marked as a duplicate of this bug. *** Hi, please provide verification steps thanks and regards, tazim 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 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] 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. (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 VERIFIED e-mail settings no longer default to 'katello' 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 |