Skip to content

[WW-2029] Fix boolean inputs and default values in database actions - #64

Open
kevinbarfleur wants to merge 6 commits into
mainfrom
WW-2029_boolean-input
Open

kevinbarfleur wants to merge 6 commits into
mainfrom
WW-2029_boolean-input

Conversation

@kevinbarfleur

Copy link
Copy Markdown
Contributor

boolean columns were missing from the type map so they fell back to a code block, and the falsy cleanup was stripping literal false values on update

The On/Off control displayed false for undefined fields without saving it. Initialize visible boolean arguments together, preserving configured values, bindings and bulk payloads.

Add regression coverage for Insert, Update, Upsert and boolean primary keys.
@leo91000 leo91000 changed the title WW-2029 Render boolean columns as on/off toggle in database action forms [WW-2029] Fix boolean inputs and default values in database actions Sep 28, 2026

@AlexisJC AlexisJC left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor. The tests import vue, which isn't in devDependencies (only reachable through @weweb/cli 'latest'), and no CI runs them, so nothing guarantees they pass. Adding vue to devDependencies and a test script, even manual, would make them reliable.

@leo91000

Copy link
Copy Markdown
Contributor

Reprise après review — décision validée avec Leo : un booléen absent reste absent, pour préserver les valeurs par défaut Supabase et les mises à jour partielles.

Corrigé dans 28beae8 :

  • Suppression du watcher qui écrivait false à l'ouverture d'une action enregistrée, y compris sur les clés primaires.
  • Le champ affiche maintenant explicitement « Not set », « On », « Off » ou « Null ». Choisir Off enregistre bien false; Not set retire le champ du payload. Les bindings restent conservés.
  • Insert/Upsert conservent un null explicitement choisi; Update n'ajoute aucun autre booléen. Les modes multiples ne sont pas modifiés.
  • Vue est déclarée directement pour les tests et une CI npm ci / npm test est ajoutée.

Validation : installation propre avec npm ci, 17 tests de non-régression réussis, compilation du plugin et des cinq composants Vue, revue indépendante du diff complet sans anomalie. Cette validation couvre aussi le comportement des watchers des vrais inputs Editor. Pas de test Supabase distant effectué.

PR : #64

@WeWeb-AI-Agent WeWeb-AI-Agent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 Automated review by WeWeb AI Agent (ticket moved to To Be Reviewed).

WW-2029 · [PLUGIN] [SUPABASE] Fix boolean type input

Ticket · PR #64 · looms in the ticket (not watched)
kevinbarfleur + leo91000 · base main · +884 / −220 (≈290 without package-lock.json) · 10 files · CI ❌ (npm ci fails) · mergeable ✅ (3 commits behind main, no net file change)

Verdict: ⚠️ Risk to settle (the product fix looks right and addresses every earlier review point, but the CI workflow added by this PR fails at npm ci, so the new tests never run)

What it does

In the Supabase plugin's Insert, Update, Upsert and Delete actions, boolean columns had no entry in the plugin's type map and were shown as a code block. They now get a 4-choice radio (Not set / On / Off / Null). Off really saves false (it used to be dropped by the falsy cleanup), Null is kept, and Not set leaves the column out of the payload, so Supabase defaults and partial updates still apply. This matches the decision written in the ticket (comment of 16 Oct: an omitted boolean stays omitted).

How

  • boolean: 'onoff' in wwPlugin.js types, and a new DatabaseField.vue that renders onoff fields as a radio with a sentinel value for "Not set" and passes other types through unchanged.
  • The setData / setPrimaryData cleanups keep false and, for onoff fields, null.
  • The watcher from eba3e50 that wrote false when a saved action was opened is gone (no booleanDefaults.js left in the diff).
  • vue added as a devDependency, a node --test suite mounting the real forms, and a GitHub Actions workflow.

Earlier review (AlexisJC on eba3e50), checked against 28beae8:

  • Saved actions rewritten on open: addressed (watcher removed; a test asserts nothing is emitted on mount).
  • Update "All fields" writing false to every boolean: addressed (missing booleans stay omitted).
  • Insert/Upsert ignoring column defaults and nullable booleans: addressed (Not set omits the column, Null is kept).
  • Boolean primary keys prefilled to false: addressed (nothing is prefilled).
  • Tests not run / vue missing: vue added, but the new CI fails (see the first finding).

I checked the editor side on weweb origin/main. With type="radio", wwEditorInput.vue does not emit when the computed value matches the model (no write when the form mounts), and RadioGroup.vue uses strict equality, so null, true, false and the sentinel each select the right choice.

Findings

Important. The Tests workflow added here fails on its first run (run 38898808279): npm ci stops when node-gyp builds node-sass 8.0.0 (pulled in by @weweb/cli 3.8.0), which does not support Node 22. So npm test never runs, and once merged every PR and push to main gets a red check. Running the job on Node 18 (node-version: 18, which the test file comments target), or npm ci --ignore-scripts since the tests don't need node-sass, would likely fix it (not tried). Unless the failing check is accepted for now? tests.yml#L13

Minor. In Delete and Update by primary key, a boolean key now offers a "Null" choice, and setPrimaryData keeps that null (the onoff exception). The runtime then runs query.match({ key: null }), which would filter on key=eq.null (not run). A primary key can never be null, so the action can only fail or match nothing, whereas before the null was dropped and the user got "No primary key defined.". Hiding the Null choice for primary keys, or dropping the onoff null exception in setPrimaryData, would avoid that. Delete.vue#L175, Update.vue#L212

Published sites & regressions

  • Published sites: not affected by the plugin update alone. The runtime (insert/update/upsert/delete in wwPlugin.js) is unchanged and saved arguments are only rewritten when a user edits the action. After such an edit plus a publish, an Off boolean is sent as false (it used to be dropped).
  • Possible regressions: none found. Non-boolean fields go through DatabaseField with the same props. The placeholder now shows falsy defaults too (!== undefined), and primary key fields show "Default: …" instead of "Enter a value" (cosmetic). Saved boolean values that are not true/false/null (e.g. a string typed in the old code block) select no choice but are still sent unchanged.

Legacy

  • Created: none · Reused: the plugin itself is legacy (fixes only), and this change is a fix.

Conflicts with work in flight

  • none

Questions for the developer

  • Was the test suite run with Node 18 locally? The test file mentions Node 18, the workflow uses 22.
  • Should "Null" stay available for non-nullable boolean columns on Insert, or is a Supabase error considered enough feedback there?

Reviewed without the full product context: answer rather than change anything already agreed.
Not checked: looms not watched, plugin not run in an editor, no call to a real Supabase project.

Comment thread .github/workflows/tests.yml Resolved
@leo91000

Copy link
Copy Markdown
Contributor

Complément validation : la CI GitHub est verte sur 29b494d. Le job installe les dépendances avec npm ci --ignore-scripts, car les tests Vue n'utilisent pas le build natif node-sass de l'ancien CLI. Les 17 tests passent en CI ; une deuxième revue indépendante du diff complet ne relève aucune anomalie.

@leo91000

Copy link
Copy Markdown
Contributor

Dernier follow-up traité dans cfa4511 : le choix Null est retiré pour les clés primaires booléennes dans les quatre formulaires ; une clé primaire explicite false reste bien conservée. Insert/Upsert exposent désormais les métadonnées de clé primaire au champ partagé. Les valeurs null restent disponibles pour les autres champs booléens.

22 tests passent, y compris les cas de clé primaire et une vérification indépendante des vrais composants Radio/Input/Row de l'éditeur. Quatrième revue indépendante du diff complet : aucune anomalie. La correction CI précédente est également vérifiée verte sur GitHub.

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.

4 participants