Fixed plugin failure on Issue with no priority.

[DummyThatMatters]

Summary

Fixes the situation when plugin process the issue with Priority field set to nil, which can potentially lead to situation when nill pointer dereference error occurs.

Ticket Link

Seem to fix #820

[mattermod]

Hello @DummyThatMatters,

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 @DummyThatMatters, thanks for catching this and providing the fix 🎉

Just so I can understand the issue better, do you know if this is occurring in one of these two cases:

• the issue scheme does not contain the priority field
• the issue scheme does have a priority, but the priority is not assigned for a given issue somehow

As far as the fix you've provided, I have one request to provide the same fix to other fields potentially having this issue in the same function. Thanks for this great contribution @DummyThatMatters!

[mickmister]

I think we should apply the same fix to anything that can possibly be nil in this function, such as issue.Fields.Status. These may be the only two to apply the fix for.

Can you please check the values being accessed in this function, and which ones are pointers that could produce the same problem?

[DummyThatMatters]

Hi @DummyThatMatters, thanks for catching this and providing the fix 🎉

Just so I can understand the issue better, do you know if this is occurring in one of these two cases:

* the issue scheme does not contain the `priority` field

* the issue scheme does have a `priority`, but the priority is not assigned for a given issue somehow

As far as the fix you've provided, I have one request to provide the same fix to other fields potentially having this issue in the same function. Thanks for this great contribution @DummyThatMatters!

Frankly, I don't really know. Does this thing matter? We debugged this a long time ago and I have implemented my patch on our prod server, so it no longer fail and I don't know which exact tickets caused the issue.
I can try to find out if this info is essential in order to investigate the bug and fix potential problems. And we do not have any problems with other fields for some reason.

[mickmister]

@DummyThatMatters Fair enough, I'll approve at its current state. Note that you'll need to sign our CLA before getting this merged, as explained here #821 (comment)

Thanks again for this fix 😄

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

[dipak-demansol]

@DummyThatMatters priority is a mandatory field at the time of creating jira issue using the plugin so how you reported the issue with No priority?

[dipak-demansol]

@DummyThatMatters priority is a mandatory field at the time of creating jira issue using the plugin so how you reported the issue with No priority?

@mickmister can you give me some info to regenerate the issue?

[DummyThatMatters]

@DummyThatMatters priority is a mandatory field at the time of creating jira issue using the plugin so how you reported the issue with No priority?

@dipak-demansol hello! You can subscribe on issues that have not been created via plugin. then you'll have some problems with nil priority issues.

[mickmister]

@dipak-demansol I think you'll need to have an issue schema that has a priority field, but no default value set for the field. Not entirely sure how that works though

[dipak-demansol]

@DHaussermann We're trying to find a way to null out priority from web 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!

[hanzei]

/update-branch

[codecov-commenter]

Codecov Report

Base: 30.14% // Head: 30.16% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (b193a35) compared to base (7a5c616).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #821      +/-   ##
==========================================
+ Coverage   30.14%   30.16%   +0.01%     
==========================================
  Files          49       49              
  Lines        7467     7469       +2     
==========================================
+ Hits         2251     2253       +2     
  Misses       5027     5027              
  Partials      189      189              

Impacted Files	Coverage Δ
server/issue.go	30.92% <100.00%> (+0.17%)	⬆️

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

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

[hanzei]

@DHaussermann Can we merge this straight forward fix would QA review?

[hanzei]

/check-cla

[hanzei]

@DummyThatMatters Would you be open on signing the CLA?

[DHaussermann]

@hanzei If this is low risk - we should merge it without testing to resolve the problem.

0/5 There may have simple ways to build the data needed for this in the past. But I believe this filed is automatically set even when removed from all configuration and not visible in the UI. (Jira seems to assign values which it uses to hold the order of tings in lists.)

In any case, I can find no simple way to create the data needed short of manipulating a DB. I suggest we merge this and rely on release testing to catch any regression.

[DHaussermann]

Please disregard previous comment. I had the field and scenario confused with something else.

Tested and passed.
Steps:

• Picked a project
• Created a custom scheme for issues of type Bug within the ptoject
• Removed priority field from the scheme
• Confirmed the Jira webapp could create the issue
• Confirmed the Jira plugin can create the same issue form within Mattermost

This issue is resolved. No regressions found

LGTM!

Thanks for the soulution @DummyThatMatters 🎉