Add Iteration Cadences to Group Import/Export
What does this MR do and why?
This MR adds iterations cadences & iterations to Group Import/Export, so when the group is migrated, the group iteration cadences are preserved.
The migrated cadences will then be used (in the future MR) to preserve issue iteration infromation, burndown charts, etc.
Mentions #291983 (closed)
Screenshots or screen recordings
How to set up and validate locally
- Create a group with iteration cadence & cadences
- Export the group using Group Export functionality (under Settings -> Advanced)
- Download exported tarball and import it back
- Verify imported group has group iteration cadences & iterations imported and they are identical to the source group
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Merge request reports
Activity
assigned to @georgekoltsov
Suggested Reviewers (beta)
The individuals below may be good candidates to participate in the review based on various factors.
You can use slash commands in comments to quickly assign
/assign_reviewer @user1
.Suggested Reviewers @kassio
,@georgekoltsov
,@ashmckenzie
,@smcgivern
,@ayufan
If you do not believe these suggestions are useful, please apply the label Bad Suggested Reviewer. You can also provide feedback for this feature on this issue:
https://gitlab.com/gitlab-org/gitlab/-/issues/357923
.Automatically generated by Suggested Reviewers Bot - an experimental ML-based recommendation engine created by ~"group::applied ml".
Edited by GitLab Reviewer-Recommender Bot- A deleted user
added backend featureaddition typefeature labels
1 Warning featureaddition and featureenhancement merge requests normally have a documentation change. Consider adding a documentation update or confirming the documentation plan with the Technical Writer counterpart.
For more information, see:
- The Handbook page on merge request types.
- The definition of done documentation.
Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer backend Mehmet Emin Inac ( @minac
) (UTC+2, 1 hour ahead of@georgekoltsov
)Stan Hu ( @stanhu
) (UTC-7, 8 hours behind@georgekoltsov
)To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerAllure report
allure-report-publisher
generated test report!review-qa-blocking:
test report for 92d433ccexpand test summary
+-----------------------------------------------------------------------------------------+ | suites summary | +------------------------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------------------------+--------+--------+---------+-------+-------+--------+ | Plan | 47 | 0 | 1 | 47 | 48 | ❗ | | Verify | 12 | 0 | 1 | 12 | 13 | ❗ | | Create | 28 | 0 | 1 | 28 | 29 | ❗ | | Manage | 46 | 0 | 3 | 39 | 49 | ❗ | | Package | 0 | 0 | 1 | 0 | 1 | ➖ | | Protect | 2 | 0 | 0 | 2 | 2 | ❗ | | Configure | 0 | 0 | 1 | 0 | 1 | ➖ | | Version sanity check | 0 | 0 | 1 | 0 | 1 | ➖ | | Feature flag handler sanity checks | 9 | 0 | 0 | 9 | 9 | ❗ | | Secure | 2 | 0 | 0 | 2 | 2 | ❗ | +------------------------------------+--------+--------+---------+-------+-------+--------+ | Total | 146 | 0 | 9 | 139 | 155 | ❗ | +------------------------------------+--------+--------+---------+-------+-------+--------+
added 1 commit
- 6cbb959d - Add Iteration Cadences to Group Import/Export
added 1 commit
- 20295e18 - Add Iteration Cadences to Group Import/Export
added 1 commit
- d98f6129 - Add Iteration Cadences to Group Import/Export
changed milestone to %15.4
added 1 commit
- b03e5a31 - Add Iteration Cadences to Group Import/Export
added Category:Importers Importer:Group Export/Import sectiondev + 1 deleted label
@rodrigo.tomonari @carlad-gl may I ask you to review this one, please?
requested review from @rodrigo.tomonari and @carlad-gl
Setting label(s) devopsmanage based on ~"group::import".
added devopsmanage label
added 206 commits
-
b03e5a31...c18ba625 - 205 commits from branch
master
- 9ac92af4 - Add Iteration Cadences to Group Import/Export
-
b03e5a31...c18ba625 - 205 commits from branch
added 1 commit
- 2eb4ff06 - Add Iteration Cadences to Group Import/Export
36 - :duration_in_weeks 37 - :iterations_in_advance 38 - :active 39 - :automatic 40 - :roll_over 41 - :title 42 - :description 43 iterations_cadences: *iterations_cadence_definition 44 iteration: &iteration_definition 45 - :iid 46 - :created_at 47 - :updated_at 48 - :start_date 49 - :due_date 50 - :group_id 51 - :description @rodrigo.tomonari forgot to mention about these fields:
-
cached_markdown_version
is a prohibited field so theres no point in exporting it https://gitlab.com/gitlab-org/gitlab/blob/georgekoltsov%2Fadd-iterations-to-import-export/lib/gitlab/import_export/attribute_cleaner.rb#L15-15 same for*_html
fields. -
title
of iteration will become a deprecated field. I read that in the documentation https://docs.gitlab.com/ee/user/group/iterations/After Iteration Cadences becomes generally available, manual iteration scheduling will be deprecated in GitLab 15.6. To enhance the role of iterations as time boundaries, we will also deprecate the title field.
-
state_enum
is recalculated in before save hook, so I left this field out https://gitlab.com/gitlab-org/gitlab/blob/georgekoltsov%2Fadd-iterations-to-import-export/ee/app/models/ee/iteration.rb#L56-56 -
sequence
- I didn't notice this field before. I'll add it
-
Actually,
sequence
is also updated in an after_save hook, so I'll leave it out https://gitlab.com/gitlab-org/gitlab/blob/georgekoltsov%2Fadd-iterations-to-import-export/ee/app/models/ee/iteration.rb#L307-307Edited by George KoltsovLooking at the SQL that is ran there we might need to preserve sequence and not run the after_save hook
not sure. I'll leave sequence out for now.Edited by George Koltsov@euko I think you worked on this. I assume
sequence
is useful to export since it captures how many times the iteration was updated?Yes
sequence
is a useful piece of information and should be exported.
For iterations cadence, the following fields don't need to be preserved during import/export in my opinion:
- :last_run_date - :duration_in_weeks - :iterations_in_advance - :roll_over
When
automatic
is settrue
, it indicates automatic scheduling should be enabled and a worker creates new iteration objects for the cadence according to the fields. I think a user can re-enable the automation after importing a cadence appropriately. What do you think @gweaver?@georgekoltsov if we omit them in import_export.yml, what values do they get in the db when imported again?
Issue: #371472
if we omit them in import_export.yml, what values do they get in the db when imported again?
@euko These values will be
nil
when importing. I see thatroll_over
has not null constraint, but the rest can be excluded if we want. I can create a new MR, if needed.Can you please clarify on
automatic
- are you suggesting we do not migrateautomatic
field either?I will also add a new MR to add
sequence
, thanks for clarification on it.These values will be
nil
when importing. I see thatroll_over
has not null constraint, but the rest can be excluded if we want. I can create a new MR, if needed.Can you please clarify on
automatic
- are you suggesting we do not migrateautomatic
field either?Yes I am suggesting we don't migrate
automatic
field and set them tofalse
on import by default. I would like to get an additional opinion from @gweaver on this though.To confirm, when
automatic
isfalse
,roll_over
should be defaulted tofalse
as well. The rest can be null:last_run_date
,duration_in_weeks
,iterations_in_advance
incl.start_date
.(note to myself: I should consider adding check constraints that mirror the model validations i.e., certain fields must be non-null when automatic is true.)
also wondering if this callback should be ran for updating
sequence
?Running the callback should not be necessary to update
sequence
on import unless they've been tempered with during export/import.Yes I am suggesting we don't migrate
automatic
field and set them tofalse
on import by default.@euko is there a reason why we wouldn't migrate the automated settings? If there is a technical challenge, then I'm fine with not doing this. I also see value in potentially including these from a user experience perspective.
I also see value in potentially including these from a user experience perspective.
@gweaver There is no technical challenge. I was thinking a user would re-enable automatic scheduling for (like power-on) a cadence if it's really actively used after an import and the rest of the cadences can stop generating new iterations. I don't have a strong opinion here. It's up to you.
@euko @georgekoltsov lets go with your intuition on this one and not migrate the automated settings. This is a two way door so we can always iterate based on feedback
added 1 commit
- aaa6a5ed - Add Iteration Cadences to Group Import/Export
added 1 commit
- 92d433cc - Add Iteration Cadences to Group Import/Export
removed review request for @carlad-gl
@carlad-gl
, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.
For more info, please refer to the following links:
removed review request for @rodrigo.tomonari
@stanhu may I ask you to review this one, please?
requested review from @stanhu
@stanhu, did you forget to run a pipeline before you merged this work? Based on our code review process, if the latest pipeline finished more than 2 hours ago, you should:
- Ensure the merge request is not in Draft status.
- Start a pipeline (especially important for Community contribution merge requests).
- Set the merge request to merge when pipeline succeeds.
This is a guideline, not a rule. Please consider replying to this comment for transparency.
This message was generated automatically. You're welcome to improve it.
mentioned in commit 49ec240a
added workflowstaging-canary label
added workflowcanary label and removed workflowstaging-canary label
added workflowstaging label and removed workflowcanary label
added workflowproduction label and removed workflowstaging label
added workflowpost-deploy-db-staging label and removed workflowproduction label
added workflowpost-deploy-db-production label and removed workflowpost-deploy-db-staging label
mentioned in issue #371462 (closed)
mentioned in commit 94d4c1ea
mentioned in commit 7872735a
mentioned in merge request !95872 (merged)
mentioned in commit 87d4f8fd
mentioned in commit 9c8d88bb
mentioned in commit 99a43156
mentioned in commit 19596726
mentioned in merge request !95918 (merged)
added releasedcandidate label
mentioned in merge request kubitus-project/kubitus-installer!1453 (merged)
mentioned in issue gitlab-com/www-gitlab-com#13806 (closed)
added groupimport and integrate label and removed 1 deleted label