[GH-855] Save user's last used field values #969

raghavaggarwal2308 · 2023-09-12T14:51:57Z

Summary

Updated logic to store the last used issue type value while creating the issue and prefill the modal with those values the next time the user creates an issue.

What should be tested?

The specified fields are getting pre-filled properly.
A new issue is getting created successfully.
The prefilled value does not exist on Jira anymore.

Ticket Link

Fixes #855

…values (#67) * [MI-3466] Fix Jira issue mattermost#955: Save user's last used field values * [MI-3466] Code refactoring * [MI-3466] Fixed issue: not able to select prject while creating subscriptions * [MI-3466] Review fixes

mickmister · 2023-10-13T05:18:36Z

server/user.go

@@ -33,8 +33,14 @@ type Connection struct {
 	Oauth1AccessSecret string        `json:",omitempty"`
 	OAuth2Token        *oauth2.Token `json:",omitempty"`
 	Settings           *ConnectionSettings
-	DefaultProjectKey  string   `json:"default_project_key,omitempty"`
-	MattermostUserID   types.ID `json:"mattermost_user_id"`
+	SavedFieldValues   *SavedFieldValues `json:"saved_field_values,omitempty"`


We are technically losing data here by changing its shape, but the "old" value of DefaultProjectKey is not too important. Just something to think about when changing persistent data structures

mickmister · 2023-10-13T05:21:45Z

webapp/src/components/modals/create_issue/create_issue_form.tsx

+            };
+        }
+
+        fields = {...fields};


Does this need to be here?

@mickmister Fixed

mickmister · 2023-10-13T05:29:56Z

So the one field we save is the "component" type. I'm thinking the point of the feature is to fill in more fields than just the components. From the discussion at #855, we didn't land on a definitive requirement to fulfill here. @matthewbirtch Any thoughts on how exactly this feature should work?

Personally I think we should save everything that's not the summary nor a textarea (description or custom textarea), which seems to be what Jira does with their "Create another issue" checkbox feature

mickmister · 2023-10-13T05:31:46Z

One thing I suggested in that discussion is having a "Clear All" button that allows you to start from scratch if the filled out fields don't apply to the second ticket you're creating

#855 (comment)

matthewbirtch · 2023-10-13T16:23:02Z

So the one field we save is the "component" type. I'm thinking the point of the feature is to fill in more fields than just the components. From the discussion at #855, we didn't land on a definitive requirement to fulfill here. @matthewbirtch Any thoughts on how exactly this feature should work?

Personally I think we should save everything that's not the summary nor a textarea (description or custom textarea), which seems to be what Jira does with their "Create another issue" checkbox feature

I'm not sure we should prefill that much. I think we should prefill only the following based on the previous selection:

Project
Issue Type
Components
Team

I don't imagine Fix Version, Epic, Assignee, Labels, etc would be repeated as much from one ticket to the next

mickmister · 2023-10-13T17:33:42Z

@matthewbirtch "Team" is not a built-in Jira field. Instead, it's a custom field we have on our Jira instance. Unfortunately we don't have a way to give precedence of that field over other custom fields, unless we make it so this can be explicitly configured somewhere. The plugin already supports the last selected "project", and this feature is currently adding functionality for "issue type" and "components".

I personally don't use components that often (though I know they are commonly used), and I think auto-selecting "issue type" wouldn't save me much time when creating an issue. This feature request came up for use during a bug bash, where many of the fields would be the same for each ticket. That's where I see it being most useful, and saving the most clicks in the form.

matthewbirtch · 2023-10-13T17:41:35Z

@matthewbirtch "Team" is not a built-in Jira field. Instead, it's a custom field we have on our Jira instance. Unfortunately we don't have a way to give precedence of that field over other custom fields, unless we make it so this can be explicitly configured somewhere. The plugin already supports the last selected "project", and this feature is currently adding functionality for "issue type" and "components".

I personally don't use components that often (though I know they are commonly used), and I think auto-selecting "issue type" wouldn't save me much time when creating an issue. This feature request came up for use during a bug bash, where many of the fields would be the same for each ticket. That's where I see it being most useful, and saving the most clicks in the form.

@mickmister I'm not sure the bug bash use case is ubiquitous enough to save all fields from previous issue created with the plugin. Maybe we could assume you want that if you're creating issues in quick succession, but if I create an issue today and then create the next issue tomorrow, I would find it confusing why all the fields were prefilled and I would likely have to clear most of them which actually makes it take longer.

mickmister · 2023-10-13T18:44:31Z

I would likely have to clear most of them which actually makes it take longer.

@matthewbirtch This is where the "Clear all" button would be useful, though it's really just a bandaid on the issue

Thinking about this some more, I think the pre-populating of form fields should be a bit more explicit. Any "implicit" approach (pre-filling Component without explicitly telling the user this is happening) can cause issues where the value was unknowingly and accidentally assigned a value, when it should have been blank.

We could have a button for the user to click when they open the modal that says "prefill fields with values from my last submission". This way the user has full control over the automation happening, and can decide on the spot what they want to happen in the form.

matthewbirtch · 2023-10-13T20:35:32Z

I would likely have to clear most of them which actually makes it take longer.

@matthewbirtch This is where the "Clear all" button would be useful, though it's really just a bandaid on the issue

Thinking about this some more, I think the pre-populating of form fields should be a bit more explicit. Any "implicit" approach (pre-filling Component without explicitly telling the user this is happening) can cause issues where the value was unknowingly and accidentally assigned a value, when it should have been blank.

We could have a button for the user to click when they open the modal that says "prefill fields with values from my last submission". This way the user has full control over the automation happening, and can decide on the spot what they want to happen in the form.

@mickmister I'm hesitant to add either of the 'clear all' or the 'prefill fields' buttons since they are not common, expected elements in a form. Reset buttons are not recommended as a UX best practice. I'd be a bigger proponent of prefilling a few key fields as sensible defaults and just leaving the rest out.

mickmister · 2023-10-27T21:32:28Z

I'd be a bigger proponent of prefilling a few key fields as sensible defaults and just leaving the rest out.

@matthewbirtch I'm thinking the functionality of "autofill Component" could provide more trouble than good. Making the "issue type" autofilled does make sense as it's an obvious step in the form that's being filled out, but the Component field is more implicit and potentially hard to notice that was autofilled, resulting in potentially incorrect data being submitted.

I'm thinking we should decrease the scope of this task to only include issue type, and leave the Component out of the changes. What do you think?

matthewbirtch · 2023-10-28T02:29:27Z

I'm thinking we should decrease the scope of this task to only include issue type, and leave the Component out of the changes. What do you think?

I'm good with that for now @mickmister

codecov-commenter · 2023-10-31T14:51:11Z

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (97184a5) 29.65% compared to head (30105b9) 29.68%.
Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #969      +/-   ##
==========================================
+ Coverage   29.65%   29.68%   +0.02%     
==========================================
  Files          52       52              
  Lines        7792     7805      +13     
==========================================
+ Hits         2311     2317       +6     
- Misses       5285     5292       +7     
  Partials      196      196

Files	Coverage Δ
server/subscribe.go	`65.46% <100.00%> (-0.09%)`	⬇️
server/command.go	`15.32% <0.00%> (ø)`
server/user.go	`26.70% <33.33%> (ø)`
server/issue.go	`30.33% <16.66%> (-0.12%)`	⬇️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

raghavaggarwal2308 · 2023-10-31T14:51:17Z

@mickmister @matthewbirtch Removed the logic to store the component field.

AayushChaudhary0001 · 2023-12-19T12:36:09Z

Tested and working fine for the following conditions:

Issue type field prefilled correctly.
New issue is being created.
Issue type field is empty if the desired value (issue type) does not exist on Jira.

Tested for both Jira Cloud and Jira Server. Approved, LGTM.

raghavaggarwal2308 · 2024-02-26T12:06:15Z

@mickmister Gentle reminder for re-review

raghavaggarwal2308 · 2024-03-04T14:29:09Z

@mickmister Gentle reminder for re-review

hanzei · 2024-03-15T11:36:27Z

@raghavaggarwal2308 Heads up that the PR broke master: https://github.com/mattermost/mattermost-plugin-jira/actions/runs/8276109706/job/22644115255

raghavaggarwal2308 requested a review from mickmister as a code owner September 12, 2023 14:51

mickmister approved these changes Oct 13, 2023

View reviewed changes

Review fix

19892f9

mickmister added the 3: QA Review Requires review by a QA tester label Oct 30, 2023

mickmister requested a review from AayushChaudhary0001 October 30, 2023 13:31

[MI-3690] Removed the logic to save the component field

30105b9

AayushChaudhary0001 approved these changes Dec 19, 2023

View reviewed changes

mickmister self-requested a review December 21, 2023 21:10

mickmister approved these changes Mar 14, 2024

View reviewed changes

mickmister added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Mar 14, 2024

mickmister merged commit b5f0696 into mattermost:master Mar 14, 2024
9 checks passed

raghavaggarwal2308 deleted the fix_issue_855 branch March 14, 2024 06:02

raghavaggarwal2308 mentioned this pull request Mar 18, 2024

[MM-302] Fix CI error #1035

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GH-855] Save user's last used field values #969

[GH-855] Save user's last used field values #969

raghavaggarwal2308 commented Sep 12, 2023 •

edited

Loading

mickmister Oct 13, 2023 •

edited

Loading

mickmister Oct 13, 2023

raghavaggarwal2308 Oct 26, 2023

mickmister commented Oct 13, 2023 •

edited

Loading

mickmister commented Oct 13, 2023

matthewbirtch commented Oct 13, 2023

mickmister commented Oct 13, 2023

matthewbirtch commented Oct 13, 2023 •

edited

Loading

mickmister commented Oct 13, 2023

matthewbirtch commented Oct 13, 2023

mickmister commented Oct 27, 2023

matthewbirtch commented Oct 28, 2023

codecov-commenter commented Oct 31, 2023 •

edited

Loading

raghavaggarwal2308 commented Oct 31, 2023

AayushChaudhary0001 commented Dec 19, 2023

raghavaggarwal2308 commented Feb 26, 2024

raghavaggarwal2308 commented Mar 4, 2024

hanzei commented Mar 15, 2024

[GH-855] Save user's last used field values #969

[GH-855] Save user's last used field values #969

Conversation

raghavaggarwal2308 commented Sep 12, 2023 • edited Loading

Summary

What should be tested?

Ticket Link

mickmister Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

mickmister Oct 13, 2023

Choose a reason for hiding this comment

raghavaggarwal2308 Oct 26, 2023

Choose a reason for hiding this comment

mickmister commented Oct 13, 2023 • edited Loading

mickmister commented Oct 13, 2023

matthewbirtch commented Oct 13, 2023

mickmister commented Oct 13, 2023

matthewbirtch commented Oct 13, 2023 • edited Loading

mickmister commented Oct 13, 2023

matthewbirtch commented Oct 13, 2023

mickmister commented Oct 27, 2023

matthewbirtch commented Oct 28, 2023

codecov-commenter commented Oct 31, 2023 • edited Loading

Codecov Report

raghavaggarwal2308 commented Oct 31, 2023

AayushChaudhary0001 commented Dec 19, 2023

raghavaggarwal2308 commented Feb 26, 2024

raghavaggarwal2308 commented Mar 4, 2024

hanzei commented Mar 15, 2024

raghavaggarwal2308 commented Sep 12, 2023 •

edited

Loading

mickmister Oct 13, 2023 •

edited

Loading

mickmister commented Oct 13, 2023 •

edited

Loading

matthewbirtch commented Oct 13, 2023 •

edited

Loading

codecov-commenter commented Oct 31, 2023 •

edited

Loading