Skip to content

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)

jira_integration_config jira_integration_extended_labelled

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 of jira_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 of send_message previously, when it was called from create_cross_reference_note)
    • 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')

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 and add_issue_solved_comment type of messages regarding checking the existence of the comment.

Does this MR meet the acceptance criteria?

Conformity

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)

Database checklist

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 🤖

Merge request reports