-
Notifications
You must be signed in to change notification settings - Fork 897
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
Create generic task notifications #17835
Conversation
d433e37
to
08a22f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a great start.
Going forward, we should be able to add link to notification (pointing to MiqTask, or the related entity)
08a22f2
to
9fba984
Compare
Can't really test this until the |
9fba984
to
f15d021
Compare
This pull request is not mergeable. Please rebase and repush. |
f15d021
to
755930f
Compare
This pull request is not mergeable. Please rebase and repush. |
@skateman can you unwip this and rebase? I think this can get in |
755930f
to
a999de5
Compare
a999de5
to
b936ca8
Compare
Checked commits skateman/manageiq@2fd2c9a~...b936ca8 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍 thanks @skateman
These notifications types are intended to be use with provider-initiated tasks when there is a need to notify the user about the progress. The
expires_in
andaudience
values are just my assumptions and they might be wrong, this is one of the reasons why this is WIP.I also added helper methods for initiating these notifications from models, you just have to include the
NotificationMixin
concern and you can use thenotify_task(start|finish|fail|update)
methods.If the last argument is omitted, the given record will be set as the notification subject automatically.
Related demo: ManageIQ/manageiq-providers-amazon#468
@miq-bot add_reviewer @martinpovolny