Skip to content

fix(db): define clan_members before applying data migrations (#82) - #92

Closed
Rodrigoue9 wants to merge 3 commits into
Bitcoindefi:mainfrom
Rodrigoue9:fix/bounty-82
Closed

fix(db): define clan_members before applying data migrations (#82)#92
Rodrigoue9 wants to merge 3 commits into
Bitcoindefi:mainfrom
Rodrigoue9:fix/bounty-82

Conversation

@Rodrigoue9

Copy link
Copy Markdown

Title

fix(db): define clan_members and indices before applying data migrations in schema.sql (#82)

Description

  • Moves the definition of clan_members, its unique constraints, and associated indices prior to the data migration CTEs in api/schema.sql.
  • Adds \set ON_ERROR_STOP on to api/schema.sql so that any future schema ordering or syntax errors fail fast during CI rather than silently succeeding.
  • Ensures a clean PostgreSQL initialization completes without relation not found errors.

Closes #82

Comment thread api/schema.sql Outdated
Comment thread PR_DESCRIPTION_DRAFT.md
Comment on lines +1 to +10
# feat(world-builder): register uploaded graphics and extend palette schemas (#6)

## Summary
Resolves #6 by providing `paletteEntrySchema` and `validatePaletteEntry` to validate multi-layer palette definitions, enforce non-colliding graphic index allocations (`UPLOADED_GRAPHIC_INDEX_START = 1_000_000`), and verify graphic existence across engine and uploaded assets.

### Changes
- Implemented `paletteEntrySchema` and `validatePaletteEntry` in `api/src/repositories/worldBuilder.ts`.
- Added unit tests in `api/src/repositories/__tests__/paletteValidation.test.ts`.

Closes #6

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Quality: Committed draft file describes unrelated PR content

PR_DESCRIPTION_DRAFT.md was added but its content describes a world-builder palette feature (#6), which is unrelated to this PR's stated purpose of reordering clan_members schema definitions before data migrations (#82). This looks like an accidentally committed scratch/draft file. Remove it from the branch so it isn't merged into main.

Was this helpful? React with 👍 / 👎

@gitar-bot

gitar-bot Bot commented Aug 21, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 1 resolved / 2 findings

Defines clan_members before data migrations and adds error-stop directives in schema.sql to fix initialization errors, though the committed draft file describes unrelated PR content.

💡 Quality: Committed draft file describes unrelated PR content

📄 PR_DESCRIPTION_DRAFT.md:1-10

PR_DESCRIPTION_DRAFT.md was added but its content describes a world-builder palette feature (#6), which is unrelated to this PR's stated purpose of reordering clan_members schema definitions before data migrations (#82). This looks like an accidentally committed scratch/draft file. Remove it from the branch so it isn't merged into main.

✅ 1 resolved
Bug: psql \set meta-command breaks node-postgres migration

📄 api/schema.sql:1
schema.sql is applied two ways: via psql (docker-entrypoint-initdb.d) and via pool.query(schemaSql) in api/src/migrate.ts, which the start script (pnpm run migrate && node dist/server.js) runs on every API container startup. \set ON_ERROR_STOP on is a psql client meta-command, not SQL. node-postgres sends the whole file over the simple-query protocol, so Postgres parses line 1 and throws a syntax error at or near the backslash, causing migrate to exit(1) and the API to never start. node-postgres already aborts a multi-statement query on the first error and rejects the promise, so this directive adds no benefit on that path. Remove the line (or set ON_ERROR_STOP only for the psql/initdb path).

🤖 Prompt for agents
Code Review: Defines clan_members before data migrations and adds error-stop directives in schema.sql to fix initialization errors, though the committed draft file describes unrelated PR content.

1. 💡 Quality: Committed draft file describes unrelated PR content
   Files: PR_DESCRIPTION_DRAFT.md:1-10

   PR_DESCRIPTION_DRAFT.md was added but its content describes a world-builder palette feature (#6), which is unrelated to this PR's stated purpose of reordering clan_members schema definitions before data migrations (#82). This looks like an accidentally committed scratch/draft file. Remove it from the branch so it isn't merged into main.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@leocagli

Copy link
Copy Markdown
Collaborator

Cierro esta porque la #82 ya quedo resuelta en main. El diagnostico era correcto y el arreglo tambien.

Que paso

La #82 junto cuatro PRs de cuatro personas: #92, #99, #113 y #117. Las cuatro mueven la definicion de clan_members (y de clan_requests, y la columna characters.clan_id) antes de las migraciones que las referencian. Ninguna estaba mal.

Se mergeo #114, de @trexfr-ops, porque:

  1. La issue estaba asignada a esa persona. Las asignaciones se respetan.
  2. Resolvia dos issues de una. fix(api): handle nulls in ranking query and log error stack trace (closes #84) #114 trae el mismo cambio de schema.sql que fix(db): move clan_members table definition before data migrations (closes #82) #113 mas el arreglo del ranking de la El endpoint de ranking devuelve 500 en un entorno limpio #84: el COALESCE sobre los contadores de bajas nulos que hacia devolver 500 al endpoint.
  3. Envuelve la migracion en una transaccion. migrate.ts corria schema.sql y dos indices sueltos sin BEGIN; si algo fallaba a mitad, la base quedaba a medio migrar. Ahora hay BEGIN/COMMIT con ROLLBACK en el error.

Contexto que no se veia desde afuera

Los PRs que vienen de un fork quedan en action_required y el workflow no corre hasta que un maintainer lo aprueba. Nadie lo estaba aprobando, asi que el CI de esta PR nunca dijo nada.

Y main estaba en rojo desde el dia que se agrego el workflow, por dos archivos de seed que faltaban en el repositorio. Se arreglo hoy (#109), asi que a partir de ahora el CI sirve de verdad.

Gracias por el laburo. Si queres seguir, hay issues abiertas sin asignar: comenta con un plan concreto y te la asigno.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

schema.sql aplica tres migraciones sobre clan_members antes de crear la tabla

2 participants