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.
This commit is contained in:
Roman Rizzi
GitHub
parent
4a167a7f5a
commit
51a047b60c
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
@ -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])
|
||||
|
||||
run_task
|
||||
|
||||
expect(find_query_group(original_query_id)).not_to be_nil
|
||||
end
|
||||
|
||||
it 'works with additional conflicts' do
|
||||
different_query_name = 'Another query'
|
||||
additional_conflict = create_query(different_query_name, [group.id])
|
||||
create_query(query_name, [group.id])
|
||||
create_plugin_store_row(query_name, additional_conflict.id, [group.id])
|
||||
|
||||
run_task
|
||||
|
||||
conflict = find(different_query_name).query_groups.first
|
||||
fixed = find_query_group(additional_conflict.id)
|
||||
|
||||
expect(conflict.query_id).not_to eq(additional_conflict.id)
|
||||
expect(fixed.query_id).to eq(additional_conflict.id)
|
||||
end
|
||||
|
||||
def find_query_group(id)
|
||||
DataExplorer::QueryGroup.find_by(query_id: id)
|
||||
end
|
||||
end
|
||||
|
||||
it 'changes the serial sequence for future queries' do
|
||||
original_query_id = 4
|
||||
create_plugin_store_row(query_name, original_query_id)
|
||||
create_query(query_name)
|
||||
|
||||
run_task
|
||||
post_fix_query = create_query(query_name)
|
||||
|
||||
expect(post_fix_query.id).to eq(original_query_id + 1)
|
||||
end
|
||||
|
||||
def run_task
|
||||
Rake::Task['data_explorer:fix_query_ids'].invoke
|
||||
end
|
||||
|
||||
def create_plugin_store_row(name, id, group_ids = [])
|
||||
key = "q:#{id}"
|
||||
|
||||
PluginStore.set(
|
||||
DataExplorer.plugin_name,
|
||||
key,
|
||||
attributes(name).merge(group_ids: group_ids, id: id)
|
||||
)
|
||||
end
|
||||
|
||||
def create_query(name, group_ids = [])
|
||||
DataExplorer::Query.create!(attributes(name)).tap do |query|
|
||||
group_ids.each do |group_id|
|
||||
query.query_groups.create!(group_id: group_id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def attributes(name)
|
||||
{
|
||||
name: name,
|
||||
description: 'A Query',
|
||||
sql: "SELECT 1",
|
||||
created_at: 3.hours.ago,
|
||||
last_run_at: 1.hour.ago,
|
||||
hidden: false
|
||||
}
|
||||
end
|
||||
|
||||
def find(name)
|
||||
DataExplorer::Query.find_by(name: name)
|
||||
end
|
||||
end
|
||||
Loading…
Reference in New Issue