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

[MM-507,699]: Added feature to receive notifications for different Jira actions. #1097

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

Kshitij-Katiyar
Copy link
Contributor

@Kshitij-Katiyar Kshitij-Katiyar commented Jul 5, 2024

Summary

  • To get a DM notification from the Jira plugin, the user has to meet certain criteria like being assigned or mentioned. But now even if a user is a reporter or watching that issue, he will still get a DM notification.
  • This PR contains all the changes from Pr #872 & Pr #840 So this PR alone is enough to fix this issue so we can close Pr #872 & Pr #840 afterward.
  • Updated commands for notifications are:
  1. /jira instance settings notification reporter on
  2. /jira instance settings notification watching on
  3. /jira instance settings notification assignee on
  4. /jira instance settings notification mention on

Screenshots

Watching

image

Reporter

image

Invalid command

image
image
image

Issue

@Kshitij-Katiyar Kshitij-Katiyar changed the title Mm 507,699 [MM-507,699]: Added feature to recieve notification for different Jira actions Jul 5, 2024
@Kshitij-Katiyar Kshitij-Katiyar changed the title [MM-507,699]: Added feature to recieve notification for different Jira actions [MM-507,699]: Added feature to receive notifications for different Jira actions. Jul 5, 2024
@wiggin77 wiggin77 added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core committer labels Jul 9, 2024
@wiggin77 wiggin77 requested a review from esethna July 9, 2024 16:20
@esethna
Copy link

esethna commented Jul 9, 2024

Thanks @Kshitij-Katiyar! A few comments:

  1. Can we remove the term "because" in the notifications:
    image
  2. Can you please share a screenshot of the /jira help command? If not done already we should add these new options to that response
  3. The screenshot shows "task", but is this term dynamic depending on the ticket type? (ie story, bug etc?)
  4. What is the default for these, and if someone runs /jira notifications on/off without the role specified, what happens to the values of the notifications watching and notifications reporter? I'm trying to figure out how we treat these settings, ie as subsettings of "notifications" as a whole, or treated as separate entities that are always disconnected from the state of notifications for being assigned and mentioned.

cc// @asaadmahmood if you have thoughts here?

@cwarnermm cwarnermm added the Docs/Needed Requires documentation label Jul 9, 2024
@raghavaggarwal2308
Copy link
Contributor

raghavaggarwal2308 commented Jul 15, 2024

@esethna

  1. Removed
    image
    image

  2. Here is the screenshot of help command:
    image

  3. Yes, the value is dynamic

  4. If we skip the role in the command or enter invalid value, we will get a message
    image
    image
    image

Copy link

@fmartingr fmartingr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done a quick review mostly because I'm not super experienced with this plugin. Logic seems good, just pointed out a few nitpicks.

server/client.go Show resolved Hide resolved
server/settings.go Outdated Show resolved Hide resolved
server/settings.go Show resolved Hide resolved
@wiggin77 wiggin77 added the 3: QA Review Requires review by a QA tester label Jul 31, 2024
@raghavaggarwal2308 raghavaggarwal2308 added this to the v4.3.0 milestone Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester Docs/Needed Requires documentation
Projects
None yet
6 participants