Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable notifier role by default #22587

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

jrafanie
Copy link
Member

It's easy to configure smtp and not realize you need to enable this role to get email functionality working.

See also:

#10504
ManageIQ/manageiq-ui-classic#2079

It's easy to configure smtp and not realize you need to enable this role to get email functionality working.

See also:

ManageIQ#10504
ManageIQ/manageiq-ui-classic#2079
@miq-bot
Copy link
Member

miq-bot commented Jun 27, 2023

Checked commit jrafanie@178ac1f with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. ⭐

@Fryguy
Copy link
Member

Fryguy commented Jun 28, 2023

@miq-bot cross-repo-test /all

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Jun 28, 2023
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jun 28, 2023
Followup to ManageIQ#22587

Enabling notifier but forgetting to set smtp settings isn't technically an error but it's
an application problem.  The same can be said if you configure smtp but don't enable notifier.

Previously, we could only prevent saving configuration if it was invalid.  Now, we can notify
someone who might be able to fix the problem.

TODO: We still have errors about notifications without duplicate messages.
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Jun 28, 2023
Followup to ManageIQ#22587

Enabling notifier but forgetting to set smtp settings isn't technically an error but it's
an application problem.  The same can be said if you configure smtp but don't enable notifier.

Previously, we could only prevent saving configuration if it was invalid.  Now, we can notify
someone who might be able to fix the problem.

TODO: We still have errors about notifications without duplicate messages.
@jrafanie
Copy link
Member Author

Only failure in cross repo seems to be an unrelated cic error

@kbrock kbrock merged commit 80beb4b into ManageIQ:master Jul 11, 2023
9 checks passed
@kbrock kbrock assigned kbrock and unassigned Fryguy Jul 11, 2023
@kbrock kbrock deleted the enable_notifier_role_by_default branch July 11, 2023 16:47
@Fryguy
Copy link
Member

Fryguy commented Jul 25, 2023

Backported to petrosian in commit f41da48.

commit f41da489a028386e447cf4f4f4b33ae1b006200d
Author: Keenan Brock <keenan@thebrocks.net>
Date:   Tue Jul 11 12:46:29 2023 -0400

    Merge pull request #22587 from jrafanie/enable_notifier_role_by_default
    
    Enable notifier role by default
    
    (cherry picked from commit 80beb4bd691eeb6c7281b7902953a1eb01eabf17)

Fryguy pushed a commit that referenced this pull request Jul 25, 2023
Enable notifier role by default

(cherry picked from commit 80beb4b)
@Fryguy Fryguy added this to the Petrosian milestone Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Petrosian
Development

Successfully merging this pull request may close these issues.

4 participants