Repository navigation
[WW-2029] Fix boolean inputs and default values in database actions - #64
kevinbarfleur wants to merge 6 commits into
Conversation
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.
AlexisJC
left a comment
There was a problem hiding this comment.
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.
|
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 :
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
left a comment
There was a problem hiding this comment.
🤖 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: 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'inwwPlugin.jstypes, and a newDatabaseField.vuethat rendersonofffields as aradiowith a sentinel value for "Not set" and passes other types through unchanged.- The
setData/setPrimaryDatacleanups keepfalseand, foronofffields,null. - The watcher from eba3e50 that wrote
falsewhen a saved action was opened is gone (nobooleanDefaults.jsleft in the diff). vueadded as a devDependency, anode --testsuite 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
falseto 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 /
vuemissing:vueadded, 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/deleteinwwPlugin.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 asfalse(it used to be dropped). - Possible regressions: none found. Non-boolean fields go through
DatabaseFieldwith 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 nottrue/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.
|
Complément validation : la CI GitHub est verte sur 29b494d. Le job installe les dépendances avec |
|
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 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. |
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