Skip to content

Commit d6a1d93

Browse files
committed
fix: address CodeRabbit review comments on PR #321
- Add missing `query` import from `$api/rumble` in amendment.ts, operativeClauseVote.ts, and resolutionPaper.ts (build failures) - Guard `window.location.reload()` with `browser` check in client.ts (SSR crash) - Fix GraphQL injection in sendBeacon by using variables instead of string interpolation in both participant and chair paper pages - Replace weak 6-char share code with nanoid helper per repo conventions - Fix final vote guard to only accept VOTING_PHASE (was also accepting AMENDMENT_PHASE, allowing voting phase to be skipped) - Add paper phase check to adoptByConsensus and acceptAmendment to prevent amendment application after paper leaves AMENDMENT_PHASE - Fix ALTER_POSITION off-by-one: align with "insert after" convention used by ADD, and allow targetPosition of -1 for beginning - Fix restored amendments reset to SUBMITTED instead of PENDING so they remain visible and actionable after revert - Fix subscription cleanup in participant papers list (websocket leak) - Fix i18n: "Stimmung" -> "Abstimmung" in de.json, "notes" -> "votes" and "two-thrids" -> "two-thirds" in en.json https://claude.ai/code/session_013PjiR7TYfuzeoqmy6zRPT9
1 parent ae83960 commit d6a1d93

File tree

10 files changed

+56
-23
lines changed

10 files changed

+56
-23
lines changed

messages/de.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@
364364
"publicComment": "Öffentlich",
365365
"publish": "Veröffentlichen",
366366
"publishChanges": "Änderungen Veröffentlichen",
367-
"recordVoteFromVoting": "Stimmung erfassen",
367+
"recordVoteFromVoting": "Abstimmung erfassen",
368368
"redeemShareCode": "Freigabecode einlösen",
369369
"redo": "Wiederholen",
370370
"regionalGroup_africa": "Afrika",
@@ -563,7 +563,7 @@
563563
"unassigned": "Nicht zugewiesen",
564564
"underline": "Unterstrichen",
565565
"undo": "Rückgängig",
566-
"undoVote": "Stimmung rückgängig",
566+
"undoVote": "Abstimmung rückgängig",
567567
"unknown": "unbekannt",
568568
"unrecognizedCodes": "Nicht erkannte Codes:",
569569
"until": "bis {time} Uhr",

messages/en.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@
488488
"short_sleek_snake_hint": "Committee",
489489
"showOfHandsVoting": "Vote by Show of Hands",
490490
"simpleMajority": "Simple",
491-
"simpleMajorityTooltip": "Needed notes for simple majority",
491+
"simpleMajorityTooltip": "Needed votes for simple majority",
492492
"speaker": "Speaker",
493493
"speakersList": "General Speakers' List",
494494
"speakersListNamePlaceholder": "New name...",
@@ -556,7 +556,7 @@
556556
"topCandidate": "Top Candidate",
557557
"totalCountriesPresent": "Count of Present Countries",
558558
"twoThirdsMajority": "Two-thirds",
559-
"twoThirdsMajorityTooltip": "Needed votes for two-thrids majority",
559+
"twoThirdsMajorityTooltip": "Needed votes for two-thirds majority",
560560
"typeOfVoting": "Type of Vote",
561561
"unActor": "UN Actor",
562562
"unActors": "UN Actors",

src/api/handlers/amendment.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { db, schema } from '$api/db/db';
2-
import { abilityBuilder, enum_, schemaBuilder, pubsub as rumblePubsub } from '$api/rumble';
2+
import { abilityBuilder, enum_, query, schemaBuilder, pubsub as rumblePubsub } from '$api/rumble';
33
import { basics } from './basics';
44
import { isGlobalAdmin } from '$api/services/isAdminEmail';
55
import { assertFindFirstExists, assertFirstEntryExists } from '@m1212e/rumble';
@@ -193,19 +193,28 @@ async function applyAmendmentToResolution(
193193
case 'ALTER_POSITION': {
194194
// Resolve current index from stable clause ID
195195
const sourceIdx = findClauseIndex(resolution.operative, amendment.targetClauseId!);
196-
const destIdx = amendment.targetPosition!;
197-
if (destIdx < 0 || destIdx > resolution.operative.length) {
196+
const targetPosition = amendment.targetPosition!;
197+
// targetPosition follows the "insert after" convention: -1 means beginning
198+
if (targetPosition < -1 || targetPosition >= resolution.operative.length) {
198199
throw new GraphQLError('Destination index out of range');
199200
}
200201
const [clause] = resolution.operative.splice(sourceIdx, 1);
201202
// After removing from source, the target index might shift
202-
const adjustedDest = destIdx > sourceIdx ? destIdx - 1 : destIdx;
203-
resolution.operative.splice(adjustedDest, 0, clause);
203+
const adjustedTargetPosition =
204+
targetPosition >= sourceIdx ? targetPosition - 1 : targetPosition;
205+
resolution.operative.splice(adjustedTargetPosition + 1, 0, clause);
204206
// Adjust other pending amendments' targetPosition for the structural shift
205207
// First: source removal shifts indices down
206208
await adjustPendingPositions(tx, paper.id, amendment.id, 'decrement', sourceIdx, 'gt');
207209
// Then: destination insertion shifts indices up
208-
await adjustPendingPositions(tx, paper.id, amendment.id, 'increment', adjustedDest, 'gte');
210+
await adjustPendingPositions(
211+
tx,
212+
paper.id,
213+
amendment.id,
214+
'increment',
215+
adjustedTargetPosition + 1,
216+
'gte'
217+
);
209218
break;
210219
}
211220
}
@@ -562,6 +571,10 @@ schemaBuilder.mutationFields((t) => ({
562571
.findFirst({ where: { id: amendment.paperId } })
563572
.then(assertFindFirstExists);
564573

574+
if (paper.status !== 'AMENDMENT_PHASE') {
575+
throw new GraphQLError('Paper must be in AMENDMENT_PHASE to adopt amendments');
576+
}
577+
565578
await assertCommitteeChairOrAdmin(ctx, paper.committeeId);
566579

567580
await db.transaction(async (tx) => {
@@ -606,6 +619,10 @@ schemaBuilder.mutationFields((t) => ({
606619
.findFirst({ where: { id: amendment.paperId } })
607620
.then(assertFindFirstExists);
608621

622+
if (paper.status !== 'AMENDMENT_PHASE') {
623+
throw new GraphQLError('Paper must be in AMENDMENT_PHASE to accept amendments');
624+
}
625+
609626
await assertCommitteeChairOrAdmin(ctx, paper.committeeId);
610627

611628
await db.transaction(async (tx) => {

src/api/handlers/operativeClauseVote.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { db, schema } from '$api/db/db';
2-
import { abilityBuilder, enum_, schemaBuilder, pubsub as rumblePubsub } from '$api/rumble';
2+
import { abilityBuilder, enum_, query, schemaBuilder, pubsub as rumblePubsub } from '$api/rumble';
33
import { basics } from './basics';
44
import { isGlobalAdmin } from '$api/services/isAdminEmail';
55
import { assertCommitteeChairOrAdmin } from './resolutionPaper';

src/api/handlers/paperShareCode.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import { basics } from './basics';
55
import { isGlobalAdmin } from '$api/services/isAdminEmail';
66
import { assertFindFirstExists, assertFirstEntryExists } from '@m1212e/rumble';
77
import { GraphQLError } from 'graphql';
8-
import { customAlphabet } from 'nanoid';
8+
import { nanoid } from '$lib/helpers/nanoid';
99

10-
const generateShareCode = customAlphabet('ABCDEFGHJKLMNPQRSTUVWXYZ23456789', 6);
10+
const generateShareCode = () => nanoid();
1111

1212
const { arg, ref, pubsub, table } = basics('paperShareCode');
1313
const paperPubsub = rumblePubsub({ table: 'resolutionPaper' });

src/api/handlers/resolutionPaper.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { db, schema } from '$api/db/db';
2-
import { abilityBuilder, enum_, schemaBuilder, pubsub as rumblePubsub } from '$api/rumble';
2+
import { abilityBuilder, enum_, query, schemaBuilder, pubsub as rumblePubsub } from '$api/rumble';
33
import { and, eq, isNull, count as drizzleCount, desc, inArray } from 'drizzle-orm';
44
import { basics } from './basics';
55
import { isGlobalAdmin } from '$api/services/isAdminEmail';
@@ -660,7 +660,7 @@ schemaBuilder.mutationFields((t) => ({
660660
})
661661
.then(assertFindFirstExists);
662662

663-
if (paper.status !== 'VOTING_PHASE' && paper.status !== 'AMENDMENT_PHASE') {
663+
if (paper.status !== 'VOTING_PHASE') {
664664
throw new GraphQLError('Paper must be in VOTING_PHASE to record final vote');
665665
}
666666

@@ -855,10 +855,10 @@ schemaBuilder.mutationFields((t) => ({
855855
.set({ content: snapshot.content })
856856
.where(eq(schema.resolutionPaper.id, args.paperId));
857857
}
858-
// Reset applied amendments back to PENDING
858+
// Reset applied amendments back to SUBMITTED so they are visible and actionable
859859
await tx
860860
.update(schema.amendment)
861-
.set({ status: 'PENDING' })
861+
.set({ status: 'SUBMITTED' })
862862
.where(
863863
and(
864864
eq(schema.amendment.paperId, args.paperId),

src/client.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import toast from 'svelte-french-toast';
33
import { error } from '@sveltejs/kit';
44
import { subscription } from '$houdini/plugins';
55
import { createClient } from 'graphql-sse';
6+
import { browser } from '$app/environment';
67

78
let redirecting = false;
89

@@ -11,7 +12,9 @@ const authRedirect: ClientPlugin = () => ({
1112
if (!redirecting && value.errors?.some((e) => e.message === 'Must be logged in')) {
1213
console.warn('[auth] Session expired, redirecting to login...');
1314
redirecting = true;
14-
window.location.reload();
15+
if (browser) {
16+
window.location.reload();
17+
}
1518
}
1619
resolve(ctx);
1720
}

src/routes/app/[conferenceId]/[committeeId]/(chairs)/resolutions/[paperId]/+page.svelte

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,8 @@
152152
// Best-effort lock release on tab close
153153
const handleBeforeUnload = () => {
154154
const body = JSON.stringify({
155-
query: `mutation { releaseAllMyLocks(paperId: "${page.params.paperId}") }`
155+
query: 'mutation ReleaseAllMyLocks($paperId: ID!) { releaseAllMyLocks(paperId: $paperId) }',
156+
variables: { paperId: page.params.paperId }
156157
});
157158
navigator.sendBeacon('/api/graphql', new Blob([body], { type: 'application/json' }));
158159
};

src/routes/app/[conferenceId]/participant/[committeeId]/papers/+page.svelte

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import { goto } from '$app/navigation';
55
import type { PageData } from './$houdini';
66
import { graphql } from '$houdini';
7-
import { onMount } from 'svelte';
7+
import { onMount, onDestroy } from 'svelte';
88
import { ParticipantPapersSubscription } from './papersSubscription';
99
import { ParticipantCommitteeSubscription } from '../committeeSubscription';
1010
import { generatePaperName } from '$lib/utils/paperNameGenerator';
@@ -53,11 +53,21 @@
5353
)
5454
);
5555
56+
let paperUnsubscribe: (() => void) | undefined;
57+
let committeeUnsubscribe: (() => void) | undefined;
58+
5659
onMount(() => {
57-
ParticipantPapersSubscription.listen({
60+
paperUnsubscribe = ParticipantPapersSubscription.listen({
5861
committeeId: page.params.committeeId!
5962
});
60-
ParticipantCommitteeSubscription.listen({ id: page.params.committeeId! });
63+
committeeUnsubscribe = ParticipantCommitteeSubscription.listen({
64+
id: page.params.committeeId!
65+
});
66+
});
67+
68+
onDestroy(() => {
69+
paperUnsubscribe?.();
70+
committeeUnsubscribe?.();
6171
});
6272
6373
// Create paper mutation

src/routes/app/[conferenceId]/participant/[committeeId]/papers/[paperId]/+page.svelte

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,9 @@
166166
const handleBeforeUnload = () => {
167167
if (canEdit) {
168168
const body = JSON.stringify({
169-
query: `mutation { releaseAllMyLocks(paperId: "${page.params.paperId}") }`
169+
query:
170+
'mutation ReleaseAllMyLocks($paperId: ID!) { releaseAllMyLocks(paperId: $paperId) }',
171+
variables: { paperId: page.params.paperId }
170172
});
171173
navigator.sendBeacon('/api/graphql', new Blob([body], { type: 'application/json' }));
172174
}

0 commit comments

Comments
 (0)