Skip to content

[HELD for deploy window] fix(onboarding): key answers on uid so a failed step cannot wedge the wizard (#33 isolated) #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
EddyOne81 wants to merge 1 commit into
base: preview
Choose a base branch
from
Open

[HELD for deploy window] fix(onboarding): key answers on uid so a failed step cannot wedge the wizard (#33 isolated) #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 41 additions & 21 deletions acl/onboarding.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,145 +5,165 @@
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"save_signup_info": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Pre-auth signup step: by definition there is no user yet."
},
"save_usage_plan": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"save_industry": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"save_role": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"save_team_size": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"save_intent": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"save_challenges": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"save_tools": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"save_privacy": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"get_response": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"check_completion": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"mark_complete": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"get_countries": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Static reference list used by the signup country picker."
},
"reset": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"update_profile": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"get_env": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Static client config (db_name / xlink). No user data."
},
"get_onboarding_invite_link": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"send_onboarding_invites": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
},
"log": true
"log": true,
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
},
"get_activation_status": {
"scope": "hub",
"permission": {
"src": "anonymous",
"fast_check": "public-api"
}
},
"doc": "Reachability only - authentication is enforced in service/onboarding.js by _identity(), which rejects anonymous callers (ID_NOBODY) with 401 and keys every row on the caller's uid. Do NOT set src:owner here: onboarding requests carry no hub_id, so the ACL resolves them against the endpoint's own hub, and a user in onboarding never owns that hub - it denies every call with PERMISSION_DENIED. (contact.invite can use src:owner only because it passes hub_id: Visitor.id, i.e. the caller's own hub.)"
}
},
"modules": {
"private": "service/onboarding",
"public": "service/onboarding"
}
}
}
56 changes: 56 additions & 0 deletions schemas/migrations/alter_onboarding_responses_identity.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
-- File: loby/schemas/migrations/alter_onboarding_responses_identity.sql
--
-- Additive, idempotent. Safe to run repeatedly and on any v2 instance.
--
-- WHY
-- ---
-- `session_id` was the only write key on onboarding_responses. A session is a
-- transient artefact (it rotates on re-login, token refresh and expiry), so
-- keying durable survey answers on it means the answers are lost the moment
-- the session changes: every UPDATE-only step procedure matched zero rows and
-- raised "Onboarding session not found. Start at step 1."
--
-- `uid` gives the row a STABLE owner. session_id is kept as-is (still UNIQUE,
-- still the lookup key for legacy/anonymous rows) so nothing that reads this
-- table today has to change; uid is simply a second, durable way in. See
-- procedures/onboarding_resolve_row.sql for the resolution order.
--
-- `tools_other` completes the "Other -> type your own" model. industry and
-- role already store their custom text in dedicated *_other columns; tools
-- was the odd one out, splicing the raw user string into the current_tools
-- JSON array where it was indistinguishable from a canonical key. See
-- migrations/backfill_tools_other.sql for the legacy data fix-up.

ALTER TABLE `onboarding_responses`
ADD COLUMN IF NOT EXISTS `uid` VARCHAR(16) CHARACTER SET ascii COLLATE ascii_general_ci NULL
COMMENT 'Stable owner (yp.drumate.id). Survives session rotation.'
AFTER `session_id`,
ADD COLUMN IF NOT EXISTS `tools_other` VARCHAR(255) NULL
COMMENT 'Free-text value when current_tools contains "other"'
AFTER `current_tools`;

-- Non-unique on purpose: a user may legitimately hold more than one row
-- (legacy anonymous row + current one). onboarding_resolve_row picks the most
-- recently touched, so this must not be a UNIQUE constraint.
ALTER TABLE `onboarding_responses`
ADD INDEX IF NOT EXISTS `idx_uid` (`uid`);

-- Repair pre-existing schema drift: `lastname` must be nullable.
--
-- The table definition in tables/onboarding_responses.sql has declared this
-- column NULL since the v2 rework (it is collected at signup, not by the
-- wizard), but instances created from the v1 definition still carry
-- NOT NULL and alter_onboarding_responses_v2.sql never relaxed it. Found on
-- stage, where the column is NOT NULL with no default.
--
-- Under STRICT_TRANS_TABLES — which is the server default here — that makes
-- ANY insert that does not name `lastname` fail outright with
-- "Field 'lastname' doesn't have a default value". That breaks
-- onboarding_resolve_row's stub insert, and it equally breaks the v2 wizard's
-- own step 1, which posts firstname only and stores NULL for lastname.
--
-- Widening NOT NULL -> NULL cannot lose data, and MODIFY is idempotent: on an
-- instance that is already correct this is a no-op. It must run BEFORE the
-- procedures, which the manifest guarantees.
ALTER TABLE `onboarding_responses`
MODIFY COLUMN `lastname` VARCHAR(128) NULL;
105 changes: 105 additions & 0 deletions schemas/migrations/backfill_tools_other.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
-- File: loby/schemas/migrations/backfill_tools_other.sql
--
-- One-shot, idempotent data migration. Requires alter_onboarding_responses_identity.sql.
--
-- WHY
-- ---
-- Before this change the tools step stored a user's custom "Other" text by
-- REPLACING the "other" marker with the raw string inside the current_tools
-- JSON array (see onboarding-ui app/lib/other-option.js buildToolsPayload).
-- That made the array a mix of canonical keys and free text, so no consumer
-- could tell "the user picked Notion" from "the user typed Notion" — and the
-- analytics export flattened both into the same cell.
--
-- This walks existing rows, moves any non-canonical entry out to the new
-- tools_other column, and puts the canonical "other" marker back in the array,
-- bringing legacy records in line with the industry/role model.
--
-- Idempotent by construction: it only touches rows where tools_other IS NULL
-- (i.e. not yet migrated) AND a non-canonical entry is actually present. A
-- second run finds nothing to do. Rows whose arrays are already clean are
-- left untouched, so no mtime churn.

DROP PROCEDURE IF EXISTS `_ob_backfill_tools_other`;

DELIMITER $$

CREATE PROCEDURE `_ob_backfill_tools_other`()
BEGIN
DECLARE _done INT DEFAULT 0;
DECLARE _id INT UNSIGNED;
DECLARE _tools JSON;
DECLARE _out JSON;
DECLARE _custom VARCHAR(255);
DECLARE _val VARCHAR(255);
DECLARE _i INT;
DECLARE _len INT;
DECLARE _has_other TINYINT;

DECLARE cur CURSOR FOR
SELECT id, current_tools
FROM onboarding_responses
WHERE tools_other IS NULL
AND current_tools IS NOT NULL
AND JSON_VALID(current_tools)
AND JSON_TYPE(current_tools) = 'ARRAY'
AND JSON_LENGTH(current_tools) > 0;

DECLARE CONTINUE HANDLER FOR NOT FOUND SET _done = 1;

OPEN cur;
scan: LOOP
FETCH cur INTO _id, _tools;
IF _done = 1 THEN
LEAVE scan;
END IF;

SET _out = JSON_ARRAY();
SET _custom = NULL;
SET _has_other = 0;
SET _i = 0;
SET _len = JSON_LENGTH(_tools);

WHILE _i < _len DO
SET _val = JSON_VALUE(_tools, CONCAT('$[', _i, ']'));
IF _val IS NOT NULL AND _val <> '' THEN
IF _val IN ('google_drive','notion','slack','dropbox',
'clickup','trello','jira') THEN
SET _out = JSON_ARRAY_APPEND(_out, '$', _val);
ELSEIF _val = 'other' THEN
SET _has_other = 1;
ELSE
-- Non-canonical entry: this is the user's free text.
-- Keep the first one; extra entries are unreachable via
-- the UI (a single "Other" input) but concatenating would
-- corrupt the value, so later ones are dropped.
IF _custom IS NULL THEN
SET _custom = _val;
END IF;
SET _has_other = 1;
END IF;
END IF;
SET _i = _i + 1;
END WHILE;

IF _has_other = 1 THEN
SET _out = JSON_ARRAY_APPEND(_out, '$', 'other');
END IF;

-- Only rewrite rows that actually carried free text. A row that merely
-- held canonical keys is already correct and must not be re-stamped.
IF _custom IS NOT NULL THEN
UPDATE onboarding_responses
SET current_tools = _out,
tools_other = _custom
WHERE id = _id;
END IF;
END LOOP;
CLOSE cur;
END$$

DELIMITER ;

CALL `_ob_backfill_tools_other`();

DROP PROCEDURE `_ob_backfill_tools_other`;
Loading
Loading