Merge branch '273574-fix-bad-data-in-projects-has_external_wiki' into 'master'

Fix bad data in projects.has_external_wiki

See merge request gitlab-org/gitlab!53790

changelogs/unreleased/273574-fix-bad-data-in-projects-has_external_issue_tracker.yml

+---
+title: Cleanup incorrect data in projects.has_external_wiki
+merge_request: 53790
+author:
+type: fixed

db/post_migrate/20210105030125_cleanup_projects_with_bad_has_external_wiki_data.rb

+# frozen_string_literal: true
+
+class CleanupProjectsWithBadHasExternalWikiData < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  TMP_INDEX_NAME = 'tmp_index_projects_on_id_where_has_external_wiki_is_true'.freeze
+  BATCH_SIZE = 100
+
+  disable_ddl_transaction!
+
+  class Service < ActiveRecord::Base
+    include EachBatch
+    belongs_to :project
+
+    self.table_name = 'services'
+    self.inheritance_column = :_type_disabled
+  end
+
+  class Project < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'projects'
+  end
+
+  def up
+    update_projects_with_active_external_wikis
+    update_projects_without_active_external_wikis
+  end
+
+  def down
+    # no-op : can't go back to incorrect data
+  end
+
+  private
+
+  def update_projects_with_active_external_wikis
+    # 11 projects are scoped in this query on GitLab.com.
+    scope = Service.where(active: true, type: 'ExternalWikiService').where.not(project_id: nil)
+
+    scope.each_batch(of: BATCH_SIZE) do |relation|
+      scope_with_projects = relation
+        .joins(:project)
+        .select('project_id')
+        .merge(Project.where(has_external_wiki: false).where(pending_delete: false).where(archived: false))
+
+      execute(<<~SQL)
+      WITH project_ids_to_update (id) AS (
+        #{scope_with_projects.to_sql}
+      )
+      UPDATE projects SET has_external_wiki = true WHERE id IN (SELECT id FROM project_ids_to_update)
+      SQL
+    end
+  end
+
+  def update_projects_without_active_external_wikis
+    # Add a temporary index to speed up the scoping of projects.
+    index_where = <<~SQL
+      (
+        "projects"."has_external_wiki" = TRUE
+      )
+      AND "projects"."pending_delete" = FALSE
+      AND "projects"."archived" = FALSE
+    SQL
+
+    add_concurrent_index(:projects, :id, where: index_where, name: TMP_INDEX_NAME)
+
+    services_sub_query = Service
+      .select('1')
+      .where('services.project_id = projects.id')
+      .where(type: 'ExternalWikiService')
+      .where(active: true)
+
+    # 322 projects are scoped in this query on GitLab.com.
+    Project.where(index_where).each_batch(of: BATCH_SIZE) do |relation|
+      relation_with_exists_query = relation.where('NOT EXISTS (?)', services_sub_query)
+      execute(<<~SQL)
+      WITH project_ids_to_update (id) AS (
+        #{relation_with_exists_query.select(:id).to_sql}
+      )
+      UPDATE projects SET has_external_wiki = false WHERE id IN (SELECT id FROM project_ids_to_update)
+      SQL
+    end
+
+    # Drop the temporary index.
+    remove_concurrent_index_by_name(:projects, TMP_INDEX_NAME)
+  end
+end

db/schema_migrations/20210105030125

+c5a780e5b5e62043fb04e77ebf89f7d04dfc9bfdc70df8d89c16a3f3fd960ea3
\ No newline at end of file

spec/migrations/cleanup_projects_with_bad_has_external_wiki_data_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require_migration!
+
+RSpec.describe CleanupProjectsWithBadHasExternalWikiData, :migration do
+  let(:namespace) { table(:namespaces).create!(name: 'foo', path: 'bar') }
+  let(:projects) { table(:projects) }
+  let(:services) { table(:services) }
+
+  def create_projects!(num)
+    Array.new(num) do
+      projects.create!(namespace_id: namespace.id)
+    end
+  end
+
+  def create_active_external_wiki_integrations!(*projects)
+    projects.each do |project|
+      services.create!(type: 'ExternalWikiService', project_id: project.id, active: true)
+    end
+  end
+
+  def create_disabled_external_wiki_integrations!(*projects)
+    projects.each do |project|
+      services.create!(type: 'ExternalWikiService', project_id: project.id, active: false)
+    end
+  end
+
+  def create_active_other_integrations!(*projects)
+    projects.each do |project|
+      services.create!(type: 'NotAnExternalWikiService', project_id: project.id, active: true)
+    end
+  end
+
+  it 'sets `projects.has_external_wiki` correctly' do
+    allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false)
+
+    project_with_external_wiki_1,
+      project_with_external_wiki_2,
+      project_with_disabled_external_wiki_1,
+      project_with_disabled_external_wiki_2,
+      project_without_external_wiki_1,
+      project_without_external_wiki_2 = create_projects!(6)
+
+    create_active_external_wiki_integrations!(
+      project_with_external_wiki_1,
+      project_with_external_wiki_2
+    )
+
+    create_disabled_external_wiki_integrations!(
+      project_with_disabled_external_wiki_1,
+      project_with_disabled_external_wiki_2
+    )
+
+    create_active_other_integrations!(
+      project_without_external_wiki_1,
+      project_without_external_wiki_2
+    )
+
+    # PG triggers on the services table added in a previous migration
+    # will have set the `has_external_wiki` columns to correct data when
+    # the services records were created above.
+    #
+    # We set the `has_external_wiki` columns for projects to incorrect
+    # data manually below to emulate projects in a state before the PG
+    # triggers were added.
+    project_with_external_wiki_2.update!(has_external_wiki: false)
+    project_with_disabled_external_wiki_2.update!(has_external_wiki: true)
+    project_without_external_wiki_2.update!(has_external_wiki: true)
+
+    migrate!
+
+    expected_true = [
+      project_with_external_wiki_1,
+      project_with_external_wiki_2
+    ].each(&:reload).map(&:has_external_wiki)
+
+    expected_not_true = [
+      project_without_external_wiki_1,
+      project_without_external_wiki_2,
+      project_with_disabled_external_wiki_1,
+      project_with_disabled_external_wiki_2
+    ].each(&:reload).map(&:has_external_wiki)
+
+    expect(expected_true).to all(eq(true))
+    expect(expected_not_true).to all(be_falsey)
+  end
+end