Skip to content

Reset column information after the schema is migrated in MigrationsHelpers.schema_migrate_up!

Rémy Coutable requested to merge ce-4813-fix-ee-projects-destroy_service_spec into master

What does this MR do?

Because we call [Project, MergeRequest, MergeRequestDiff].each(&:reset_column_information) in an after block in spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb (https://gitlab.com/gitlab-org/gitlab-ee/blob/cbaa5cedfcbb1a9bf40ccbc1a975e6501e5895c2/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb#L18-20) but we only migrate up in an after(:context) (https://gitlab.com/gitlab-org/gitlab-ee/blob/cbaa5cedfcbb1a9bf40ccbc1a975e6501e5895c2/spec/spec_helper.rb#L175-177), columns information are still outdated after the schema_migrate_up! call.

If we reset the column information after we migrate the schema up, the column information are correctly updated.

Are there points in the code the reviewer needs to double check?

We're now resetting all the models in MigrationsHelpers.schema_migrate_up!, I think that's ok performance-wise, as this is only executed for migration specs, I think the pros outweigh the cons.

Why was this MR needed?

Before this change, bin/rspec spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb spec/lib/gitlab/background_migration/populate_merge_request_metrics_with_events_data_spec.rb spec/ee/spec/services/ee/projects/destroy_service_spec.rb would fail.

After this change, it passes.

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Closes gitlab-ee#4813

Merge request reports