Rename Basic to Personal plan #8951
Conversation
📝 WalkthroughWalkthroughRename pricing plan value "Basic" to "Personal" across DB, backend, and frontend; update DB migration/schema/views, WKORG default initialPricingParameters, UI strings/utilities, fixture, and docs to use the new plan name and adjusted included-user defaults. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
conf/evolutions/142-personal-plan.sql (2)
5-8: Make view drops idempotentUse IF EXISTS to avoid failures on environments where views might be absent.
Apply:
-DROP VIEW webknossos.userinfos; -DROP VIEW webknossos.organizations_; +DROP VIEW IF EXISTS webknossos.userinfos; +DROP VIEW IF EXISTS webknossos.organizations_;
13-23: Exclude deleted users in the migration countCurrently counts from base tables; use filtered views to avoid counting deleted rows.
Apply:
-AND ( - SELECT COUNT(*) - FROM webknossos.users u - JOIN webknossos.multiUsers m ON u._multiUser = m._id - WHERE u._organization = webknossos.organizations._id - AND m.isSuperUser = FALSE -) = 1; +AND ( + SELECT COUNT(*) + FROM webknossos.users_ u + JOIN webknossos.multiUsers_ m ON u._multiUser = m._id + WHERE u._organization = webknossos.organizations._id + AND m.isSuperUser = FALSE +) = 1;conf/evolutions/reversions/142-personal-plan.sql (4)
6-7: Make drops idempotent with IF EXISTSSafer reversions when the views might not exist (local/dev DBs or partial runs).
-DROP VIEW webknossos.userinfos; -DROP VIEW webknossos.organizations_; +DROP VIEW IF EXISTS webknossos.userinfos; +DROP VIEW IF EXISTS webknossos.organizations_;
10-11: Typo in temporary type name: use pricing_plans_old (not prizing_plans_old)Purely a naming/clarity fix; avoids confusion for future maintainers.
-ALTER TYPE webknossos.PRICING_PLANS RENAME TO prizing_plans_old; +ALTER TYPE webknossos.PRICING_PLANS RENAME TO pricing_plans_old; @@ - WHEN 'Personal'::webknossos.prizing_plans_old THEN 'Basic'::webknossos.PRICING_PLANS - WHEN 'Team'::webknossos.prizing_plans_old THEN 'Team'::webknossos.PRICING_PLANS - WHEN 'Power'::webknossos.prizing_plans_old THEN 'Power'::webknossos.PRICING_PLANS - WHEN 'Team_Trial'::webknossos.prizing_plans_old THEN 'Team_Trial'::webknossos.PRICING_PLANS - WHEN 'Power_Trial'::webknossos.prizing_plans_old THEN 'Power_Trial'::webknossos.PRICING_PLANS - WHEN 'Custom'::webknossos.prizing_plans_old THEN 'Custom'::webknossos.PRICING_PLANS + WHEN 'Personal'::webknossos.pricing_plans_old THEN 'Basic'::webknossos.PRICING_PLANS + WHEN 'Team'::webknossos.pricing_plans_old THEN 'Team'::webknossos.PRICING_PLANS + WHEN 'Power'::webknossos.pricing_plans_old THEN 'Power'::webknossos.PRICING_PLANS + WHEN 'Team_Trial'::webknossos.pricing_plans_old THEN 'Team_Trial'::webknossos.PRICING_PLANS + WHEN 'Power_Trial'::webknossos.pricing_plans_old THEN 'Power_Trial'::webknossos.PRICING_PLANS + WHEN 'Custom'::webknossos.pricing_plans_old THEN 'Custom'::webknossos.PRICING_PLANS @@ -DROP TYPE webknossos.prizing_plans_old; +DROP TYPE webknossos.pricing_plans_old;Also applies to: 16-21, 24-24
13-13: Fix column name casing for consistencyUnquoted identifiers are case-folded, but this reads like a typo.
- ALTER COLUMN pricingPLan DROP DEFAULT, + ALTER COLUMN pricingPlan DROP DEFAULT,
15-22: Add ELSE branch in CASE to future-proof mappingPrevents unexpected NULLs if new enum values appear.
ALTER COLUMN pricingPlan TYPE webknossos.PRICING_PLANS USING CASE pricingPlan - WHEN 'Personal'::webknossos.pricing_plans_old THEN 'Basic'::webknossos.PRICING_PLANS + WHEN 'Personal'::webknossos.pricing_plans_old THEN 'Basic'::webknossos.PRICING_PLANS WHEN 'Team'::webknossos.pricing_plans_old THEN 'Team'::webknossos.PRICING_PLANS WHEN 'Power'::webknossos.pricing_plans_old THEN 'Power'::webknossos.PRICING_PLANS WHEN 'Team_Trial'::webknossos.pricing_plans_old THEN 'Team_Trial'::webknossos.PRICING_PLANS WHEN 'Power_Trial'::webknossos.pricing_plans_old THEN 'Power_Trial'::webknossos.PRICING_PLANS WHEN 'Custom'::webknossos.pricing_plans_old THEN 'Custom'::webknossos.PRICING_PLANS + ELSE pricingPlan::text::webknossos.PRICING_PLANS END,frontend/javascripts/admin/organization/pricing_plan_utils.ts (1)
79-84: Tighten grammar in comment and debug messageMinor polish; avoids noisy console wording.
- // This function should not be called to check for "Personal" plans since its the default plan for all users anyway. + // This function should not be called to check for "Personal" plans since it's the default plan for all users anyway. @@ - "Restricting a feature to Personal Plan does not make sense. Consider removing the restriction", + "Restricting a feature to the Personal plan doesn't make sense. Consider removing the restriction",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test/db/organizations.csvis excluded by!**/*.csv
📒 Files selected for processing (10)
app/models/organization/OrganizationService.scala(1 hunks)app/models/team/PricingPlan.scala(1 hunks)conf/evolutions/142-personal-plan.sql(1 hunks)conf/evolutions/reversions/142-personal-plan.sql(1 hunks)frontend/javascripts/admin/onboarding.tsx(2 hunks)frontend/javascripts/admin/organization/organization_overview_view.tsx(4 hunks)frontend/javascripts/admin/organization/pricing_plan_utils.ts(5 hunks)frontend/javascripts/admin/organization/upgrade_plan_modal.tsx(2 hunks)frontend/javascripts/test/fixtures/dummy_organization.ts(1 hunks)tools/postgres/schema.sql(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/javascripts/admin/onboarding.tsx (1)
frontend/javascripts/admin/organization/pricing_plan_utils.ts (1)
maxInludedUsersInPersonalPlan(30-30)
app/models/organization/OrganizationService.scala (2)
app/utils/WkConf.scala (1)
Features(134-154)app/models/team/PricingPlan.scala (1)
PricingPlan(5-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-smoketest-push
- GitHub Check: frontend-tests
🔇 Additional comments (16)
frontend/javascripts/test/fixtures/dummy_organization.ts (1)
8-8: Fixture plan rename to Personal: LGTMAligned with enum rename and limits (includedUsers=1).
app/models/organization/OrganizationService.scala (1)
116-118: Default plan change to Personal with 1 included user: LGTMMatches the migration and frontend expectations.
Please verify tests/seed data that previously assumed 3 included users for the base plan.
tools/postgres/schema.sql (2)
24-24: Schema version bump to 142: LGTM
335-335: Enum now includes 'Personal': LGTMIn sync with Scala/TS enums and migration.
frontend/javascripts/admin/organization/upgrade_plan_modal.tsx (2)
59-62: Copy updated to reference Personal: LGTM
170-175: Default upgrade branch for Personal: LGTMfrontend/javascripts/admin/onboarding.tsx (2)
18-18: Import switched to Personal constant: LGTM
213-214: InviteUsersModal default max users set for Personal plan: LGTMfrontend/javascripts/admin/organization/organization_overview_view.tsx (2)
76-104: Upgrade actions treat Personal as baseline: LGTMClick handlers correctly route Personal to the plan upgrade modal.
197-207: “Unlock more features” gating updated for Personal: LGTMapp/models/team/PricingPlan.scala (1)
7-9: LGTM — no code usages ofBasicremain; only migrations/docs mention itNo references to PricingPlan.Basic / PricingPlanEnum.Basic found in runtime code. Remaining "Basic" hits are in DB evolutions (e.g. conf/evolutions/142-personal-plan.sql, 094-pricing-plans.sql, 068-pricing-plan.sql) and docs/tests — expected and safe to keep as-is.
conf/evolutions/reversions/142-personal-plan.sql (1)
44-46: LGTM: schemaVersion reverted to 141Matches this being the down migration for 142.
frontend/javascripts/admin/organization/pricing_plan_utils.ts (4)
14-14: LGTM: updated feature textCopy aligns with the new plan name.
23-23: LGTM: updated feature textConsistent wording with Personal plan.
60-66: LGTM: PLAN_TO_RANK baseline switched to PersonalRanking logic remains consistent for trials and paid tiers.
5-5: Rename OK — no PricingPlan.Basic usages foundSearch found no references to PricingPlan.Basic or PricingPlanEnum.Basic; remaining "Basic" hits are test titles (frontend/javascripts/test/...) and a UI label ("Basic authentication" in frontend/javascripts/admin/dataset/remote/add_remote_layer.tsx), not pricing-enum call sites.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
conf/evolutions/142-personal-plan.sql (3)
5-8: Use IF EXISTS to make view drops resilient.Avoids migration failures if views are missing in some environments.
-DROP VIEW webknossos.userinfos; -DROP VIEW webknossos.organizations_; +DROP VIEW IF EXISTS webknossos.userinfos; +DROP VIEW IF EXISTS webknossos.organizations_;
13-23: Count non-deleted users to avoid skew from soft-deleted rows.Use the filtered views (users_, multiUsers_) or add isDeleted filters to ensure COUNT = 1 reflects active records.
-AND ( - SELECT COUNT(*) - FROM webknossos.users u - JOIN webknossos.multiUsers m ON u._multiUser = m._id - WHERE u._organization = webknossos.organizations._id - AND m.isSuperUser = FALSE -) = 1; +AND ( + SELECT COUNT(*) + FROM webknossos.users_ u + JOIN webknossos.multiUsers_ m ON u._multiUser = m._id + WHERE u._organization = webknossos.organizations._id + AND m.isSuperUser = FALSE +) = 1;
25-35: Minor: Normalize identifier casing for consistency.Unquoted identifiers are folded to lowercase; prefer a consistent style throughout (e.g., userinfos vs userInfos). No functional change required.
-CREATE VIEW webknossos.userInfos AS +CREATE VIEW webknossos.userinfos AS SELECT u._id AS _user, m.email, u.firstName, u.lastName, o.name AS organization_name,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
conf/evolutions/142-personal-plan.sql(1 hunks)conf/evolutions/reversions/142-personal-plan.sql(1 hunks)docs/automation/animations.md(1 hunks)docs/sharing/dataset_sharing.md(2 hunks)frontend/javascripts/admin/onboarding.tsx(2 hunks)frontend/javascripts/admin/organization/pricing_plan_utils.ts(5 hunks)unreleased_changes/8951.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/automation/animations.md
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/javascripts/admin/organization/pricing_plan_utils.ts
- conf/evolutions/reversions/142-personal-plan.sql
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/javascripts/admin/onboarding.tsx (1)
frontend/javascripts/admin/organization/pricing_plan_utils.ts (1)
maxIncludedUsersInPersonalPlan(30-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: backend-tests
- GitHub Check: frontend-tests
- GitHub Check: build-smoketest-push
🔇 Additional comments (5)
frontend/javascripts/admin/onboarding.tsx (2)
18-18: Updated import aligns with Personal plan rename.Consistent with pricing_plan_utils export.
213-214: Default prop matches Personal plan limit.Matches InviteUsersModal defaults; I couldn't verify removal of the old Basic-plan constant because the repo search didn't complete — please run locally: rg -nP -uu 'maxInludedUsersInBasicPlan|maxIncludedUsersInBasicPlan|maxIncludedUsersInPersonalPlan|maxUserCountPerOrganization' -g '!/node_modules/' -g '!/dist/' and confirm no matches.
docs/sharing/dataset_sharing.md (2)
64-66: Alt text/caption update looks good — image presentConfirmed docs/images/dataset_settings_metadata.jpeg exists; caption/alt update is fine.
32-34: Alt text/caption update verified — image present
docs/images/dataset_settings_sharing.jpeg exists.unreleased_changes/8951.md (1)
1-5: Changelog entry LGTM — migration present; update remaining "Basic" user-facing references.
- Migration present: conf/evolutions/142-personal-plan.sql
- Action: replace user-facing "Basic" → "Personal" in docs/UI: docs/ui/toolbar.md, docs/skeleton_annotation/tools.md, docs/users/new_users.md; confirm whether frontend/javascripts/admin/dataset/remote/add_remote_layer.tsx ("Basic authentication") is unrelated to pricing and should remain. Leave historical evolutions/reversions under conf/evolutions/* unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
conf/evolutions/reversions/142-personal-plan.sql (1)
8-9: Use IF EXISTS on DROP VIEWMakes the script more robust without changing semantics.
-DROP VIEW webknossos.userinfos; -DROP VIEW webknossos.organizations_; +DROP VIEW IF EXISTS webknossos.userinfos; +DROP VIEW IF EXISTS webknossos.organizations_;conf/evolutions/142-personal-plan.sql (2)
15-25: Optional: Guard against overwriting customized includedUsersOnly adjust orgs still at the old default (3) to avoid touching customized values.
-SET includedUsers = 1 -WHERE pricingplan = 'Personal'::webknossos.PRICING_PLANS +SET includedUsers = 1 +WHERE pricingplan = 'Personal'::webknossos.PRICING_PLANS +AND includedUsers = 3 AND (
8-9: Use IF EXISTS on DROP VIEWSafer and consistent with resilient evolutions.
-DROP VIEW webknossos.userinfos; -DROP VIEW webknossos.organizations_; +DROP VIEW IF EXISTS webknossos.userinfos; +DROP VIEW IF EXISTS webknossos.organizations_;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
conf/evolutions/142-personal-plan.sql(1 hunks)conf/evolutions/reversions/142-personal-plan.sql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-smoketest-push
- GitHub Check: frontend-tests
- GitHub Check: backend-tests
🔇 Additional comments (7)
conf/evolutions/reversions/142-personal-plan.sql (3)
1-1: Remove issue link from evolution headerMatches house style noted in prior reviews.
--- https://github.com/scalableminds/webknossos/issues/8951
3-6: LGTM: version guard and transaction blockVersion assertion and transactional DDL are correct.
11-13: LGTM: enum value rename (Personal -> Basic)Appropriate use of ALTER TYPE RENAME VALUE.
conf/evolutions/142-personal-plan.sql (4)
1-1: Remove issue link from evolution headerPer prior feedback, omit issue/PR URLs in evolutions.
--- https://github.com/scalableminds/webknossos/issues/8951
11-13: LGTM: enum value rename (Basic -> Personal)Correct and minimal change.
27-37: LGTM: view definitionsRecreated views look consistent and correct.
3-6: LGTM: version guard and transaction blockMatches evolution conventions.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
conf/evolutions/142-personal-plan.sql(1 hunks)conf/evolutions/reversions/142-personal-plan.sql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
- GitHub Check: frontend-tests
| UPDATE webknossos.organizations | ||
| SET includedUsers = 3 | ||
| WHERE pricingplan = 'Basic'::webknossos.PRICING_PLANS | ||
| AND includedUsers = 1 | ||
| AND ( | ||
| SELECT COUNT(*) | ||
| FROM webknossos.users u | ||
| JOIN webknossos.multiUsers m ON u._multiUser = m._id | ||
| WHERE u._organization = webknossos.organizations._id | ||
| AND m.isSuperUser = FALSE | ||
| ) < 3; |
There was a problem hiding this comment.
Fix downgrade to revert both 1- and 2-seat orgs using the same active-user filter as the upgrade.
Right now the downgrade only touches rows where includedUsers = 1 and counts on the base tables. Any org that the upgrade moved to includedUsers = 2 (two active non-superusers) is left at 2 after the downgrade, so the schema isn’t actually reversible. Counting on webknossos.users / multiUsers also pulls in soft-deleted rows, which means we can even miss the =1 case if there are deleted accounts. Please align the filter with the forward migration so we reset every row the upgrade touched.
UPDATE webknossos.organizations
SET includedUsers = 3
WHERE pricingplan = 'Basic'::webknossos.PRICING_PLANS
- AND includedUsers = 1
- AND (
- SELECT COUNT(*)
- FROM webknossos.users u
- JOIN webknossos.multiUsers m ON u._multiUser = m._id
+ AND includedUsers IN (1, 2)
+ AND (
+ SELECT COUNT(*)
+ FROM webknossos.users_ u
+ JOIN webknossos.multiUsers_ m ON u._multiUser = m._id
WHERE u._organization = webknossos.organizations._id
AND m.isSuperUser = FALSE
- ) < 3;
+ ) <= 2;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UPDATE webknossos.organizations | |
| SET includedUsers = 3 | |
| WHERE pricingplan = 'Basic'::webknossos.PRICING_PLANS | |
| AND includedUsers = 1 | |
| AND ( | |
| SELECT COUNT(*) | |
| FROM webknossos.users u | |
| JOIN webknossos.multiUsers m ON u._multiUser = m._id | |
| WHERE u._organization = webknossos.organizations._id | |
| AND m.isSuperUser = FALSE | |
| ) < 3; | |
| UPDATE webknossos.organizations | |
| SET includedUsers = 3 | |
| WHERE pricingplan = 'Basic'::webknossos.PRICING_PLANS | |
| AND includedUsers IN (1, 2) | |
| AND ( | |
| SELECT COUNT(*) | |
| FROM webknossos.users_ u | |
| JOIN webknossos.multiUsers_ m ON u._multiUser = m._id | |
| WHERE u._organization = webknossos.organizations._id | |
| AND m.isSuperUser = FALSE | |
| ) <= 2; |
PR renames Basic to Personal plan.
Issues:
(Please delete unneeded items, merge only when none are left open)
$PR_NUMBER.mdfile inunreleased_changesor use./tools/create-changelog-entry.py)