From 0e2f6b7831796a3c579210d080a8e1d261781d97 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Mon, 6 Jan 2025 09:38:19 -0600 Subject: [PATCH 1/9] feat(migration): Add MetadataTemplates table with fields and namespace reference --- .../20250106153442_create_metadata_templates.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 db/migrate/20250106153442_create_metadata_templates.rb diff --git a/db/migrate/20250106153442_create_metadata_templates.rb b/db/migrate/20250106153442_create_metadata_templates.rb new file mode 100644 index 0000000000..eb15ba40ff --- /dev/null +++ b/db/migrate/20250106153442_create_metadata_templates.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# Migration to add MetadataTemplates table +class CreateMetadataTemplates < ActiveRecord::Migration[7.2] + def change + create_table :metadata_templates do |t| + t.string :name + t.string :description + t.jsonb :fields + t.references :namespace, foreign_key: true, index: true + t.timestamps + end + end +end From 2dfbec1cd69a9583bef775584925efdb3dde7473 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Tue, 7 Jan 2025 06:19:59 -0600 Subject: [PATCH 2/9] feat(metadata): Implement MetadataTemplate model with associations and validations --- app/models/metadata_template.rb | 16 ++++ ...0250106153442_create_metadata_templates.rb | 9 +- db/schema.rb | 50 ++++++---- test/fixtures/metadata_templates.yml | 16 ++++ test/models/metadata_template_test.rb | 93 +++++++++++++++++++ 5 files changed, 162 insertions(+), 22 deletions(-) create mode 100644 app/models/metadata_template.rb create mode 100644 test/fixtures/metadata_templates.yml create mode 100644 test/models/metadata_template_test.rb diff --git a/app/models/metadata_template.rb b/app/models/metadata_template.rb new file mode 100644 index 0000000000..c308462e71 --- /dev/null +++ b/app/models/metadata_template.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# MetadataTemplate represents a template for structured metadata fields +class MetadataTemplate < ApplicationRecord + include TrackActivity + + has_logidze + acts_as_paranoid + broadcasts_refreshes + + # Associations + belongs_to :namespace + belongs_to :created_by, class_name: 'User' + + has_many :metadata_fields, dependent: :destroy +end diff --git a/db/migrate/20250106153442_create_metadata_templates.rb b/db/migrate/20250106153442_create_metadata_templates.rb index eb15ba40ff..4edc8534c1 100644 --- a/db/migrate/20250106153442_create_metadata_templates.rb +++ b/db/migrate/20250106153442_create_metadata_templates.rb @@ -3,11 +3,12 @@ # Migration to add MetadataTemplates table class CreateMetadataTemplates < ActiveRecord::Migration[7.2] def change - create_table :metadata_templates do |t| - t.string :name + create_table :metadata_templates, id: :uuid do |t| + t.references :namespace, type: :uuid, null: false, foreign_key: true + t.references :created_by, type: :uuid, null: false, foreign_key: { to_table: :users } + t.string :name, null: false t.string :description - t.jsonb :fields - t.references :namespace, foreign_key: true, index: true + t.jsonb :fields, null: false, default: [] t.timestamps end end diff --git a/db/schema.rb b/db/schema.rb index 5dafb55105..ac765bfacc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_12_12_164410) do +ActiveRecord::Schema[7.2].define(version: 2025_01_06_153442) do # These are extensions that must be enabled in order to support this database enable_extension "hstore" enable_extension "plpgsql" @@ -131,6 +131,18 @@ t.index ["user_id"], name: "index_members_on_user_id" end + create_table "metadata_templates", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "namespace_id", null: false + t.uuid "created_by_id", null: false + t.string "name" + t.string "description" + t.jsonb "fields", default: [], null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["created_by_id"], name: "index_metadata_templates_on_created_by_id" + t.index ["namespace_id"], name: "index_metadata_templates_on_namespace_id" + end + create_table "namespace_bots", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "user_id", null: false t.uuid "namespace_id", null: false @@ -332,6 +344,8 @@ add_foreign_key "data_exports", "users" add_foreign_key "members", "namespaces" add_foreign_key "members", "users" + add_foreign_key "metadata_templates", "namespaces" + add_foreign_key "metadata_templates", "users", column: "created_by_id" add_foreign_key "namespace_group_links", "namespaces" add_foreign_key "namespaces", "users", column: "owner_id" add_foreign_key "personal_access_tokens", "users" @@ -714,34 +728,34 @@ SQL - create_trigger :logidze_on_attachments, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_attachments BEFORE INSERT OR UPDATE ON public.attachments FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') - SQL - create_trigger :logidze_on_automated_workflow_executions, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_automated_workflow_executions BEFORE INSERT OR UPDATE ON public.automated_workflow_executions FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') + create_trigger :logidze_on_users, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_users BEFORE INSERT OR UPDATE ON public.users FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL - create_trigger :logidze_on_data_exports, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_data_exports BEFORE INSERT OR UPDATE ON public.data_exports FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') + create_trigger :logidze_on_namespaces, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_namespaces BEFORE INSERT OR UPDATE ON public.namespaces FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at', '{created_at,metadata_summary,updated_at,attachments_updated_at}') SQL create_trigger :logidze_on_members, sql_definition: <<-SQL CREATE TRIGGER logidze_on_members BEFORE INSERT OR UPDATE ON public.members FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL - create_trigger :logidze_on_namespace_bots, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_namespace_bots BEFORE INSERT OR UPDATE ON public.namespace_bots FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') + create_trigger :logidze_on_samples, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_samples BEFORE INSERT OR UPDATE ON public.samples FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at', '{created_at,metadata_provenance,updated_at,attachments_updated_at}') + SQL + create_trigger :logidze_on_personal_access_tokens, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_personal_access_tokens BEFORE INSERT OR UPDATE ON public.personal_access_tokens FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at', '{last_used_at}') SQL create_trigger :logidze_on_namespace_group_links, sql_definition: <<-SQL CREATE TRIGGER logidze_on_namespace_group_links BEFORE INSERT OR UPDATE ON public.namespace_group_links FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL - create_trigger :logidze_on_namespaces, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_namespaces BEFORE INSERT OR UPDATE ON public.namespaces FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at', '{created_at,metadata_summary,updated_at,attachments_updated_at}') + create_trigger :logidze_on_attachments, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_attachments BEFORE INSERT OR UPDATE ON public.attachments FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL - create_trigger :logidze_on_personal_access_tokens, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_personal_access_tokens BEFORE INSERT OR UPDATE ON public.personal_access_tokens FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at', '{last_used_at}') + create_trigger :logidze_on_data_exports, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_data_exports BEFORE INSERT OR UPDATE ON public.data_exports FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL - create_trigger :logidze_on_samples, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_samples BEFORE INSERT OR UPDATE ON public.samples FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at', '{created_at,metadata_provenance,updated_at,attachments_updated_at}') + create_trigger :logidze_on_namespace_bots, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_namespace_bots BEFORE INSERT OR UPDATE ON public.namespace_bots FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL - create_trigger :logidze_on_users, sql_definition: <<-SQL - CREATE TRIGGER logidze_on_users BEFORE INSERT OR UPDATE ON public.users FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') + create_trigger :logidze_on_automated_workflow_executions, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_automated_workflow_executions BEFORE INSERT OR UPDATE ON public.automated_workflow_executions FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL end diff --git a/test/fixtures/metadata_templates.yml b/test/fixtures/metadata_templates.yml new file mode 100644 index 0000000000..136dde8032 --- /dev/null +++ b/test/fixtures/metadata_templates.yml @@ -0,0 +1,16 @@ +# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html + +valid_metadata_template: + name: "Valid Template" + description: "This is a valid template." + fields: ["field_1", "field_2", "field_3"] + created_at: <%= 3.hours.ago %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:project1_namespace, :uuid) %> + created_by_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> + +invalid_metadata_template: + description: "This is an invalid template." + fields: ["field_1", "field_2", "field_3"] + created_at: <%= 3.hours.ago %> + namespace_id: <%= ActiveRecord::FixtureSet.identify(:project1_namespace, :uuid) %> + created_by_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> diff --git a/test/models/metadata_template_test.rb b/test/models/metadata_template_test.rb new file mode 100644 index 0000000000..ee21d13567 --- /dev/null +++ b/test/models/metadata_template_test.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'test_helper' + +class MetadataTemplateTest < ActiveSupport::TestCase + setup do + @namespace = namespaces_user_namespaces(:john_doe_namespace) + @user = users(:john_doe) + @valid_metadata_template = metadata_templates(:valid_metadata_template) + @invalid_metadata_template = metadata_templates(:invalid_metadata_template) + end + + # Validation Tests + test 'valid metadata template' do + assert @valid_metadata_template.valid? + assert_not_nil @valid_metadata_template.name + end + + test 'invalid without name' do + assert_not @invalid_metadata_template.valid? + # assert_not_nil @invalid_metadata_template.errors[:name] + end + + test 'invalid with duplicate name in same namespace' do + duplicate = @valid_metadata_template.dup + assert_not duplicate.valid? + assert_includes duplicate.errors[:name], 'has already been taken' + end + + test 'valid with same name in different namespace' do + duplicate = @valid_metadata_template.dup + duplicate.namespace = namespaces_user_namespaces(:jane_doe_namespace) + assert duplicate.valid? + end + + test 'description length validation' do + @valid_metadata_template.description = 'a' * 1001 + assert_not @valid_metadata_template.valid? + assert_includes @valid_metadata_template.errors[:description], 'is too long' + end + + # Association Tests + test 'belongs to namespace' do + assert_respond_to @valid_metadata_template, :namespace + assert_instance_of Namespace, @valid_metadata_template.namespace + end + + test 'belongs to created_by user' do + assert_respond_to @valid_metadata_template, :created_by + assert_instance_of User, @valid_metadata_template.created_by + end + + test 'has many metadata fields' do + assert_respond_to @valid_metadata_template, :metadata_fields + assert_kind_of ActiveRecord::Associations::CollectionProxy, @valid_metadata_template.metadata_fields + end + + # Soft Delete Tests + test 'soft deletes record' do + @valid_metadata_template.destroy + assert_not_nil @valid_metadata_template.deleted_at + assert_not MetadataTemplate.find_by(id: @valid_metadata_template.id) + assert MetadataTemplate.with_deleted.find_by(id: @valid_metadata_template.id) + end + + # Broadcasting Tests + test 'broadcasts refresh on save' do + assert_broadcasts @valid_metadata_template, :refresh do + @valid_metadata_template.save + end + end + + # Activity Tracking Tests + test 'tracks activity on create' do + template = MetadataTemplate.new( + name: 'Activity Test', + namespace: @namespace, + created_by: @user + ) + assert_difference 'Activity.count' do + template.save + end + end + + # Logidze Tests + test 'tracks history changes' do + original_name = @valid_metadata_template.name + assert_difference '@valid_metadata_template.log_size' do + @valid_metadata_template.update!(name: 'Updated Name') + end + assert_equal original_name, @valid_metadata_template.log_data.versions.first['changes']['name'] + end +end From 11791600d60dafc62abbfe8f9f81db104e88b81d Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Tue, 7 Jan 2025 07:24:16 -0600 Subject: [PATCH 3/9] feat(metadata): Add name validation and update migration for MetadataTemplate --- app/models/metadata_template.rb | 3 +++ ...0250106153442_create_metadata_templates.rb | 3 ++- test/models/metadata_template_test.rb | 21 +++++++++++++------ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/app/models/metadata_template.rb b/app/models/metadata_template.rb index c308462e71..2e74f07506 100644 --- a/app/models/metadata_template.rb +++ b/app/models/metadata_template.rb @@ -8,6 +8,9 @@ class MetadataTemplate < ApplicationRecord acts_as_paranoid broadcasts_refreshes + # Validations + validates :name, presence: true + # Associations belongs_to :namespace belongs_to :created_by, class_name: 'User' diff --git a/db/migrate/20250106153442_create_metadata_templates.rb b/db/migrate/20250106153442_create_metadata_templates.rb index 4edc8534c1..aed0b12931 100644 --- a/db/migrate/20250106153442_create_metadata_templates.rb +++ b/db/migrate/20250106153442_create_metadata_templates.rb @@ -6,9 +6,10 @@ def change create_table :metadata_templates, id: :uuid do |t| t.references :namespace, type: :uuid, null: false, foreign_key: true t.references :created_by, type: :uuid, null: false, foreign_key: { to_table: :users } - t.string :name, null: false + t.string :name t.string :description t.jsonb :fields, null: false, default: [] + t.datetime :deleted_at t.timestamps end end diff --git a/test/models/metadata_template_test.rb b/test/models/metadata_template_test.rb index ee21d13567..0b9143d465 100644 --- a/test/models/metadata_template_test.rb +++ b/test/models/metadata_template_test.rb @@ -7,7 +7,7 @@ class MetadataTemplateTest < ActiveSupport::TestCase @namespace = namespaces_user_namespaces(:john_doe_namespace) @user = users(:john_doe) @valid_metadata_template = metadata_templates(:valid_metadata_template) - @invalid_metadata_template = metadata_templates(:invalid_metadata_template) + @invalid_metadata_template = nil end # Validation Tests @@ -16,9 +16,16 @@ class MetadataTemplateTest < ActiveSupport::TestCase assert_not_nil @valid_metadata_template.name end + test 'invalid without namespace' do + @valid_metadata_template.namespace = nil + assert_not @valid_metadata_template.valid? + assert_not_nil metadata_template.errors[:namespace] + end + test 'invalid without name' do - assert_not @invalid_metadata_template.valid? - # assert_not_nil @invalid_metadata_template.errors[:name] + @valid_metadata_template.name = nil + assert_not @valid_metadata_template.valid? + assert_not_nil @valid_metadata_template.errors[:name] end test 'invalid with duplicate name in same namespace' do @@ -85,9 +92,11 @@ class MetadataTemplateTest < ActiveSupport::TestCase # Logidze Tests test 'tracks history changes' do original_name = @valid_metadata_template.name - assert_difference '@valid_metadata_template.log_size' do - @valid_metadata_template.update!(name: 'Updated Name') + travel 1.minute do + assert_difference -> { @valid_metadata_template.reload.log_size } do + @valid_metadata_template.update!(name: 'Updated Name') + end end - assert_equal original_name, @valid_metadata_template.log_data.versions.first['changes']['name'] + assert_equal original_name, @valid_metadata_template.reload.log_data.versions.first['changes']['name'] end end From 2218813fe18b56edabb2cc345c208eb2f8fcb60b Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Tue, 7 Jan 2025 11:37:39 -0600 Subject: [PATCH 4/9] feat(metadata): Enhance validations for MetadataTemplate and update migration for unique name constraint --- app/models/metadata_template.rb | 11 ++-- ...0250106153442_create_metadata_templates.rb | 6 +- db/schema.rb | 4 +- test/models/metadata_template_test.rb | 62 +++++++------------ 4 files changed, 38 insertions(+), 45 deletions(-) diff --git a/app/models/metadata_template.rb b/app/models/metadata_template.rb index 2e74f07506..826b822c62 100644 --- a/app/models/metadata_template.rb +++ b/app/models/metadata_template.rb @@ -8,12 +8,15 @@ class MetadataTemplate < ApplicationRecord acts_as_paranoid broadcasts_refreshes - # Validations - validates :name, presence: true - # Associations belongs_to :namespace belongs_to :created_by, class_name: 'User' - has_many :metadata_fields, dependent: :destroy + # Validations + validates :name, presence: true, uniqueness: { scope: [:namespace_id] } + validates :description, length: { maximum: 1000 } + # validates :namespace_type, + # inclusion: { + # in: [Group.sti_name, Namespaces::ProjectNamespace.sti_name] + # } end diff --git a/db/migrate/20250106153442_create_metadata_templates.rb b/db/migrate/20250106153442_create_metadata_templates.rb index aed0b12931..0baa7f24fc 100644 --- a/db/migrate/20250106153442_create_metadata_templates.rb +++ b/db/migrate/20250106153442_create_metadata_templates.rb @@ -4,7 +4,7 @@ class CreateMetadataTemplates < ActiveRecord::Migration[7.2] def change create_table :metadata_templates, id: :uuid do |t| - t.references :namespace, type: :uuid, null: false, foreign_key: true + t.references :namespace, type: :uuid, foreign_key: true, index: true t.references :created_by, type: :uuid, null: false, foreign_key: { to_table: :users } t.string :name t.string :description @@ -12,5 +12,9 @@ def change t.datetime :deleted_at t.timestamps end + add_index :metadata_templates, %i[namespace_id name], + unique: true, + where: '(deleted_at IS NULL)', + name: 'index_template_name_with_namespace' end end diff --git a/db/schema.rb b/db/schema.rb index ac765bfacc..fd60af004f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -132,14 +132,16 @@ end create_table "metadata_templates", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| - t.uuid "namespace_id", null: false + t.uuid "namespace_id" t.uuid "created_by_id", null: false t.string "name" t.string "description" t.jsonb "fields", default: [], null: false + t.datetime "deleted_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["created_by_id"], name: "index_metadata_templates_on_created_by_id" + t.index ["namespace_id", "name"], name: "index_template_name_with_namespace", unique: true, where: "(deleted_at IS NULL)" t.index ["namespace_id"], name: "index_metadata_templates_on_namespace_id" end diff --git a/test/models/metadata_template_test.rb b/test/models/metadata_template_test.rb index 0b9143d465..dfa82200ad 100644 --- a/test/models/metadata_template_test.rb +++ b/test/models/metadata_template_test.rb @@ -7,21 +7,14 @@ class MetadataTemplateTest < ActiveSupport::TestCase @namespace = namespaces_user_namespaces(:john_doe_namespace) @user = users(:john_doe) @valid_metadata_template = metadata_templates(:valid_metadata_template) - @invalid_metadata_template = nil + @invalid_metadata_template = metadata_templates(:invalid_metadata_template) end - # Validation Tests test 'valid metadata template' do assert @valid_metadata_template.valid? assert_not_nil @valid_metadata_template.name end - test 'invalid without namespace' do - @valid_metadata_template.namespace = nil - assert_not @valid_metadata_template.valid? - assert_not_nil metadata_template.errors[:namespace] - end - test 'invalid without name' do @valid_metadata_template.name = nil assert_not @valid_metadata_template.valid? @@ -43,13 +36,12 @@ class MetadataTemplateTest < ActiveSupport::TestCase test 'description length validation' do @valid_metadata_template.description = 'a' * 1001 assert_not @valid_metadata_template.valid? - assert_includes @valid_metadata_template.errors[:description], 'is too long' end # Association Tests test 'belongs to namespace' do assert_respond_to @valid_metadata_template, :namespace - assert_instance_of Namespace, @valid_metadata_template.namespace + assert_instance_of Namespaces::ProjectNamespace, @valid_metadata_template.namespace end test 'belongs to created_by user' do @@ -58,8 +50,8 @@ class MetadataTemplateTest < ActiveSupport::TestCase end test 'has many metadata fields' do - assert_respond_to @valid_metadata_template, :metadata_fields - assert_kind_of ActiveRecord::Associations::CollectionProxy, @valid_metadata_template.metadata_fields + assert_respond_to @valid_metadata_template, :fields + assert_kind_of Array, @valid_metadata_template.fields end # Soft Delete Tests @@ -70,33 +62,25 @@ class MetadataTemplateTest < ActiveSupport::TestCase assert MetadataTemplate.with_deleted.find_by(id: @valid_metadata_template.id) end - # Broadcasting Tests - test 'broadcasts refresh on save' do - assert_broadcasts @valid_metadata_template, :refresh do - @valid_metadata_template.save - end - end - # Activity Tracking Tests - test 'tracks activity on create' do - template = MetadataTemplate.new( - name: 'Activity Test', - namespace: @namespace, - created_by: @user - ) - assert_difference 'Activity.count' do - template.save - end - end + # test 'tracks activity on create' do + # template = MetadataTemplate.new( + # name: 'Activity Test', + # namespace: @namespace, + # created_by: @user + # ) + # assert_difference 'Activity.count' do + # template.save + # end + # end - # Logidze Tests - test 'tracks history changes' do - original_name = @valid_metadata_template.name - travel 1.minute do - assert_difference -> { @valid_metadata_template.reload.log_size } do - @valid_metadata_template.update!(name: 'Updated Name') - end - end - assert_equal original_name, @valid_metadata_template.reload.log_data.versions.first['changes']['name'] - end + # test 'tracks history changes' do + # original_name = @valid_metadata_template.name + # travel 1.minute do + # assert_difference -> { @valid_metadata_template.reload.log_size } do + # @valid_metadata_template.update!(name: 'Updated Name') + # end + # end + # assert_equal original_name, @valid_metadata_template.reload.log_data.versions.first['changes']['name'] + # end end From 3c57fa737891674980884596f69ce7452b8d3822 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Tue, 7 Jan 2025 14:18:21 -0600 Subject: [PATCH 5/9] feat(metadata): Add Logidze support to MetadataTemplates for change tracking --- ...94839_add_logidze_to_metadata_templates.rb | 20 ++++++++++++++++ db/schema.rb | 6 ++++- .../logidze_on_metadata_templates_v01.sql | 6 +++++ test/models/metadata_template_test.rb | 24 ++++++++++++------- 4 files changed, 46 insertions(+), 10 deletions(-) create mode 100644 db/migrate/20250107194839_add_logidze_to_metadata_templates.rb create mode 100644 db/triggers/logidze_on_metadata_templates_v01.sql diff --git a/db/migrate/20250107194839_add_logidze_to_metadata_templates.rb b/db/migrate/20250107194839_add_logidze_to_metadata_templates.rb new file mode 100644 index 0000000000..576c9d8174 --- /dev/null +++ b/db/migrate/20250107194839_add_logidze_to_metadata_templates.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# Migration to add Logidze to MetadataTemplates table +class AddLogidzeToMetadataTemplates < ActiveRecord::Migration[7.2] + def change + add_column :metadata_templates, :log_data, :jsonb + + reversible do |dir| + dir.up do + create_trigger :logidze_on_metadata_templates, on: :metadata_templates + end + + dir.down do + execute <<~SQL.squish + DROP TRIGGER IF EXISTS "logidze_on_metadata_template" on "metadata_templates"; + SQL + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index fd60af004f..348653eed3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_01_06_153442) do +ActiveRecord::Schema[7.2].define(version: 2025_01_07_194839) do # These are extensions that must be enabled in order to support this database enable_extension "hstore" enable_extension "plpgsql" @@ -140,6 +140,7 @@ t.datetime "deleted_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.jsonb "log_data" t.index ["created_by_id"], name: "index_metadata_templates_on_created_by_id" t.index ["namespace_id", "name"], name: "index_template_name_with_namespace", unique: true, where: "(deleted_at IS NULL)" t.index ["namespace_id"], name: "index_metadata_templates_on_namespace_id" @@ -760,4 +761,7 @@ create_trigger :logidze_on_automated_workflow_executions, sql_definition: <<-SQL CREATE TRIGGER logidze_on_automated_workflow_executions BEFORE INSERT OR UPDATE ON public.automated_workflow_executions FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') SQL + create_trigger :logidze_on_metadata_templates, sql_definition: <<-SQL + CREATE TRIGGER logidze_on_metadata_templates BEFORE INSERT OR UPDATE ON public.metadata_templates FOR EACH ROW WHEN ((COALESCE(current_setting('logidze.disabled'::text, true), ''::text) <> 'on'::text)) EXECUTE FUNCTION logidze_logger('null', 'updated_at') + SQL end diff --git a/db/triggers/logidze_on_metadata_templates_v01.sql b/db/triggers/logidze_on_metadata_templates_v01.sql new file mode 100644 index 0000000000..39b8af679e --- /dev/null +++ b/db/triggers/logidze_on_metadata_templates_v01.sql @@ -0,0 +1,6 @@ +CREATE TRIGGER "logidze_on_metadata_templates" +BEFORE UPDATE OR INSERT ON "metadata_templates" FOR EACH ROW +WHEN (coalesce(current_setting('logidze.disabled', true), '') <> 'on') +-- Parameters: history_size_limit (integer), timestamp_column (text), filtered_columns (text[]), +-- include_columns (boolean), debounce_time_ms (integer) +EXECUTE PROCEDURE logidze_logger(null, 'updated_at'); diff --git a/test/models/metadata_template_test.rb b/test/models/metadata_template_test.rb index dfa82200ad..4ceec535c0 100644 --- a/test/models/metadata_template_test.rb +++ b/test/models/metadata_template_test.rb @@ -74,13 +74,19 @@ class MetadataTemplateTest < ActiveSupport::TestCase # end # end - # test 'tracks history changes' do - # original_name = @valid_metadata_template.name - # travel 1.minute do - # assert_difference -> { @valid_metadata_template.reload.log_size } do - # @valid_metadata_template.update!(name: 'Updated Name') - # end - # end - # assert_equal original_name, @valid_metadata_template.reload.log_data.versions.first['changes']['name'] - # end + test 'tracks history changes' do + @valid_metadata_template.create_logidze_snapshot! + + assert_equal 1, @valid_metadata_template.log_data.version + assert_equal 1, @valid_metadata_template.log_data.size + + assert_changes -> { @valid_metadata_template.name }, to: 'Updated Name' do + @valid_metadata_template.update(name: 'Updated Name') + end + + @valid_metadata_template.create_logidze_snapshot! + + assert_equal 2, @valid_metadata_template.log_data.version + assert_equal 2, @valid_metadata_template.log_data.size + end end From 795e427bbb84c99153c1d6332486cb5eaff2655a Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Wed, 8 Jan 2025 08:18:17 -0600 Subject: [PATCH 6/9] feat(tests): Remove invalid metadata template fixture from tests --- test/fixtures/metadata_templates.yml | 7 ------- test/models/metadata_template_test.rb | 1 - 2 files changed, 8 deletions(-) diff --git a/test/fixtures/metadata_templates.yml b/test/fixtures/metadata_templates.yml index 136dde8032..891e4c04a5 100644 --- a/test/fixtures/metadata_templates.yml +++ b/test/fixtures/metadata_templates.yml @@ -7,10 +7,3 @@ valid_metadata_template: created_at: <%= 3.hours.ago %> namespace_id: <%= ActiveRecord::FixtureSet.identify(:project1_namespace, :uuid) %> created_by_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> - -invalid_metadata_template: - description: "This is an invalid template." - fields: ["field_1", "field_2", "field_3"] - created_at: <%= 3.hours.ago %> - namespace_id: <%= ActiveRecord::FixtureSet.identify(:project1_namespace, :uuid) %> - created_by_id: <%= ActiveRecord::FixtureSet.identify(:john_doe, :uuid) %> diff --git a/test/models/metadata_template_test.rb b/test/models/metadata_template_test.rb index 4ceec535c0..662b4f8b28 100644 --- a/test/models/metadata_template_test.rb +++ b/test/models/metadata_template_test.rb @@ -7,7 +7,6 @@ class MetadataTemplateTest < ActiveSupport::TestCase @namespace = namespaces_user_namespaces(:john_doe_namespace) @user = users(:john_doe) @valid_metadata_template = metadata_templates(:valid_metadata_template) - @invalid_metadata_template = metadata_templates(:invalid_metadata_template) end test 'valid metadata template' do From 2aaedc1fc1c2b2717bdc59a5c223508bc8097a57 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Wed, 8 Jan 2025 14:10:48 -0600 Subject: [PATCH 7/9] feat(metadata): Refactor MetadataTemplate model to include namespace validation and associations --- app/models/metadata_template.rb | 22 ++++++++++++++++------ test/models/metadata_template_test.rb | 21 +++++++++++---------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/app/models/metadata_template.rb b/app/models/metadata_template.rb index 826b822c62..20923268d1 100644 --- a/app/models/metadata_template.rb +++ b/app/models/metadata_template.rb @@ -6,17 +6,27 @@ class MetadataTemplate < ApplicationRecord has_logidze acts_as_paranoid - broadcasts_refreshes # Associations - belongs_to :namespace belongs_to :created_by, class_name: 'User' # Validations validates :name, presence: true, uniqueness: { scope: [:namespace_id] } validates :description, length: { maximum: 1000 } - # validates :namespace_type, - # inclusion: { - # in: [Group.sti_name, Namespaces::ProjectNamespace.sti_name] - # } + + belongs_to :namespace, autosave: true + + belongs_to :group, optional: true, foreign_key: :namespace_id # rubocop:disable Rails/InverseOf + belongs_to :project_namespace, optional: true, foreign_key: :namespace_id, class_name: 'Namespaces::ProjectNamespace' # rubocop:disable Rails/InverseOf + + validate :validate_namespace + + private + + def validate_namespace + # Only Groups and Projects should have metadata templates + return if %w[Group Project].include?(namespace.type) + + errors.add(namespace.type, 'namespace cannot have metadata templates') + end end diff --git a/test/models/metadata_template_test.rb b/test/models/metadata_template_test.rb index 662b4f8b28..b6ffe7307c 100644 --- a/test/models/metadata_template_test.rb +++ b/test/models/metadata_template_test.rb @@ -62,16 +62,17 @@ class MetadataTemplateTest < ActiveSupport::TestCase end # Activity Tracking Tests - # test 'tracks activity on create' do - # template = MetadataTemplate.new( - # name: 'Activity Test', - # namespace: @namespace, - # created_by: @user - # ) - # assert_difference 'Activity.count' do - # template.save - # end - # end + test 'tracks activity on create' do + skip 'TrackActivity concern is not yet implemented' + template = MetadataTemplate.new( + name: 'Activity Test', + namespace: @namespace, + created_by: @user + ) + assert_difference 'Activity.count' do + template.save + end + end test 'tracks history changes' do @valid_metadata_template.create_logidze_snapshot! From 2918a754691687cb644cb28abeabc11f97684764 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Wed, 8 Jan 2025 14:34:13 -0600 Subject: [PATCH 8/9] test(metadata): Update namespace references in MetadataTemplate tests --- test/models/metadata_template_test.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/models/metadata_template_test.rb b/test/models/metadata_template_test.rb index b6ffe7307c..91243c5365 100644 --- a/test/models/metadata_template_test.rb +++ b/test/models/metadata_template_test.rb @@ -4,7 +4,6 @@ class MetadataTemplateTest < ActiveSupport::TestCase setup do - @namespace = namespaces_user_namespaces(:john_doe_namespace) @user = users(:john_doe) @valid_metadata_template = metadata_templates(:valid_metadata_template) end @@ -28,7 +27,7 @@ class MetadataTemplateTest < ActiveSupport::TestCase test 'valid with same name in different namespace' do duplicate = @valid_metadata_template.dup - duplicate.namespace = namespaces_user_namespaces(:jane_doe_namespace) + duplicate.namespace = groups(:subgroup_one_group_three) assert duplicate.valid? end @@ -66,7 +65,7 @@ class MetadataTemplateTest < ActiveSupport::TestCase skip 'TrackActivity concern is not yet implemented' template = MetadataTemplate.new( name: 'Activity Test', - namespace: @namespace, + namespace: project_namespace(:project1), created_by: @user ) assert_difference 'Activity.count' do From 8d83e3cde3ad1f4554d8142c860e8640aa010f27 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Thu, 9 Jan 2025 11:45:35 -0600 Subject: [PATCH 9/9] feat(metadata): Add JSON schema validation for MetadataTemplate fields and create schema file --- app/models/metadata_template.rb | 2 ++ app/models/workflow_execution.rb | 2 +- config/schemas/metadata_template_metadata.json | 10 ++++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 config/schemas/metadata_template_metadata.json diff --git a/app/models/metadata_template.rb b/app/models/metadata_template.rb index 20923268d1..6899a922a9 100644 --- a/app/models/metadata_template.rb +++ b/app/models/metadata_template.rb @@ -3,6 +3,7 @@ # MetadataTemplate represents a template for structured metadata fields class MetadataTemplate < ApplicationRecord include TrackActivity + METADATA_TEMPLATE_JSON_SCHEMA = Rails.root.join('config/schemas/metadata_template_metadata.json') has_logidze acts_as_paranoid @@ -19,6 +20,7 @@ class MetadataTemplate < ApplicationRecord belongs_to :group, optional: true, foreign_key: :namespace_id # rubocop:disable Rails/InverseOf belongs_to :project_namespace, optional: true, foreign_key: :namespace_id, class_name: 'Namespaces::ProjectNamespace' # rubocop:disable Rails/InverseOf + validates :fields, presence: true, json: { message: ->(errors) { errors }, schema: METADATA_TEMPLATE_JSON_SCHEMA } validate :validate_namespace private diff --git a/app/models/workflow_execution.rb b/app/models/workflow_execution.rb index c3663fdcee..54102bd15e 100644 --- a/app/models/workflow_execution.rb +++ b/app/models/workflow_execution.rb @@ -40,7 +40,7 @@ def send_email end def cancellable? - %w[submitted running prepared initial].include?(state) + %w[submitted running prepared initial].include?(state) end def deletable? diff --git a/config/schemas/metadata_template_metadata.json b/config/schemas/metadata_template_metadata.json new file mode 100644 index 0000000000..0d6fe2c1ff --- /dev/null +++ b/config/schemas/metadata_template_metadata.json @@ -0,0 +1,10 @@ +{ + "type": "array", + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string" + } + }