FIX: Store query groups in a temp table when fixing ids. (#68)

FIX: Store query groups in a temp table when fixing ids. (#68)

Reintroduces the migration removed in cdfc5d4, and fixes it to work with query_groups. Since the code is fairly complex, I moved into a rake task so it can be tested and make sure it works.

diff --git a/db/migrate/20200902225712_fix_query_ids.rb b/db/migrate/20200902225712_fix_query_ids.rb
new file mode 100644
index 0000000..4dcc4c3
--- /dev/null
+++ b/db/migrate/20200902225712_fix_query_ids.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class FixQueryIds < ActiveRecord::Migration[6.0]
+  def up
+    Rake::Task['data_explorer:fix_query_ids'].invoke
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end
diff --git a/lib/tasks/fix_query_ids.rake b/lib/tasks/fix_query_ids.rake
new file mode 100644
index 0000000..2939478
--- /dev/null
+++ b/lib/tasks/fix_query_ids.rake
@@ -0,0 +1,129 @@
+# frozen_string_literal: true
+
+desc 'Fix query IDs to match the old ones used in the plugin store (q:id)'
+task 'data_explorer:fix_query_ids' => :environment do
+  ActiveRecord::Base.transaction do
+    # Only queries with unique title can be fixed
+    movements = DB.query <<~SQL
+    SELECT deq.id AS from, (replace(plugin_store_rows.key, 'q:',''))::integer AS to
+    FROM plugin_store_rows
+    INNER JOIN data_explorer_queries deq ON deq.name = plugin_store_rows.value::json->>'name'
+    WHERE
+      (replace(plugin_store_rows.key, 'q:',''))::integer != deq.id AND
+      plugin_store_rows.plugin_name = 'discourse-data-explorer' AND
+      plugin_store_rows.type_name = 'JSON' AND
+      (SELECT COUNT(*) from data_explorer_queries deq2 WHERE deq.name = deq2.name) = 1
+    SQL
+
+    if movements.present?
+      # If there are new queries, they still may have conflict
+      # We just want to move their ids to safe space and we will not move them back
+      additional_conflicts = DB.query(<<~SQL, from: movements.map { |m| m.from }, to: movements.map { |m| m.to })
+        SELECT id FROM data_explorer_queries
+        WHERE id IN (:to)
+        AND id NOT IN (:from)
+      SQL
+      additional_conflicts = additional_conflicts.map(&:id)
+
+      # Create temporary tables
+      DB.exec <<~SQL
+        CREATE TEMPORARY TABLE tmp_data_explorer_queries(
+          id INTEGER PRIMARY KEY,
+          name VARCHAR,
+          description TEXT,
+          sql TEXT,
+          user_id INTEGER,
+          last_run_at TIMESTAMP,
+          hidden BOOLEAN,
+          created_at TIMESTAMP,
+          updated_at TIMESTAMP
+        )
+      SQL
+
+      DB.exec <<-SQL
+        CREATE TEMPORARY TABLE tmp_data_explorer_query_groups(
+          id INTEGER PRIMARY KEY,
+          query_id INTEGER,
+          group_id INTEGER
+        )
+      SQL
+
+      movements.each do |movement|
+        # insert movements to temporary tables
+        DB.exec <<-SQL
+          INSERT INTO tmp_data_explorer_queries(id, name, description, sql, user_id, last_run_at, hidden, created_at, updated_at)
+          SELECT #{movement.to}, name, description, sql, user_id, last_run_at, hidden, created_at, updated_at
+          FROM data_explorer_queries
+          WHERE id = #{movement.from}
+        SQL
+
+        DB.exec <<-SQL
+          INSERT INTO tmp_data_explorer_query_groups(id, query_id, group_id)
+          SELECT id, #{movement.to}, group_id
+          FROM data_explorer_query_groups
+          WHERE query_id = #{movement.from}
+        SQL
+      end
+
+      # insert rest to temporary tables
+      already_moved_ids = movements.map(&:from) | additional_conflicts
+      DB.exec(<<-SQL, already_moved_ids: already_moved_ids)
+        INSERT INTO tmp_data_explorer_queries(id, name, description, sql, user_id, last_run_at, hidden, created_at, updated_at)
+        SELECT id, name, description, sql, user_id, last_run_at, hidden, created_at, updated_at
+        FROM data_explorer_queries
+        WHERE id NOT IN (:already_moved_ids)
+      SQL
+
+      DB.exec(<<-SQL, already_moved_ids: already_moved_ids)
+        INSERT INTO tmp_data_explorer_query_groups(id, query_id, group_id)
+        SELECT id, query_id, group_id
+        FROM data_explorer_query_groups
+        WHERE query_id NOT IN (:already_moved_ids)
+      SQL
+
+      # insert additional_conflicts to temporary tables
+      new_id = DB.query("select greatest(max(id), 1) from tmp_data_explorer_queries").first.greatest + 1
+      additional_conflicts.each do |conflict_id|
+        DB.exec <<-SQL
+          INSERT INTO tmp_data_explorer_queries(id, name, description, sql, user_id, last_run_at, hidden, created_at, updated_at)
+          SELECT #{new_id}, name, description, sql, user_id, last_run_at, hidden, created_at, updated_at
+          FROM data_explorer_queries
+          WHERE id = #{conflict_id}
+        SQL
+
+        DB.exec <<~SQL
+          INSERT INTO tmp_data_explorer_query_groups(id, query_id, group_id)
+          SELECT id, #{new_id}, group_id
+          FROM data_explorer_query_groups
+          WHERE query_id = #{conflict_id}
+        SQL
+
+        new_id = new_id + 1
+      end
+
+      # clear original tables and copy data from temporary tables
+      DB.exec("DELETE FROM data_explorer_queries")
+      DB.exec("INSERT INTO data_explorer_queries SELECT * FROM tmp_data_explorer_queries")
+
+      DB.exec("DELETE FROM data_explorer_query_groups")
+      DB.exec("INSERT INTO data_explorer_query_groups SELECT * FROM tmp_data_explorer_query_groups")
+
+      # Update id sequences
+      DB.exec <<~SQL
+        SELECT
+          setval(
+            pg_get_serial_sequence('data_explorer_queries', 'id'),
+            (select greatest(max(id), 1) from data_explorer_queries)
+          );
+      SQL
+
+      DB.exec <<~SQL
+        SELECT
+          setval(
+            pg_get_serial_sequence('data_explorer_query_groups', 'id'),
+            (select greatest(max(id), 1) from data_explorer_query_groups)
+          );
+      SQL
+    end
+  end
+end
diff --git a/spec/lib/tasks/fix_query_ids_spec.rb b/spec/lib/tasks/fix_query_ids_spec.rb
new file mode 100644
index 0000000..45d66ff
--- /dev/null
+++ b/spec/lib/tasks/fix_query_ids_spec.rb
@@ -0,0 +1,148 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe 'fix query ids rake task' do
+  before do
+    Rake::Task.clear
+    Discourse::Application.load_tasks
+  end
+
+  let(:query_name) { 'Awesome query' }
+
+  it 'fixes the ID of the query if they share the same name' do
+    original_query_id = 4
+    create_plugin_store_row(query_name, original_query_id)
+    create_query(query_name)
+
+    run_task
+
+    expect(find(query_name).id).to eq(original_query_id)
+  end
+
+  it 'only fixes queries with unique name' do
+    original_query_id = 4
+    create_plugin_store_row(query_name, original_query_id)
+    create_query(query_name)
+    create_query(query_name)
+
+    run_task
+
+    expect(find(query_name).id).not_to eq(original_query_id)
+  end
+
+  it 'skips queries that already have the same ID' do
+    db_query = create_query(query_name)
+    last_updated_at = db_query.updated_at
+    create_plugin_store_row(query_name, db_query.id)
+
+    run_task
+
+    expect(find(query_name).updated_at).to eq_time(last_updated_at)
+  end
+
+  it 'keeps queries the rest of the queries' do
+    original_query_id = 4
+    different_query_name = 'Another query'
+    create_plugin_store_row(query_name, original_query_id)
+    create_query(query_name)
+    create_query(different_query_name)
+
+    run_task
+
+    expect(find(different_query_name)).not_to be_nil
+  end
+
+  it 'works even if they are additional conflicts' do
+    different_query_name = 'Another query'
+    additional_conflict = create_query(different_query_name)
+    create_query(query_name)
+    create_plugin_store_row(query_name, additional_conflict.id)
+
+    run_task
+
+    expect(find(different_query_name).id).not_to eq(additional_conflict.id)
+    expect(find(query_name).id).to eq(additional_conflict.id)
+  end
+
+  context 'query groups' do
+    let(:group) { Fabricate(:group) }
+
+    it "fixes the query group's query_id" do
+      original_query_id = 4
+      create_query(query_name, [group.id])
+      create_plugin_store_row(query_name, original_query_id, [group.id])
+

[... diff too long, it was truncated ...]

GitHub sha: 51a047b6

This commit appears in #68 which was approved by danielwaterworth. It was merged by romanrizzi.