[issue 513] After subscribing the event, need to display event name with the success message. #513

[sibasankarnayak]

added a post in channel on successfully subscribing any org/[repo] with event it subscribed with message if it got overwritten.

ticket here Fixes #513 Fixes #512

[mattermod]

Hello @sibasankarnayak,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

[codecov-commenter]

Codecov Report

Merging #517 (d839826) into master (b7368d5) will decrease coverage by 0.27%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #517      +/-   ##
==========================================
- Coverage   15.57%   15.29%   -0.28%     
==========================================
  Files          15       15              
  Lines        4136     4211      +75     
==========================================
  Hits          644      644              
- Misses       3450     3525      +75     
  Partials       42       42              

Impacted Files	Coverage Δ
server/plugin/command.go	7.41% <0.00%> (-1.21%)	:arrow_down:
server/plugin/subscriptions.go	10.46% <0.00%> (+0.35%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b7368d5...d839826. Read the comment docs.

[dipak-demansol]

@sibasankarnayak when i deploy it i'm getting this error this plugin failed to start. Check your system logs for errors.plugin is starting. can you pls share the zip file of this PR?

[sibasankarnayak]

@sibasankarnayak when i deploy it i'm getting this error this plugin failed to start. Check your system logs for errors.plugin is starting. can you pls share the zip file of this PR?

it is not a easy task to share the zip file , it will be good if you start using it in any local to deploy as mentioned using plugin doc

[sibasankarnayak]

@hanzei i have added the fix for issue #512 as per our discussion.

[dipak-demansol]

@hanzei @mickmister when i subscribe to the repo then i'm getting two bot post, i think we should only show 1 bot post to the user who subscribe the repo, we Should show the 1st bot post that visible to the all member in the channel including the member who subscribe the repo and we should remove the "only visible to you" bot post because why we need 2 bot post that give the same info. pls see the screenshot

[hanzei]

Good catch @dipak-demansol :+1:

[dipak-demansol]

@sibasankarnayak pls let me know when its ready for review.

[dipak-demansol]

Non blocking point @hanzei pls see the last 2 subscription in the screenshot. may be we should also show the org/user name so there will be no confusion if someone subscribe same name repo of the different org/user ?

[hanzei]

@dipak-demansol Good suggestion, see https://github.com/mattermost/mattermost-plugin-github/pull/517#discussion_r769377088 for the request

[mattermod]

This PR has been automatically labelled "stale" because it hasn't had recent activity. A core team member will check in on the status of the PR to help with questions. Thank you for your contribution!

/cc @aspleenic

[hanzei]

@mickmister This is ready for your review.

[dipak-demansol]

@hanzei @mickmister pls see the attached image, when i subscribe the repo then i got two post, 1st is visible to all member in the channel and 2nd is only visible to me, in 1st post only "repo" name is visible but in 2nd post "user-name/repo-name" is visible, my point is same information should be added in the 1st post so all channel member can see the "user-name/repo-name".

non-blocking point :- i don't know why we need two post, we can also show One Post that visible to all member, pls let me know your ok with that two post? of visible to all member & only visible to me.

[DHaussermann]

@sibasankarnayak I would agree here that the ephemeral posts for successfully adding subscription are redundant. Only failures and validation would still need ephemeral posts. That said it is minor and I'd be happy to open a separate issue for this.

However, the 1st point made by @dipak-demansol is more impactful. The post should include the owner or organization in the format we always use. Having only the repo name can cause confusion as there are many cases where this could overlap. Is it possible to address this in this PR?

[hanzei]

:+1: for fixing both issues in this PR

[sibasankarnayak]

@sibasankarnayak I would agree here that the ephemeral posts for successfully adding subscription are redundant. Only failures and validation would still need ephemeral posts. That said it is minor and I'd be happy to open a separate issue for this.

However, the 1st point made by @dipak-demansol is more impactful. The post should include the owner or organization in the format we always use. Having only the repo name can cause confusion as there are many cases where this could overlap. Is it possible to address this in this PR?

@DHaussermann sure added org info in message

[hanzei]

@sibasankarnayak Would you please remove the duplicate message as request in https://github.com/mattermost/mattermost-plugin-github/pull/517#issuecomment-1027729108?

[dipak-demansol]

FYI :- Build is Fail After Enable it, internally working on with dev.

[hanzei]

Both of your findings should be fixed as part of this PR. The PR won't go into v2.2.0 anyway.

[sibasankarnayak]

Blocking on #517 (review)

@dipak-demansol can you test now

1. only one notification by bot will be there
2. while unsubscribe there will be a bot notification

please share if you found any issue

[dipak-demansol]

Blocking on #517 (review)

@dipak-demansol can you test now

1. only one notification by bot will be there
2. while unsubscribe there will be a bot notification

please share if you found any issue

As we know the issue, build is failed because its created from my linux machine, later i will create build from mac machine then test it.

[dipak-demansol]

@sibasankarnayak pls do the text changes as per the below example.

when user Subscribe repo @someone added a subscription to repository "ORG/repo-name" with events A, B, C

When user remove repo @someone delete a subscription to repository "ORG/repo name"

When user overwrite events of same repo @someone added a subscription to repository "ORG/repo-name" with events A The previous subscription with: pulls, issues, creates, deletes was overwritten.

pls let me know if you have any Question.

[sibasankarnayak]

@sibasankarnayak pls do the text changes as per the below example.

when user Subscribe repo @someone added a subscription to repository "ORG/repo-name" with events A, B, C

When user remove repo @someone delete a subscription to repository "ORG/repo name"

When user overwrite events of same repo @someone added a subscription to repository "ORG/repo-name" with events A The previous subscription with: pulls, issues, creates, deletes was overwritten.

pls let me know if you have any Question.

@mickmister @hanzei need some suggestion for the message to better the ux message shown on subscribe and unsubscribe

current messages are like this Subscribe subscription made first time : A subscription to repository sibasankarnayak/mattermost-plugin-github, was added to this channel by @sibasankarnayak with events: issues,creates A subscription to organization sibasankarnayak, was added to this channel by @sibasankarnayak with events: issues,creates

unsubscribe notification

Unsubscribed from organization sibasankarnayak on this channel was done by @sibasankarnayak Unsubscribed from repositories sibasankarnayak/mattermost-plugin-github on this channel was done by @sibasankarnayak

please suggest some better notification messages.

[sibasankarnayak]

@sibasankarnayak pls do the text changes as per the below example. when user Subscribe repo @Someone added a subscription to repository "ORG/repo-name" with events A, B, C When user remove repo @Someone delete a subscription to repository "ORG/repo name" When user overwrite events of same repo @Someone added a subscription to repository "ORG/repo-name" with events A The previous subscription with: pulls, issues, creates, deletes was overwritten. pls let me know if you have any Question.

@mickmister @hanzei need some suggestion for the message to better the ux message shown on subscribe and unsubscribe

current messages are like this Subscribe subscription made first time : A subscription to repository sibasankarnayak/mattermost-plugin-github, was added to this channel by @sibasankarnayak with events: issues,creates A subscription to organization sibasankarnayak, was added to this channel by @sibasankarnayak with events: issues,creates

unsubscribe notification

Unsubscribed from organization sibasankarnayak on this channel was done by @sibasankarnayak Unsubscribed from repositories sibasankarnayak/mattermost-plugin-github on this channel was done by @sibasankarnayak

please suggest some better notification messages.

@aaronrothschild @cwarnermm can you share your input on same to make the text notification more meaningful.

[cwarnermm]

Hi @sibasankarnayak! Thanks for reaching out for guidance on developing helpful product guidance for customers. For this set of use cases, here are the UI strings I would recommend:

Subscribe: [name] subscribed this channel to [reponame] with the following events: [x], [y]. [name] subscribed this channel to [orgname] with the following events [x], [y].

Unsubscribe: [name] unsubscribed this channel from [orgname]. [name] unsubscribed this channel from [reponame].

[sibasankarnayak]

Hi @sibasankarnayak! Thanks for reaching out for guidance on developing helpful product guidance for customers. For this set of use cases, here are the UI strings I would recommend:

Subscribe: [name] subscribed this channel to [reponame] with the following events: [x], [y]. [name] subscribed this channel to [orgname] with the following events [x], [y].

Unsubscribe: [name] unsubscribed this channel from [orgname]. [name] unsubscribed this channel from [reponame].

@cwarnermm Thank you for suggestion

@dipak-demansol can you check have made the changes as per suggestion

[dipak-demansol]

@sibasankarnayak when user delete the repo, after the ORG name pls show the repo name same as when your showing it at the time of subscription.

it should be like this @dipak-app-bar-disable unsubscribed this channel from dipak-demansol/hello

[sibasankarnayak]

1/5 that specific format parameters should be used if possible. E.g. %s is preferred over %v.

@hanzei tried with %s perviously working as expected but not it was looking like this so used %v instead of %v

[hanzei]

%!(EXTRA is printed when where are more arguments when parameters. Are you sure the verb caused the issue you are seeing?

[sibasankarnayak]

%!(EXTRA is printed when where are more arguments when parameters. Are you sure the verb caused the issue you are seeing?

not sure but it works now with %s

can you check now