Feat: create demo dataset#20
Conversation
… setup - Added support for fetching and listing demo datasets from the subscription server. - Introduced new types for demo dataset payloads and updated the setupDummyInstance function to handle demo data. - Enhanced the DatabaseSelector component to allow users to select demo datasets or use a built-in offline demo. - Refactored helper functions to accommodate new demo-related features and ensure proper state management during instance setup. - Updated configuration files to include demo metadata. This commit lays the groundwork for a more robust demo experience, enabling users to easily access sample data for testing and development.
Add runtime shape checks for body.message so malformed subscription responses return success: false instead of unsafe casts to DemoDatasetPayload.
Remove module-level catalog/payload state; pass DummyRunContext through generators. Add resolveDummyFlow, resolvePurchaseItemPartyLookup, and getFlowConstantWithFlow in helpers to avoid shared activeFlow/purchase maps during a run (concurrent-safe generation path).
Rename DemoItemSeed.for to avoid the reserved identifier; map to Item.for when creating Item docs. Accept legacy JSON "for" from get_demo_dataset and normalize payloads in main process. Tighten printSettings.displayLogo typing. Wrap GET_DEMO_DATASET IPC handler in try/catch so failures return structured errors instead of rejecting.
Wrap LIST_DEMO_DATASETS in try/catch so failures return a structured response. Catch getDemoDataset IPC rejection in DatabaseSelector and surface the error.
Warn when applyDummyHelpersOverrides receives a non-null flowOverride whose length is not 12. Catch ipc.listDemoDatasets rejection in createDemo and fall back to the built-in sample with an info dialog. Narrow DemoConfigMeta to isDemo: true when demo metadata is passed.
Add isDemoDatasetListRow and reject the whole list with a clear message when any row is malformed, instead of casting the API array.
Bump package version. When creating a company from a demo payload, fail fast if fiscalYearStartMD/fiscalYearEndMD cannot be parsed instead of falling back to the current date.
…llable demoKey Modified the DemoConfigMeta type to make isDemo optional and change demoKey to accept null values, enhancing flexibility in demo configurations.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR introduces a subscription-gated demo dataset system that allows users to create pre-configured demo companies from server-provided payloads. The generator is refactored to be payload-driven, removing hardcoded catalog and account mappings while supporting flexible item/party seeding and flow overrides. ChangesDemo Dataset Subscription
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/DatabaseSelector.vue (1)
946-974:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlways clear
creatingDemoand surface setup failures.If
setupDummyInstance(),updateConfigFiles(), orfyo.purgeCache()throws here, the component never resetscreatingDemo, so the page stays blocked and the user gets no actionable error. Wrap this flow intry/catch/finally.🛠️ Suggested fix
this.creatingDemo = true; - await setupDummyInstance( - filePath, - fyo, - 1, - this.baseCount, - (message, percent) => { - this.creationMessage = message; - this.creationPercent = percent; - }, - demoPayload ?? undefined - ); - - updateConfigFiles(fyo, { - isDemo: true, - demoKey: demoPayload?.key ?? 'flo-clothes', - }); - await fyo.purgeCache(); - await this.setFiles(); - this.fyo.telemetry.log(Verb.Created, 'dummy-instance'); - this.creatingDemo = false; - this.$emit('file-selected', filePath); + try { + await setupDummyInstance( + filePath, + fyo, + 1, + this.baseCount, + (message, percent) => { + this.creationMessage = message; + this.creationPercent = percent; + }, + demoPayload ?? undefined + ); + + updateConfigFiles(fyo, { + isDemo: true, + demoKey: demoPayload?.key ?? 'flo-clothes', + }); + await fyo.purgeCache(); + await this.setFiles(); + this.fyo.telemetry.log(Verb.Created, 'dummy-instance'); + this.$emit('file-selected', filePath); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + await showDialog({ + title: this.t`Could not create demo`, + detail: message, + type: 'error', + }); + } finally { + this.creatingDemo = false; + this.creationPercent = 0; + this.creationMessage = ''; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/DatabaseSelector.vue` around lines 946 - 974, The startDummyInstanceSetup method can leave creatingDemo true and swallow errors if setupDummyInstance, updateConfigFiles, or fyo.purgeCache throws; wrap the sequence beginning after setting this.creatingDemo = true in a try/catch/finally: perform setupDummyInstance(...), updateConfigFiles(...), await fyo.purgeCache() inside try, on catch set a visible error (e.g. set this.creationMessage to the error.message and/or emit an error event) so the UI surfaces the failure, and in finally ensure this.creatingDemo = false (and reset creationPercent if desired) so the page is unblocked regardless of outcome.dummy/index.ts (1)
320-359:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResolve journal-entry accounts from the configured company data.
These entries still hardcode
Supreme Bank,Cash, andSecured Loans. With payload-driven demos,bankNameand even the chart template can differ, so this will break as soon as those names are absent. Reuse the same dynamic account lookup approach used elsewhere in this file instead of embedding account names here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dummy/index.ts` around lines 320 - 359, The journal entries currently hardcode account names when building JournalEntry docs via fyo.doc.getNewDoc and await doc.append('accounts'), which will break for different companies or chart templates; replace the hardcoded strings ('Supreme Bank', 'Cash', 'Secured Loans') with the dynamic account-resolution helper used elsewhere in this file (the same lookup used for bankName/chart-template resolution) so you resolve each account ID/name from the configured company data before calling append('accounts') and use those resolved values when creating both the Bank Entry and Cash Entry; keep the same usage of fyo.doc.getNewDoc, ModelNameEnum.JournalEntry, entries.push(doc) and amount/amount.percent as-is.
🧹 Nitpick comments (4)
main/demoData.ts (2)
22-24: ⚡ Quick winConsider excluding arrays from
isRecord.The current implementation allows arrays to pass as records since
typeof [] === 'object'. While validators later checkArray.isArrayseparately, explicitly excluding arrays here would make the type guard more precise and prevent potential bugs.🔧 Suggested fix
function isRecord(value: unknown): value is Record<string, unknown> { - return typeof value === 'object' && value !== null; + return typeof value === 'object' && value !== null && !Array.isArray(value); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main/demoData.ts` around lines 22 - 24, The isRecord type guard currently returns true for arrays because typeof [] === 'object'; update the function isRecord to explicitly exclude arrays by also checking !Array.isArray(value) so it only narrows to plain object records and not arrays, keeping the signature value is Record<string, unknown> and preserving the existing null check.
215-238: ⚡ Quick winInconsistent number validation between
isNumberArrayandisPeriodicPurchasesMap.
isNumberArray(line 220) checkstypeof x !== 'number' || Number.isNaN(x)which allowsInfinity, whileisPeriodicPurchasesMap(line 233) uses!Number.isFinite(n)which rejects bothNaNandInfinity. Consider using consistent validation for numeric values across both functions.🔧 Suggested fix for consistency
If you want to reject
Infinityin both cases:function isNumberArray(value: unknown): boolean { if (!Array.isArray(value)) { return false; } for (const x of value) { - if (typeof x !== 'number' || Number.isNaN(x)) { + if (typeof x !== 'number' || !Number.isFinite(x)) { return false; } } return true; }Alternatively, if
Infinityis acceptable in flow arrays, document why the difference exists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main/demoData.ts` around lines 215 - 238, isNumberArray and isPeriodicPurchasesMap perform inconsistent numeric validation (isNumberArray allows Infinity via Number.isNaN check while isPeriodicPurchasesMap uses Number.isFinite). Update isNumberArray to the same strictness by replacing its check to reject non-finite values (use Number.isFinite) so both functions consistently reject NaN and Infinity; alternatively, if Infinity should be allowed, change isPeriodicPurchasesMap to the same relaxed check—modify the logic in isNumberArray or isPeriodicPurchasesMap accordingly to make the validation behavior uniform.dummy/items.json (1)
1-1: ⚡ Quick winConsider formatting the JSON for maintainability.
The entire item catalog is on a single minified line, making diffs, reviews, and manual edits harder. Formatting with one item per line (or with indentation) would improve readability and version control.
♻️ Suggested formatting improvement
While the current structure is valid, consider pretty-printing the JSON array with indentation:
[ { "name": "Dry-Cleaning", "description": null, ... }, { "name": "Electricity", ... } ]This makes the file easier to maintain and diff in version control.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dummy/items.json` at line 1, The items.json is minified in one line which hinders reviews; reformat the JSON array (the top-level array containing items like "Dry-Cleaning", "Electricity", "Marketing - Video", "Jade Slippers", etc.) as pretty-printed JSON with indentation and one object per entry (each item as a multi-line object) so diffs are readable and maintainable; ensure valid JSON is preserved (no trailing commas) and run a JSON linter/formatter (e.g., prettier or jq) before committing.dummy/helpers.ts (1)
29-35: ⚡ Quick winAvoid exposing mutable module-scoped demo state.
The deprecated
flowalias and these getters hand out the live shared state by reference, so one caller can mutate defaults for later runs. That undercuts the per-run isolation this PR is moving toward; return clones here and make the deprecated export readonly/frozen.♻️ Minimal hardening
-export const flow = DEFAULT_FLOW_VALUES; +export const flow = Object.freeze([...DEFAULT_FLOW_VALUES]); export function getFlowArray(): number[] { - return activeFlow; + return [...activeFlow]; } export function getPurchaseItemPartyMap(): Record<string, string> { - return activePurchaseItemPartyMap; + return { ...activePurchaseItemPartyMap }; }Also applies to: 110-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dummy/helpers.ts` around lines 29 - 35, The module currently exposes mutable module-scoped state (the deprecated export flow, activeFlow array, and activePurchaseItemPartyMap object and related getters) which lets callers mutate shared defaults; change this by making the deprecated export flow a read-only/frozen value (e.g., Object.freeze clone of DEFAULT_FLOW_VALUES) and ensure any getters/functions that return activeFlow or activePurchaseItemPartyMap (and the ones referenced around lines 110-115) return shallow clones (e.g., [...activeFlow] and {...activePurchaseItemPartyMap}) so callers receive copies, not live references, preserving per-run isolation while keeping internal state mutable only within the module.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dummy/index.ts`:
- Around line 67-85: The helpers defaultReceivableAccount and
defaultPayableAccount should stop returning hard-coded names when common names
don't exist; instead, after checking for 'Debtors'/'Trade Receivable' and
'Creditors'/'Trade Payable' respectively, follow the same approach as
defaultCashAccount: query the Account model for a non-group account of the
appropriate type and return that account's name (use the same query pattern and
model symbols as defaultCashAccount, e.g. fyo.db.findOne(ModelNameEnum.Account,
{ type: <receivable|payable> , group: false }) and then return the found
account.name); if no such account is found handle the absence consistently with
defaultCashAccount (e.g. surface an error or fallback policy used there).
- Around line 171-194: Before setting fyo.store.skipTelemetryLogging = true,
capture the previous value (e.g., const prevSkip =
fyo.store?.skipTelemetryLogging) then set it to true, and in the existing
finally block restore it (fyo.store.skipTelemetryLogging = prevSkip) so any
exceptions won't leave telemetry disabled or overwrite caller state; locate the
telemetry mutation near setupInstance / generateStaticEntries /
generateDynamicEntries and restore the saved value just before calling
resetDummyHelpers().
---
Outside diff comments:
In `@dummy/index.ts`:
- Around line 320-359: The journal entries currently hardcode account names when
building JournalEntry docs via fyo.doc.getNewDoc and await
doc.append('accounts'), which will break for different companies or chart
templates; replace the hardcoded strings ('Supreme Bank', 'Cash', 'Secured
Loans') with the dynamic account-resolution helper used elsewhere in this file
(the same lookup used for bankName/chart-template resolution) so you resolve
each account ID/name from the configured company data before calling
append('accounts') and use those resolved values when creating both the Bank
Entry and Cash Entry; keep the same usage of fyo.doc.getNewDoc,
ModelNameEnum.JournalEntry, entries.push(doc) and amount/amount.percent as-is.
In `@src/pages/DatabaseSelector.vue`:
- Around line 946-974: The startDummyInstanceSetup method can leave creatingDemo
true and swallow errors if setupDummyInstance, updateConfigFiles, or
fyo.purgeCache throws; wrap the sequence beginning after setting
this.creatingDemo = true in a try/catch/finally: perform
setupDummyInstance(...), updateConfigFiles(...), await fyo.purgeCache() inside
try, on catch set a visible error (e.g. set this.creationMessage to the
error.message and/or emit an error event) so the UI surfaces the failure, and in
finally ensure this.creatingDemo = false (and reset creationPercent if desired)
so the page is unblocked regardless of outcome.
---
Nitpick comments:
In `@dummy/helpers.ts`:
- Around line 29-35: The module currently exposes mutable module-scoped state
(the deprecated export flow, activeFlow array, and activePurchaseItemPartyMap
object and related getters) which lets callers mutate shared defaults; change
this by making the deprecated export flow a read-only/frozen value (e.g.,
Object.freeze clone of DEFAULT_FLOW_VALUES) and ensure any getters/functions
that return activeFlow or activePurchaseItemPartyMap (and the ones referenced
around lines 110-115) return shallow clones (e.g., [...activeFlow] and
{...activePurchaseItemPartyMap}) so callers receive copies, not live references,
preserving per-run isolation while keeping internal state mutable only within
the module.
In `@dummy/items.json`:
- Line 1: The items.json is minified in one line which hinders reviews; reformat
the JSON array (the top-level array containing items like "Dry-Cleaning",
"Electricity", "Marketing - Video", "Jade Slippers", etc.) as pretty-printed
JSON with indentation and one object per entry (each item as a multi-line
object) so diffs are readable and maintainable; ensure valid JSON is preserved
(no trailing commas) and run a JSON linter/formatter (e.g., prettier or jq)
before committing.
In `@main/demoData.ts`:
- Around line 22-24: The isRecord type guard currently returns true for arrays
because typeof [] === 'object'; update the function isRecord to explicitly
exclude arrays by also checking !Array.isArray(value) so it only narrows to
plain object records and not arrays, keeping the signature value is
Record<string, unknown> and preserving the existing null check.
- Around line 215-238: isNumberArray and isPeriodicPurchasesMap perform
inconsistent numeric validation (isNumberArray allows Infinity via Number.isNaN
check while isPeriodicPurchasesMap uses Number.isFinite). Update isNumberArray
to the same strictness by replacing its check to reject non-finite values (use
Number.isFinite) so both functions consistently reject NaN and Infinity;
alternatively, if Infinity should be allowed, change isPeriodicPurchasesMap to
the same relaxed check—modify the logic in isNumberArray or
isPeriodicPurchasesMap accordingly to make the validation behavior uniform.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 355b07b1-ab56-4495-b73b-19e529c57dbf
📒 Files selected for processing (15)
dummy/helpers.tsdummy/index.tsdummy/items.jsondummy/types.tsfyo/core/types.tsmain/demoData.tsmain/helpers.tsmain/preload.tsmain/registerIpcMainActionListeners.tsmain/subscription.tspackage.jsonsrc/pages/DatabaseSelector.vuesrc/utils/misc.tsutils/messages.tsutils/subscriptionFeatures.ts
Save the prior flag before disabling telemetry for dummy generation and restore it in finally so errors cannot leave telemetry suppressed.
After Debtors/Trade Receivable and Creditors/Trade Payable checks, pick the first non-group Receivable/Payable account like defaultCashAccount does for Cash.
https://tasko.rukn.sh/tasko/task/TASK-2026-00486
Summary by CodeRabbit
Release Notes
New Features
Chores