Skip to content

Update system note text for relating blocking issues & epics

Stanislav Dobrovolschii requested to merge 365048-system-note-text-for-relating-issues-is-misleading into master

What does this MR do and why?

MR fixes the misleading system notes that are getting created after an issue/epic got connected using block/blocked_by link to another issue/epic. Current notes (example: marked this XXX as related to YYY) are getting extended by providing more details on the exact type of a link that is getting created (XXX blocks YYY/YYY blocked by XXX).

Notes message summary

Issuable Action Link type Message before MR Message after MR Changed
Issue Add blocks marked this issue as related to #XXX marked this issue as blocking #XXX ✅
blocked_by marked this issue as related to #XXX marked this issue as blocked by #XXX ✅
Remove blocks removed the relation with #XXX removed the relation with #XXX 🚫
blocked_by removed the relation with #XXX removed the relation with #XXX 🚫
Epic Add blocks marked this epic as related to &XXX marked this epic as blocking &XXX ✅
blocked_by marked this epic as related to &XXX marked this epic as blocked by &XXX ✅
Remove blocks removed the relation with &XXX removed the relation with &XXX 🚫
blocked_by removed the relation with &XXX removed the relation with &XXX 🚫

(Regarding the table structure, I was expecting that empty cells get merged with non-empty ones above them, for colspan-like effect ¯_(ツ)_/¯)

Notes

  • Data migration
  • Scope of this MR includes features that are part of the EE distribution
  • In the beginning I was considering another solution
    • Going that way would result in less code written and less new methods (no need in EE::SystemNotes::IssuablesService.block*_issuable)
    • But after starting to go into that direction I got a feeling that it will be safer to go with the current one
    • Here I tried to summarize current implementation (comparing it with the first version) :
      • Pros
        • Extends flow (instead of modifying the behaviour of the existing component SystemNotes::IssuablesService.relate_issuable)
        • Doesn't complicate the notes message building logic
          • Technically blocks/blocked_by are subtypes of the same relation as relates_to, though the order plays an important role there (compared to relates_to). So they can't be treated the same way as with relates_to, where #{issuable} relates to #{noteable} is the same in both directions.
      • Cons
        • Similar code in 2 places (EE::SystemNotes::IssuablesService.block*_issuable)
        • More LOC

Screenshots or screen recordings

Issues
Type of relation Before After
blocks Screenshot_2022-08-23_at_16.06.36 Screenshot_2022-08-23_at_14.43.04_1661258593942_0_blocker The text is outdated here, you can find new one in the notes message summary table ☝
blocked_by Screenshot_2022-08-23_at_16.07.10_1661263682785_0 Screenshot_2022-08-23_at_14.43.04_1661258593942_0_blocked_by
Epics
Type of relation Before After
blocks Screenshot_2022-08-23_at_16.02.31_1661263417824_0 Screenshot_2022-08-23_at_17.37.16_blocked_by The text is outdated here, you can find new one in the notes message summary table ☝
blocked_by Screenshot_2022-08-23_at_16.02.01 Screenshot_2022-08-23_at_17.37.16_copy

How to set up and validate locally

Prerequisites

Web

  1. Create 2 issues (#A and #B)
  2. Connect #A with #B as blocks/blocked_by
  3. Take a look at system notes

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Closes #365048 (closed)

Edited by Stanislav Dobrovolschii

Merge request reports