From 4f218b96062df08283a3d63d140495734f66bba2 Mon Sep 17 00:00:00 2001 From: Risto Virtaharju Date: Wed, 26 Aug 2026 17:53:02 +0000 Subject: [PATCH] fix: delete omitted quotas and questions when the same edit adds new ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admin form sends kept rows with ids and new rows with a null id. update_event_quotas and update_event_questions detect removals with `id not in (select id from unnest(input))`, and NOT IN never evaluates true against a set containing null — so one new row in the input silently disabled deletion for the whole call. Exclude nulls from the comparison set. Co-Authored-By: Claude Fable 5 --- .../functions/update_event_questions.test.ts | 61 ++++++++++ .../functions/update_event_quotas.test.ts | 54 +++++++++ .../2-delete-omitted-quotas-and-questions.sql | 111 ++++++++++++++++++ 3 files changed, 226 insertions(+) create mode 100644 @app/db/__tests__/app_public/functions/update_event_questions.test.ts create mode 100644 @app/db/__tests__/app_public/functions/update_event_quotas.test.ts create mode 100644 @app/db/migrations/current/2-delete-omitted-quotas-and-questions.sql diff --git a/@app/db/__tests__/app_public/functions/update_event_questions.test.ts b/@app/db/__tests__/app_public/functions/update_event_questions.test.ts new file mode 100644 index 00000000..34330670 --- /dev/null +++ b/@app/db/__tests__/app_public/functions/update_event_questions.test.ts @@ -0,0 +1,61 @@ +import { + createEventCategories, + createEvents, + createOrganizations, + createQuestions, + withAdminUserDb, +} from "../../helpers" + +describe("Test app_public.update_event_questions function", () => { + it("deletes an omitted question also when the same call adds a new question", () => + withAdminUserDb(async (client) => { + const [organization] = await createOrganizations(client, 1) + const [eventCategory] = await createEventCategories( + client, + 1, + organization.id + ) + const [event] = await createEvents( + client, + 1, + organization.id, + eventCategory.id + ) + const [keptQuestion, removedQuestion] = await createQuestions( + client, + 2, + event.id, + false, + "TEXT" as any + ) + + // The admin UI sends kept questions with their ids and newly added + // questions without an id. removedQuestion is omitted, so it should + // be deleted. + await client.query( + `select app_public.update_event_questions( + $1, + array[ + row($2::uuid, 0::smallint, 'TEXT'::app_public.question_type, $3::app_public.translated_field, false, null), + row(null, 1::smallint, 'TEXT'::app_public.question_type, $4::app_public.translated_field, false, null) + ]::app_public.update_event_questions[] + )`, + [ + event.id, + keptQuestion.id, + keptQuestion.label, + { fi: "Uusi kysymys", en: "New question" }, + ] + ) + + const { rows: questions } = await client.query( + `select id from app_public.event_questions where event_id = $1 order by position`, + [event.id] + ) + const questionIds = questions.map((q) => q.id) + + expect(questionIds).toHaveLength(2) + expect(questionIds).toContain(keptQuestion.id) + expect(questionIds).not.toContain(removedQuestion.id) + })) +}) diff --git a/@app/db/__tests__/app_public/functions/update_event_quotas.test.ts b/@app/db/__tests__/app_public/functions/update_event_quotas.test.ts new file mode 100644 index 00000000..ca0a3967 --- /dev/null +++ b/@app/db/__tests__/app_public/functions/update_event_quotas.test.ts @@ -0,0 +1,54 @@ +import { + createEventCategories, + createEvents, + createOrganizations, + createQuotas, + withAdminUserDb, +} from "../../helpers" + +describe("Test app_public.update_event_quotas function", () => { + it("deletes an omitted quota also when the same call adds a new quota", () => + withAdminUserDb(async (client) => { + const [organization] = await createOrganizations(client, 1) + const [eventCategory] = await createEventCategories( + client, + 1, + organization.id + ) + const [event] = await createEvents( + client, + 1, + organization.id, + eventCategory.id + ) + const [keptQuota, removedQuota] = await createQuotas(client, 2, event.id) + + // The admin UI sends kept quotas with their ids and newly added quotas + // without an id. removedQuota is omitted, so it should be deleted. + await client.query( + `select app_public.update_event_quotas( + $1, + array[ + row($2::uuid, 0::smallint, $3::jsonb, 5::smallint), + row(null, 1::smallint, $4::jsonb, 10::smallint) + ]::app_public.update_event_quotas[] + )`, + [ + event.id, + keptQuota.id, + keptQuota.title, + { fi: "Uusi kiintiö", en: "New quota" }, + ] + ) + + const { rows: quotas } = await client.query( + `select id from app_public.quotas where event_id = $1 order by position`, + [event.id] + ) + const quotaIds = quotas.map((q) => q.id) + + expect(quotaIds).toHaveLength(2) + expect(quotaIds).toContain(keptQuota.id) + expect(quotaIds).not.toContain(removedQuota.id) + })) +}) diff --git a/@app/db/migrations/current/2-delete-omitted-quotas-and-questions.sql b/@app/db/migrations/current/2-delete-omitted-quotas-and-questions.sql new file mode 100644 index 00000000..220ed34d --- /dev/null +++ b/@app/db/migrations/current/2-delete-omitted-quotas-and-questions.sql @@ -0,0 +1,111 @@ +/* + * The admin UI sends an event's kept quotas with their ids and newly added + * quotas with a null id. Deletion of omitted quotas is detected with + * `q.id not in (select id from unnest(quotas))`, and in SQL `NOT IN` never + * evaluates true when the compared set contains a null — so a single null id + * in the input disables deletion for the whole call. Excluding nulls from the + * comparison set keeps the three-valued logic out of the way: kept ids still + * match, and omitted quotas are deleted regardless of whether the same edit + * also adds new quotas. + */ + +create or replace function app_public.update_event_quotas( + event_id uuid, + quotas app_public.update_event_quotas[] +) +returns app_public.quotas[] as $$ +#variable_conflict use_variable +declare + v_quota_ids_to_delete uuid[]; + v_input app_public.update_event_quotas; + v_quota app_public.quotas; + v_ret app_public.quotas[] default '{}'; +begin + -- Check permissions + call app_public.check_is_admin(); + + -- Must specify at least one quota + if (select array_length(quotas, 1)) is null then + raise exception 'You must specify at least one quota' using errcode = 'DNIED'; + end if; + + select array( + select id from app_public.quotas as q + where q.event_id = event_id + and q.id not in (select id from unnest(quotas) where id is not null) + ) + into v_quota_ids_to_delete; + + -- Delete existing event quotas that were not supplied + -- as input to this function + delete from app_public.quotas as q + where q.id = any(v_quota_ids_to_delete); + + foreach v_input in array quotas loop + if exists(select 1 from app_public.quotas where id = v_input.id) then + -- Update existing event quotas by id + update app_public.quotas + set position = v_input.position, title = v_input.title, size = v_input.size + where id = v_input.id + returning * into v_quota; + else + -- Create new quotas that didn't exist before + insert into app_public.quotas(event_id, position, title, size) + values (event_id, v_input.position, v_input.title, v_input.size) + returning * into v_quota; + end if; + + v_ret := array_append(v_ret, v_quota); + end loop; + + return v_ret; +end; +$$ language plpgsql volatile security invoker set search_path = pg_catalog, public, pg_temp; + +create or replace function app_public.update_event_questions( + event_id uuid, + questions app_public.update_event_questions[] +) +returns app_public.event_questions[] as $$ +#variable_conflict use_variable +declare + v_question_ids_to_delete uuid[]; + v_input app_public.update_event_questions; + v_question app_public.event_questions; + v_ret app_public.event_questions[] default '{}'; +begin + -- Check permissions + call app_public.check_is_admin(); + + select array( + select id from app_public.event_questions as q + where q.event_id = event_id + and q.id not in (select id from unnest(questions) where id is not null) + ) + into v_question_ids_to_delete; + + -- Delete existing event questions that were not supplied + -- as input to this function + delete from app_public.event_questions as q + where q.id = any(v_question_ids_to_delete); + + foreach v_input in array questions loop + if exists(select 1 from app_public.event_questions where id = v_input.id) then + -- Update existing event questions by id + update app_public.event_questions + set position = v_input.position, type = v_input.type, label = v_input.label, is_required = v_input.is_required, data = v_input.data + where id = v_input.id + returning * into v_question; + else + -- Create new questions that didn't exist before + insert into app_public.event_questions(event_id, position, type, label, is_required, data) + values (event_id, v_input.position, v_input.type, v_input.label, v_input.is_required, v_input.data) + returning * into v_question; + end if; + + v_ret := array_append(v_ret, v_question); + end loop; + + return v_ret; +end; +$$ language plpgsql volatile security invoker set search_path = pg_catalog, public, pg_temp;