[GH-798] When the epic link changes for an issue, link to the epic in the resulting post (#798)

[seppewyns]

Summary

Aims to fix #798

Ticket Link

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

[mattermod]

Hello @seppewyns,

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]

Codecov Report

Base: 31.27% // Head: 31.49% // Increases project coverage by +0.21% :tada:

Coverage data is based on head (3cc5c56) compared to base (f778e6f). Patch coverage: 78.04% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #811      +/-   ##
==========================================
+ Coverage   31.27%   31.49%   +0.21%     
==========================================
  Files          49       49              
  Lines        6017     6048      +31     
==========================================
+ Hits         1882     1905      +23     
- Misses       3946     3952       +6     
- Partials      189      191       +2     

Impacted Files	Coverage Δ
server/webhook_worker.go	0.00% <0.00%> (ø)
server/issue.go	32.01% <50.00%> (+0.23%)	:arrow_up:
server/webhook_parser.go	85.03% <87.09%> (-0.20%)	:arrow_down:
server/webhook_http.go	85.96% <100.00%> (ø)

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.

[seppewyns]

@mickmister Thanks for the review, I've fixed the issues.

[mickmister]

Looks like there are some linting errors happening in CI https://app.circleci.com/pipelines/github/mattermost/mattermost-plugin-jira/2582/workflows/d55fd24e-6fe8-4bd0-aa3e-9cc3d8d48d61/jobs/8098:

server/webhook_parser.go:128:20: string `None` has 2 occurrences, make it a constant (goconst)
			toWithDefault = "None"
			                ^
server/webhook_parser.go:124:22: string `~~None~~` has 2 occurrences, make it a constant (goconst)
			fromWithDefault = "~~None~~"

You can run make check-style to see if the linting passes locally

[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

[mickmister]

@seppewyns Sorry this PR has been sitting here for a while. We will get this fully reviewed soon.

I wanted to get your opinion on increasing the scope here a bit. If we also include the epic name in the post this feature would be a lot more useful.

This would require fetching the two epics involved with the post. Then we would have access to both epic names and can fill in the post.

Let me know what you think @seppewyns :+1:

[seppewyns]

@mickmister Adding the epic name would indeed make the post more useful. :+1: I have a couple of questions about implementing this:

• How should this new post be rendered? Should the project key still be displayed or just the epic name?
  eg. Seppe **updated** Epic Link from [MM-1234: Example epic name](url) to [MM-1235: Example name 2](url) on task [MM-1111: Example Task](url) or just Seppe **updated** Epic Link from [Example epic name](url) to [Example name 2](url) on task [MM-1111: Example Task](url)
• Right now when an epic link is removed the resulting post will be: Seppe **updated** Epic Link from [Example epic name](url) to None on task [MM-1111: Example Task](url) Maybe this should be changed so it displays Seppe **removed** Epic Link [Example epic name](url) on task [MM-1111: Example Task](url) (and then **added** when an epic link is added) What are your thoughts on this?
• Does a new issue or pull request need to be opened for these changes or can I push changes to the branch for this pr?

[mickmister]

• How should this new post be rendered? Should the project key still be displayed or just the epic name?

Your first proposal makes sense to me: Seppe **updated** Epic Link from [MM-1234: Example epic name](url) to [MM-1235: Example name 2](url) on task [MM-1111: Example Task](url)

• Right now when an epic link is removed the resulting post will be: Seppe **updated** Epic Link from [Example epic name](url) to None on task [MM-1111: Example Task](url) Maybe this should be changed so it displays Seppe **removed** Epic Link [Example epic name](url) on task [MM-1111: Example Task](url) (and then **added** when an epic link is added) What are your thoughts on this?

I'm hesitant to change this as this would be a change to what people are used to seeing. I think current behavior is clear about what's happening. Saying **added** epic link seems a bit ambiguous as to whether there was one set beforehand. It also sounds like it's a multi-select field, which it is only a single-select field.

@aaronrothschild What are your thoughts on this?

• Does a new issue or pull request need to be opened for these changes or can I push changes to the branch for this pr?

Definitely for the first bullet point, it should go in this PR. The second bullet point can go in here too if it's determined we should do that.

---

@seppewyns Thanks for your contribution and the attention to detail here 🙂 I'm looking forward to seeing this improvement in the product :+1:

[aaronrothschild]

I'm hesitant to change this as this would be a change to what people are used to seeing. I think current behavior is clear about what's happening. Saying **added** epic link seems a bit ambiguous as to whether there was one set beforehand. It also sounds like it's a multi-select field, which it is only a single-select field.

I agree perhaps use the word **changed** instead of **added**

[mickmister]

I agree perhaps use the word **changed** instead of **added**

@aaronrothschild The main question here is specifically for if we are setting the Epic Link and there is currently no value set for the field before the change.

It currently shows as:

Seppe updated Epic Link from ~~None~~ to Example epic name on task MM-1111: Example Task

We're wondering if it should change, and say something like one of the following:

• Seppe updated Epic Link to MM-1234: Example epic name on task MM-1111: Example Task
• Seppe changed Epic Link to MM-1234: Example epic name on task MM-1111: Example Task
• Seppe added Epic Link MM-1234: Example epic name on task MM-1111: Example Task

1/5 I think we should keep the existing behavior. I think it's not clear that the value it wasn't set beforehand in the proposed changes.

[emilyacook]

Thanks for participating in Hacktoberfest! You can claim your sticker set here: https://get.printfection.com/hacktober21/4144583267 @seppewyns

[seppewyns]

@mickmister Sorry for the long wait. I've added some changes so epic names get fetched and displayed. I'd love to hear your feedback.

[mickmister]

Hi @seppewyns, how are you doing?

[DHaussermann]

/update-branch

[hanzei]

Closing as inactive