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-604] - ability to create a subscription template #783

Closed
wants to merge 8 commits into from

Conversation

maisnamrajusingh
Copy link
Contributor

Summary

A UI to create a subscription template. Please check the video below for details.
https://www.loom.com/share/f988879135c54d789f4bd015b207e9d8

Ticket Link

#604

@mattermod
Copy link
Contributor

Hello @maisnamrajusingh,

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
Copy link

codecov-commenter commented Jul 29, 2021

Codecov Report

Merging #783 (9da657f) into master (514e5c2) will decrease coverage by 0.66%.
The diff coverage is 0.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #783      +/-   ##
==========================================
- Coverage   31.42%   30.76%   -0.67%     
==========================================
  Files          49       49              
  Lines        5982     6104     +122     
==========================================
- Hits         1880     1878       -2     
- Misses       3913     4036     +123     
- Partials      189      190       +1     
Impacted Files Coverage Δ
server/subscribe.go 55.09% <0.00%> (-12.53%) ⬇️
server/http.go 42.30% <33.33%> (-1.85%) ⬇️

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 514e5c2...9da657f. Read the comment docs.

@mickmister mickmister added 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 labels Aug 5, 2021
@mickmister mickmister linked an issue Aug 5, 2021 that may be closed by this pull request
@mickmister mickmister added the 1: UX Review Requires review by a UX Designer label Aug 5, 2021
@mickmister
Copy link
Contributor

@aaronrothschild @matthewbirtch Please see the video attached in the PR description. Currently it looks like the feature works this way:

  • Opening the subscription modal shows the current channel's subscriptions and existing subscription templates
  • The templates can be deleted from here
  • When editing or creating a subscription, the form gives you the option to start from an existing template. When you select a template, it pre-populates the form with the template's values.
  • Selecting "Save as Template" causes the current form state to be saved as a template.
    • @maisnamrajusingh What mechanisms are in place for overwriting a template, or saving a subscription with the same name as a template?
    • We should make sure the current user can access the template's Jira project on the server when processing the API request to save the template

@maisnamrajusingh Are the subscriptions scoped to a given channel or team? If possible, we should filter on Jira projects the current user has access to.

@matthewbirtch
Copy link

@aaronrothschild @matthewbirtch Please see the video attached in the PR description

@mickmister do you know if any design work was done for this feature? I don't see anything done in our Figma files yet and I don't see a Figma link in the description here. I think the design team might need to take a pass at this to clean up the design/layout a bit.

@mickmister
Copy link
Contributor

@matthewbirtch I don't think there's been any design for the feature, which was an oversight on creating the help wanted ticket. I'll discuss with the team how we can make sure design decisions are taken care of before opening the ticket for development.

What would be next steps to get this feature under design review?

@andrewbrown00
Copy link

What would be next steps to get this feature under design review?

Hey, @mickmister, it would be helpful to know the use cases and requirements for this feature.

cc @aaronrothschild

@aaronrothschild
Copy link
Contributor

I'll keep these use cases on this ticket:

  • As a user, I want to create subscriptions to events in Jira so I can be notified of tickets I need to review/comment/approve/etc. directly into a Mattermost channel that is relevant, such as Project A. (we have this functionality for admins)
  • As an admin user I need to create subscriptions on behalf of users because they don't have permissions to create their own subscriptions, however - this becomes very repetitive when creating subscriptions for multiple users or channels because they need to re-enter the same values for each subscription - with a small variance.
    • example: I need to subscribe to all bugs/stories/epics for project XYZ, but exclude all issues marked as "secure". I need to repeat this subscription for 10 new projects and channels.

Requirements:

  • Only users permitted to create subscriptions in a channel can use this functionality

  • Allow only Admin users to create a template that can be re-used when making the next subscription

    cc @matthewbirtch @mickmister

@mickmister
Copy link
Contributor

@andrewbrown00 Our internal use case is that some of our Jira tickets are internal, and so we need to make sure all of the subscriptions going to public channels have these tickets filtered out. With this ticket implemented, we can have a baseline for a "safe" subscription to be reused among several users. This avoids user error that may expose sensitive information. We can then open up the feature to more users, since community currently has the feature locked to system admins.

The subscription template is identical to an actual subscription, except it doesn't actually process any webhook events, and it's not associated with any specific channel. It does need to adhere to every rule of subscriptions, meaning it needs to be scoped to a specific Jira project, and the user configuring the subscription needs to have access to that project in Jira.

@matthewbirtch
Copy link

@mickmister @aaronrothschild @andrewbrown00

  • Is there a way to review the UI/UX here without just the video example? The video is tough to follow in parts and the quality is too low to view the UI in detail. It would be better to be able to play with this in real life to do a UX review here.
  • Maybe we can start up a spinwick server and configure it to be tested by UX?

From my initial view, here are some thoughts. Happy to have more of a discussion here.

Overall

  • The idea of templates seems really useful for sure
  • From the video, I couldn't tell how the modal was opened. Is this opened through a slash command?
  • We have been moving away from the full-screen modals. Would it be possible to change this? We find users lose their place and context when the modal completely obscures the rest of the application
  • There are other stylistic details with the form components and buttons that I could bring up, but I don't think the design system components are ready to be used from the UI Platform team yet
  • I'm not overly familiar with what is new here vs. what is already in the Jira Plugin so bear with me in the feedback here.
  • If I have some time, I can try to mock something up that might communicate the feedback.

Add Jira Subscription Screen

  • I wonder if some more instruction may be needed to help guide users.
  • I would expect the Subscription Name to be the first input.
  • Can we change Use Template to Choose a Template
    • Should No Template or Blank be an option here?
  • It would appear that the Save as Template button is not legible with the style that is being used here.
  • I think we should actually remove the Delete and Save as Template from this screen. These actions can exist in the list screen
  • I'm curious how the Add Filter function works. I didn't see that interaction in the video

Subscriptions and Templates List Screen

  • I'm wondering if we can somehow treat the two sections visually differently so they aren't confused with each other. Like, maybe the templates don't need a table view - maybe they could display as cards or something. I'm just spitballing, but it feels like they need to be differentiated more.
  • Can we significantly increase the space between these two sections so they are more separated (rather than using another divider line)?
  • Should we have a 'Create Template` button proximal to the templates section? It feels odd to have to create a template by choosing 'Create Subscription'

Anyway, that's my first pass. As mentioned, I can try to devote some time to mocking this up (or finding another member of the team to do so) if it would be helpful

@mickmister
Copy link
Contributor

@matthewbirtch @andrewbrown00 I have two cloud servers set up for testing this. The first link is running the current release of the Jira plugin, and the second has this PR deployed. Access the feature by running /jira subscribe while signed into the usual "sysadmin" user

https://jira-plugin.test.mattermost.cloud
https://jira-plugin-pr-783.test.mattermost.cloud

@mickmister
Copy link
Contributor

@matthewbirtch Thanks for all the detailed feedback! The channel subscriptions feature was developed a while ago and hasn't been revisited for some time. Reading through your comment, I agree that the overall flow of the feature can be improved, even outside of this PR's scope. The subscription template feature may need to put on hold if the redesign/reimplementation of the subscriptions feature needs to occur first, since that should be a separate PR from introducing the templates depending on how big the changes are.

@mattermod
Copy link
Contributor

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 @jasonblais @jfrerich @emilyacook

@mickmister
Copy link
Contributor

@matthewbirtch Would you like to have a video call to discuss this feature? If you'd like to test the feature yourself, please see the spinwick URLs posted here #783 (comment)

@matthewbirtch
Copy link

@mickmister I think that would likely be a good idea. Once we do that, we should probably this up for design to work through before developing further. Can we set something up with you, me @andrewbrown00 and @aaronrothschild?

@maisnamrajusingh
Copy link
Contributor Author

@mickmister @matthewbirtch how did it go ?

@aaronrothschild
Copy link
Contributor

aaronrothschild commented Sep 23, 2021

@andrewbrown00 @matthewbirtch @aaronrothschild

One thing that was not discussed is the access control of these templates. Depending on the structure of an organization, this may be a complicated equation. If we want to support the same levels of access control that we do elsewhere in the plugin, we will need to perform some heavy operations whenever someone chooses to browse the templates. Because of this, we should make the user go out of their way to see the list of available templates. i.e. It should be clear that the user wants to create a subscription from a template before we calculate which templates they have access to.

In fact, it would be helpful if the user is able to pick the template by project first. A subscription and subscription template must be tied to a specific Jira project, since all the facets of a subscription are tied to that project. If we allowed the user to first browse the subscription templates by project, that would be very useful. Jira projects that the user is not a member of would not show up in this list.

@mickmister Can you expand on the use case for security of the templates? I think it would be fine for us to start with no access control for particular templates and perhaps add it later if requested/needed. I understand that we'd want to follow your advice for now perhaps and show subscriptions based on Project to make it easier/possible in the future to have access control.

@mickmister
Copy link
Contributor

Can you expand on the use case for security of the templates? I think it would be fine for us to start with no access control for particular templates and perhaps add it later if requested/needed.

@aaronrothschild I think leaving out access control for browsing the templates is fine, though we should still require the user to be a member of the project before editing or creating a template for a project they don't have access to. There is already similar code for access control around subscriptions so this should be straightforward.

@hanzei hanzei added the Awaiting Submitter Action Blocked on the author label Jan 24, 2022
@hanzei hanzei added this to the v3.2.0 milestone Jan 24, 2022
@hanzei
Copy link
Collaborator

hanzei commented Mar 1, 2022

@maisnamrajusingh Is this PR review for review?

@hanzei hanzei modified the milestones: v3.2.0, v3.3.0 Mar 1, 2022
@maisnamrajusingh
Copy link
Contributor Author

@maisnamrajusingh Is this PR review for review?

@hanzei no not yet. Currently prioritizing the jira ones.

@mickmister mickmister removed their request for review April 25, 2022 04:49
@dipak-demansol dipak-demansol removed the 3: QA Review Requires review by a QA tester label May 6, 2022
@hanzei hanzei added Triage Work In Progress Not yet ready for review and removed 1: UX Review Requires review by a UX Designer 2: Dev Review Requires review by a core committer labels Nov 29, 2022
@mickmister
Copy link
Contributor

Closing in favor of #897

@mickmister mickmister closed this Feb 1, 2023
@mattermost-build mattermost-build removed Awaiting Submitter Action Blocked on the author Work In Progress Not yet ready for review labels Feb 1, 2023
@mickmister mickmister removed the Triage label Feb 1, 2023
@hanzei hanzei added Lifecycle/3:orphaned and removed 1: PM Review Requires review by a product manager Lifecycle/1:stale labels Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature to create a subscription template