mattermost-plugin-jira icon indicating copy to clipboard operation
mattermost-plugin-jira copied to clipboard

Issue #558

Open javaguirre opened this issue 2 years ago • 15 comments

Summary

Let's you use the JiraUser connected of the user assigned (via mention) instead of looking for the text itself.

Ticket Link

Fixes https://github.com/mattermost/mattermost-plugin-jira/issues/558

javaguirre avatar Jul 31 '22 16:07 javaguirre

Hello @javaguirre,

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.

Per the Mattermost Contribution Guide, we need to add you to the list of approved contributors for the Mattermost project.

Please help complete the Mattermost contribution license agreement? Once you have signed the CLA, please comment with /check-cla and confirm that the CLA check is green.

This is a standard procedure for many open source projects.

Please let us know if you have any questions.

We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team.

mattermod avatar Jul 31 '22 16:07 mattermod

@mickmister Hi! Let me know if this is what was expected! Also if you have any opinions or see any code smells let me know, It's been around 8 years since I don't do any Go code!

Thank you! :-)

javaguirre avatar Jul 31 '22 18:07 javaguirre

I made a small video testing the functionality

https://user-images.githubusercontent.com/488556/182040226-e1008081-ec2d-42ab-9aaa-44cc1ec8e63b.mov

javaguirre avatar Jul 31 '22 18:07 javaguirre

I'm thinking if the user provided a valid Mattermost user mention, but the mentioned user is not connected to Jira, we should return an error to the user rather than falling back to searching in Jira for this user

mickmister avatar Aug 02 '22 08:08 mickmister

Codecov Report

Base: 31.27% // Head: 31.27% // No change to project coverage :thumbsup:

Coverage data is based on head (f778e6f) compared to base (f778e6f). Patch has no changes to coverable lines.

:exclamation: Current head f778e6f differs from pull request most recent head 782dcd1. Consider uploading reports for the commit 782dcd1 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #863   +/-   ##
=======================================
  Coverage   31.27%   31.27%           
=======================================
  Files          49       49           
  Lines        6017     6017           
=======================================
  Hits         1882     1882           
  Misses       3946     3946           
  Partials      189      189           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov-commenter avatar Aug 02 '22 09:08 codecov-commenter

I'll make all the changes next week! Thank you! :-)

javaguirre avatar Aug 08 '22 18:08 javaguirre

I'm thinking if the user provided a valid Mattermost user mention, but the mentioned user is not connected to Jira, we should return an error to the user rather than falling back to searching in Jira for this user

Wouldn't that break how it worked before? I don't mind making that change, but I didn't want to break anything or make it backward incompatible @mickmister

javaguirre avatar Aug 17 '22 14:08 javaguirre

I'm thinking if the user provided a valid Mattermost user mention, but the mentioned user is not connected to Jira, we should return an error to the user rather than falling back to searching in Jira for this user

Wouldn't that break how it worked before? I don't mind making that change, but I didn't want to break anything or make it backward incompatible @mickmister

Technically yes, but we are changing how the feature works here. If you include at @ sign, it should be assumed that you're wanting to assign to a MM user. At the moment, this feature really isn't used at all because of it's unpredictability. Adding a guard like this will increase the reliability of the feature.

mickmister avatar Aug 18 '22 17:08 mickmister

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!

mattermod avatar Aug 29 '22 01:08 mattermod

I've handled when the user mentioned is not found nor connected to Jira, I'll finish the executeAssign tests and I think It would be good to go if you're ok with it. :-)

javaguirre avatar Sep 15 '22 15:09 javaguirre

/check-cla

mickmister avatar Sep 19 '22 06:09 mickmister

I fixed the conflict

javaguirre avatar Oct 04 '22 09:10 javaguirre

Do I need to do something here @hanzei ? I'm not sure if Awaiting submitter action requires something from me. Thank you!

javaguirre avatar Oct 13 '22 09:10 javaguirre

@javaguirre just remove the label once you finished your task

levb avatar Oct 13 '22 15:10 levb

The thing is it was finished but was added after, that's why I was confused, maybe it was due to a conflict that arose after. 👍

javaguirre avatar Oct 13 '22 15:10 javaguirre

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!

mattermost-build avatar Feb 09 '23 01:02 mattermost-build

/update-branch

hanzei avatar Feb 22 '23 07:02 hanzei

@DHaussermann Heads up that the PR is ready for your review

hanzei avatar Feb 22 '23 07:02 hanzei

Tested this and the feature is very neat!

  • It works to mention channel members for the issue assignment
  • When mentioning user who are not mapped to Jira, there is clear feedback image

However there is another point to consider here. This PR does not seem to preserve the existing functionality where the string you pass in was used in a "begins with" type search of users in Jira. It would then assign the user if the string was specific enough to return only one match OR show a list of all potential matches if the string had multiple. As this no longer seems to occur - I see no way of assigning to a Jira user who is not available to @ mention in Mattermost. image

@hanzei or @m1lt0n maybe you have thoughts on next steps based on this? Perhaps when passing in a user and the value does not start with @ it should work the existing way with Jira user assignment?

DHaussermann avatar Feb 28 '23 04:02 DHaussermann

Perhaps when passing in a user and the value does not start with @ it should work the existing way with Jira user assignment?

That's a good solution. Let me do that change.

hanzei avatar Feb 28 '23 10:02 hanzei

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!

mattermost-build avatar Mar 11 '23 01:03 mattermost-build

Closing the PR and adding https://github.com/mattermost/mattermost-plugin-jira/issues/558 to the @brightscout queue.

hanzei avatar Mar 30 '23 12:03 hanzei