mattermost-plugin-jira
mattermost-plugin-jira copied to clipboard
Issue #558
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
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.
@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! :-)
I made a small video testing the functionality
https://user-images.githubusercontent.com/488556/182040226-e1008081-ec2d-42ab-9aaa-44cc1ec8e63b.mov
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
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.
I'll make all the changes next week! Thank you! :-)
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
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.
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!
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. :-)
/check-cla
I fixed the conflict
Do I need to do something here @hanzei ? I'm not sure if Awaiting submitter action
requires something from me. Thank you!
@javaguirre just remove the label once you finished your task
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. 👍
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!
/update-branch
@DHaussermann Heads up that the PR is ready for your review
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
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.
@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?
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.
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!
Closing the PR and adding https://github.com/mattermost/mattermost-plugin-jira/issues/558 to the @brightscout queue.