WIP: Choose message type in Jira Integration
What does this MR do?
Functionally
Add the option for the users to choose between having Jira comments, Jira links or both when using Jira integration.
This would solve the problem of being unable to turn off comments or links that was expressed in these issues: #25542 (closed) #40801 (moved) #51529 (moved)
Technically
Changes introduced:
-
Database: new column
jira_message_type
added:
== 20190812130627 AddMessageTypeColumnToJiraTrackerData: migrating ============
-- add_column(:jira_tracker_data, :jira_message_type, :string)
-> 0.0080s
== 20190812130627 AddMessageTypeColumnToJiraTrackerData: migrated (0.0080s) ===
-
Jira service (
jira_service.rb
) changed:-
send_message
method: it is updated to switch on the cases ofjira_message_type
:- case
'comment'
verifies that the comment doesn't exist yet and posts the comment - case
'link'
verifies that the link doesn't exist yet and posts the link - in any other case (
'all'
): verifies that the comment doesn't exist yet, then verifies if a remote link exists. if the link exists, it is updated (a comment is not created in this case), if not, then a comment and a remote link is created (this was the behavior ofsend_message
previously, when it was called fromcreate_cross_reference_note
)
- case
- message sending logic:
- previously, if message sending was done through
add_issue_solved_comment
, there was no check to see if a comment already exists. It was only checking if a link existed and posting a comment depended on whether the link exists or not. This is not true anymore, message sending always checks if a comment already exists (if the message type is 'comment' or 'all')
- previously, if message sending was done through
-
Possible risks:
- the logic change in Jira Service can impose some risk that I didn't think of
- code quality issues: due to my lack of experience with ruby on rails and rspec
Test coverage:
- new tests for System Note Service were added to cover the new logic of message sending
- tests of Branch Push Service were made more specific to cover both comment and link
- there is no test yet to cover the difference between the
create_cross_reference_note
andadd_issue_solved_comment
type of messages regarding checking the existence of the comment.
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry for user-facing changes, or community contribution. Check the link for other scenarios. -
Documentation created/updated or follow-up review issue created -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Performance and Testing
This MR contains
- a new database column in an existing table
- a new dropdown form field on an existing form
- some changed conditions that affect the number of outgoing API calls (can reduce it in some cases)
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers
Database checklist
-
Conforms to the database guides
When adding migrations:
-
Updated db/schema.rb
-
Added a down
method so the migration can be reverted -
Added the output of the migration(s) to the MR body -
Added tests for the migration in spec/migrations
if necessary (e.g. when migrating data)
Edited by 🤖 GitLab Bot 🤖