Related to #452151
When removing a related epic that has a synced work item also destroys the RelatedWorkItemLinks
record, regardless of the feature flag epic_creation_with_synced_work_item
status.
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Feature.enable(:epic_creation_with_synced_work_item)
Source Epic
and Target Epic
). Verify that the corresponding work items were also createdSource Epic
and add Target Epic
as a related. Verify that the related epic is also visible when visiting the synced work itemFeature.enable(:epic_creation_with_synced_work_item)
I personally lean towards not deleting children, but can go either way.
same!
Do we know who made the decision for this behavior on work items?
I don't think on work items we delete the full hierarchy.
@nicolasdular yeah I don't think we need one it is also not a feature and it does not change anything on the customers side.
Alexandru Croitor (12f915c8) at 28 Mar 16:57
Merge branch '452150-destroy-work_item_parent_links-records-for-a-s...
... and 1 more commit
Related to #452150
When deleting epic children that have a synced work item, these should also be deleted, regardless of the status of the feature flag epic_creation_with_synced_work_item
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Feature.enable(:epic_creation_with_synced_work_item)
Parent
and Child
). Verify that the corresponding work items were also createdParent
and add the existing epic Child
as a child. Verify that the child is also visible when visiting the synced work itemFeature.enable(:epic_creation_with_synced_work_item)
I think there is a valid use case for both scenarios:
I think ideally we would have a UI that presents the 2 options:
I agree with @egrieff about removing the ON DELETE CASCADE constraint on DB level, but I wonder if have to consider replacing it with an in-app deletion full hierarchy deletion, if we want to continue to preserve current behaviour. In app deletion is more work, than just changing the DB constraint.
Update:
I'm happy and supportive, if we decide to not reproduce the current functionality(i.e. delete full hierarchy) given it is probably a very rate occurence to remove epics in general and remove a lot of epics in particular. Just wanted to make sure we cover it in as much detail as possible.
Alexandru Croitor (6f79e979) at 28 Mar 11:38
Merge branch 'sh-fix-flaky-branch-rule-test' into 'master'
... and 1 more commit
ee/spec/requests/api/graphql/project/branch_rules_spec.rb
appears to
have failed in https://gitlab.com/gitlab-org/gitlab/-/jobs/6494504318
because the branch rule creation date was off by a second. It appears
that there was a copy and paste error here since the test was
comparing the wrong branch rule creation date.
do we need to set work_item.title_html = epic.title_html
and work_item.description_html = epic.description_html
here as well, given that we are setting it when epic
is not persisted yet in SyncAsWorkItem
or are html fields already rendered at that time?
Especially consider the update title or description use case, I wonder if we may end with out of sync title or description on the work item side ? Do we have a specific spec that covers that, that we can check ?
How about returning it earlier if
epic.valid?
is false?
that is exactly my thinking.
I do agree, I think we should not overwrite the validation errors with the opaque SyncAsWorkItemError
.
Maybe we can/should check that epic.valid?
before calling the create_work_item_for!(epic)
?
So then if epic is not valid we would expose the validation errors and only after that expose the SyncAsWorkItemError
?
ee/spec/requests/api/graphql/project/branch_rules_spec.rb
appears to
have failed in https://gitlab.com/gitlab-org/gitlab/-/jobs/6494504318
because the branch rule creation date was off by a second. It appears
that there was a copy and paste error here since the test was
comparing the wrong branch rule creation date.
Related to #452150
When deleting epic children that have a synced work item, these should also be deleted, regardless of the status of the feature flag epic_creation_with_synced_work_item
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Feature.enable(:epic_creation_with_synced_work_item)
Parent
and Child
). Verify that the corresponding work items were also createdParent
and add the existing epic Child
as a child. Verify that the child is also visible when visiting the synced work itemFeature.enable(:epic_creation_with_synced_work_item)
Alexandru Croitor (24ecbd7d) at 27 Mar 18:13
Fixes due to not null constraint added to epics.issue_id
In the effort of migrating legacy Epic to Epic Work Item we are synching movement of the issue's epic and epic's work item which generates extra queries that exceed the 100 queries limit.
Alexandru Croitor (4780f323) at 27 Mar 15:45
Remove epic_creation_with_synced_work_item FF
@nicolasdular I'm going to assign this to you and close it.
We need to make sure that when an epic is destroyed we destroy the corresponding Work Item regardless of the FF status so that we do not end up with orphaned synched work items.
thank you for helping out