feat：Add model management mock#1795
Merged
hexqi merged 6 commits intoopentiny:developfrom Mar 31, 2026
Merged
Conversation
Contributor
|
Caution Review failedFailed to post review comments WalkthroughBackend services transition from static in-memory storage to NeDB persistence with database-backed CRUD operations, route conflict detection, and CSS styling refinements across Vue components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/plugins/page/src/PageSetting.vue (1)
502-506: Consider adding overflow handling for the fixed height.Setting a fixed
height: 80pxwithout anoverflowproperty may cause content to be clipped if the lifecycle configuration content exceeds this height.♻️ Suggested improvement
.input-output { :deep(.tiny-collapse-item__content) { height: 80px; + overflow-y: auto; } }This ensures that if the content exceeds 80px, users can scroll to view it rather than having it clipped.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugins/page/src/PageSetting.vue` around lines 502 - 506, The .input-output rule sets a fixed height on :deep(.tiny-collapse-item__content) which can clip content; change it to handle overflow by replacing the fixed height with max-height: 80px (or keep height: 80px) and add overflow-y: auto (and optionally overflow-x: hidden) so users can scroll when lifecycle configuration content exceeds 80px; update the CSS block referencing .input-output and :deep(.tiny-collapse-item__content) accordingly.mockServer/src/services/model.js (1)
29-58: Extract the repeated parameter schema into a shared constant.The same
id/name/status/testdefinition is copied intoparametersand four separatechildrenblocks. Any contract change now requires five manual edits, which is likely to drift quickly. A shared schema constant cloned into each location would keep the mock consistent.Also applies to: 71-100, 129-158, 163-192, 233-262
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mockServer/src/services/model.js` around lines 29 - 58, Extract the repeated parameter definitions (the objects for id, name, status, test) into a single shared constant (e.g., BASE_PARAMETERS) in model.js and replace the inline arrays in the top-level parameters and each children block with a cloned copy of that constant; update all occurrences that currently repeat the same schema (the arrays at the shown block and the similar blocks at the other ranges noted) to reference a deep-cloned copy (e.g., using JSON.parse(JSON.stringify(...)) or a utility clone) so each location gets its own instance but stays consistent when the contract changes, and ensure any code that mutates these arrays uses the cloned instance rather than the shared reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mockServer/src/services/model.js`:
- Around line 334-337: In create() and update() ensure the client-supplied
params cannot overwrite the server-generated id: when building newModel (the
object using mockId++) and when applying updates via the Mongo-like update using
$set: params, remove/omit the id field from params first (strip params.id) so
the server's id (mockId and stored record id) remains authoritative; update the
code paths that build newModel and that call the $set update to use the
sanitized params.
- Around line 346-363: The delete and update methods currently return
getResponseData(result) even when no record was found, producing a success
envelope with data: null; change delete (which calls this.db.findOneAsync and
this.db.removeAsync) to check whether the initial find returned a record (or
whether removeAsync reports affected rows) and, if missing, call
getResponseData({ error: { code: 'NOT_FOUND', message: 'Model not found' }})
instead of returning null data; similarly modify update to verify the
post-update findOneAsync result and return the same not-found error payload via
getResponseData when the record is absent; keep using the existing
getResponseData helper to format both success and error responses.
- Around line 329-343: The create() method currently treats this.modelList as
the source of truth causing duplicate IDs and stale responses; change it to
reload or read persisted records from the DB (don't rely on this.modelList),
compute the new id from persisted records (e.g., max id + 1), call and await the
DB's async insert method (use insertAsync or equivalent instead of
this.db.insert) and only return the persisted/listed records (either by awaiting
the insert result and returning that or by calling list() after insert
completes) so responses reflect the on-disk state and updates/deletes are not
lost.
---
Nitpick comments:
In `@mockServer/src/services/model.js`:
- Around line 29-58: Extract the repeated parameter definitions (the objects for
id, name, status, test) into a single shared constant (e.g., BASE_PARAMETERS) in
model.js and replace the inline arrays in the top-level parameters and each
children block with a cloned copy of that constant; update all occurrences that
currently repeat the same schema (the arrays at the shown block and the similar
blocks at the other ranges noted) to reference a deep-cloned copy (e.g., using
JSON.parse(JSON.stringify(...)) or a utility clone) so each location gets its
own instance but stays consistent when the contract changes, and ensure any code
that mutates these arrays uses the cloned instance rather than the shared
reference.
In `@packages/plugins/page/src/PageSetting.vue`:
- Around line 502-506: The .input-output rule sets a fixed height on
:deep(.tiny-collapse-item__content) which can clip content; change it to handle
overflow by replacing the fixed height with max-height: 80px (or keep height:
80px) and add overflow-y: auto (and optionally overflow-x: hidden) so users can
scroll when lifecycle configuration content exceeds 80px; update the CSS block
referencing .input-output and :deep(.tiny-collapse-item__content) accordingly.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fe3a6d47-9c85-4f63-a4b1-32895b1d2f09
⛔ Files ignored due to path filters (1)
mockServer/src/database/model.dbis excluded by!**/*.db
📒 Files selected for processing (4)
mockServer/src/assets/json/appinfo.jsonmockServer/src/assets/json/model.jsonmockServer/src/services/model.jspackages/plugins/page/src/PageSetting.vue
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
mockServer/src/services/model.js (2)
330-330:⚠️ Potential issue | 🟠 Major
create()is still deriving state from a stale in-memory cache.
this.modelListstarts empty and is never reconciled bydelete()orupdate(). After a restart or one of those writes, Line 330 can allocate the nextidfrom outdated data, and Lines 339-342 can returnrecordsthat no longer match the DB.Also applies to: 339-342
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mockServer/src/services/model.js` at line 330, create() computes mockId and returns records from the stale in-memory cache this.modelList (and delete()/update() never reconcile it), causing incorrect ids and returned records; fix by removing reliance on this.modelList in create()—query the authoritative DB/store for the current max id and the current records (recompute mockId from that result) or ensure delete()/update() update this.modelList after mutations so the cache stays consistent; specifically update the create() logic that sets mockId and returns records, and either make delete()/update() mutate this.modelList to reflect DB changes or eliminate modelList reads entirely and always read fresh data from the DB/store.
331-336:⚠️ Potential issue | 🔴 CriticalKeep
idand audit timestamps server-owned.Line 336 still lets the request body overwrite the generated
id/created_at/updated_at, and Line 359 appliesparamsverbatim on update. That lets clients rewrite the record key, leavesupdated_atstale unless the client sends it, and can make Line 360 returnnullafter a successfulidchange.🔒 Suggested fix
async create(params) { + const { id: _ignoredId, created_at: _ignoredCreatedAt, updated_at: _ignoredUpdatedAt, ...payload } = params || {} const newModel = { ...defaultModel, id: mockId++, created_at: new Date().toISOString(), updated_at: new Date().toISOString(), - ...params + ...payload }async update(id, params) { - await this.db.updateAsync({ id: Number(id) }, { $set: params }) + const { id: _ignoredId, created_at: _ignoredCreatedAt, updated_at: _ignoredUpdatedAt, ...payload } = params || {} + await this.db.updateAsync( + { id: Number(id) }, + { $set: { ...payload, updated_at: new Date().toISOString() } } + ) const result = await this.db.findOneAsync({ id: Number(id) })Also applies to: 359-360
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mockServer/src/services/model.js` around lines 331 - 336, The server currently allows client-supplied params to overwrite server-owned fields: in the newModel creation (where newModel, defaultModel and mockId are used) and in the update code that spreads params onto an existing record, stop params from mutating id/created_at/updated_at; specifically, when building newModel remove id/created_at/updated_at from params (or apply params first and then explicitly set id: mockId++, created_at: new Date().toISOString(), updated_at: new Date().toISOString()), and in the update routine (the block that spreads params onto the existing model) strip id and created_at from incoming params and always set updated_at = new Date().toISOString() before saving so clients cannot change the primary key or timestamps and updated_at is always server-updated.
🧹 Nitpick comments (1)
mockServer/src/services/model.js (1)
28-57: Extract the repeated parameter schema into a shared definition.The same four-field schema is duplicated five times here. Any later field change has to stay in lockstep across every copy, which is easy to miss. A shared constant/helper would keep the mock metadata consistent.
Also applies to: 70-99, 128-157, 162-191, 232-261
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mockServer/src/services/model.js` around lines 28 - 57, The repeated four-field parameter schema in the "parameters" arrays should be extracted into a single shared constant (e.g., COMMON_PARAMETERS) and reused wherever the same schema appears (locations with parameter arrays at lines shown in the review). Replace the duplicated inline array in each occurrence (the object with props 'id','name','status','test') by referencing or spreading the shared constant (for example parameters: COMMON_PARAMETERS or parameters: [...COMMON_PARAMETERS]) so future changes are made in one place; update exports/imports in mockServer/src/services/model.js as needed to use the shared definition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mockServer/src/services/model.js`:
- Around line 322-325: The code currently creates a unique index on NeDB's
internal _id via this.db.ensureIndex but the API uses the field id; modify the
indexing to ensure uniqueness on the API-facing id field by adding/adjusting the
call to this.db.ensureIndex({ fieldName: 'id', unique: true }), and verify this
change alongside the create(), findOneAsync, updateAsync, and removeAsync logic
(and any reliance on this.modelList-generated ids) so duplicate API ids cannot
be inserted after restart.
---
Duplicate comments:
In `@mockServer/src/services/model.js`:
- Line 330: create() computes mockId and returns records from the stale
in-memory cache this.modelList (and delete()/update() never reconcile it),
causing incorrect ids and returned records; fix by removing reliance on
this.modelList in create()—query the authoritative DB/store for the current max
id and the current records (recompute mockId from that result) or ensure
delete()/update() update this.modelList after mutations so the cache stays
consistent; specifically update the create() logic that sets mockId and returns
records, and either make delete()/update() mutate this.modelList to reflect DB
changes or eliminate modelList reads entirely and always read fresh data from
the DB/store.
- Around line 331-336: The server currently allows client-supplied params to
overwrite server-owned fields: in the newModel creation (where newModel,
defaultModel and mockId are used) and in the update code that spreads params
onto an existing record, stop params from mutating id/created_at/updated_at;
specifically, when building newModel remove id/created_at/updated_at from params
(or apply params first and then explicitly set id: mockId++, created_at: new
Date().toISOString(), updated_at: new Date().toISOString()), and in the update
routine (the block that spreads params onto the existing model) strip id and
created_at from incoming params and always set updated_at = new
Date().toISOString() before saving so clients cannot change the primary key or
timestamps and updated_at is always server-updated.
---
Nitpick comments:
In `@mockServer/src/services/model.js`:
- Around line 28-57: The repeated four-field parameter schema in the
"parameters" arrays should be extracted into a single shared constant (e.g.,
COMMON_PARAMETERS) and reused wherever the same schema appears (locations with
parameter arrays at lines shown in the review). Replace the duplicated inline
array in each occurrence (the object with props 'id','name','status','test') by
referencing or spreading the shared constant (for example parameters:
COMMON_PARAMETERS or parameters: [...COMMON_PARAMETERS]) so future changes are
made in one place; update exports/imports in mockServer/src/services/model.js as
needed to use the shared definition.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 49c47084-dbd6-4334-a98a-8194defa6c22
📒 Files selected for processing (4)
mockServer/src/services/model.jspackages/common/component/LifeCycles.vuepackages/plugins/block/src/BlockSetting.vuepackages/plugins/page/src/PageSetting.vue
✅ Files skipped from review due to trivial changes (2)
- packages/plugins/page/src/PageSetting.vue
- packages/common/component/LifeCycles.vue
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
mockServer/src/services/model.js (3)
358-363:⚠️ Potential issue | 🟡 Minor
this.modelListcache is not updated after update.Similar to
delete(), the in-memory cache becomes stale after an update. This affects subsequentcreate()calls that read frommodelList.♻️ Suggested fix
async update(id, params) { + const { id: _ignoredId, ...safeParams } = params // Strip id from params - await this.db.updateAsync({ id: Number(id) }, { $set: params }) + await this.db.updateAsync({ id: Number(id) }, { $set: safeParams }) const result = await this.db.findOneAsync({ id: Number(id) }) + this.modelList = await this.db.findAsync() return getResponseData(result) }The diff also strips
idfrom params to prevent clients from changing a record's primary key, which was flagged in a previous review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mockServer/src/services/model.js` around lines 358 - 363, The update(id, params) method currently updates the DB but does not refresh the in-memory cache this.modelList and also allows params.id to overwrite the primary key; modify update(id, params) to remove/ignore params.id before calling db.updateAsync, then after retrieving the updated row via findOneAsync update the corresponding entry in this.modelList (replace the object with the fresh result or rebuild the list entry with the new fields) so subsequent create() and other methods see the latest data, and finally return getResponseData(result).
339-342:⚠️ Potential issue | 🔴 Critical
this.db.insert()is not awaited — response may return before persistence completes.The insert operation is fire-and-forget. If it fails or is slow, the response already returns stale data. Other services in this codebase (e.g.,
pages.js,blockGroup.js) awaitinsertAsyncto ensure data integrity.🐛 Proposed fix
- this.db.insert(newModel) - this.modelList.push(newModel) - - return getResponseData({ records: this.modelList }) + await this.db.insertAsync(newModel) + this.modelList = await this.db.findAsync() + + return getResponseData({ records: this.modelList })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mockServer/src/services/model.js` around lines 339 - 342, Await the DB insert so the response waits for persistence: replace the fire-and-forget this.db.insert(newModel) with an awaited call (use this.db.insertAsync(newModel) if available, or await this.db.insert(...) and make the enclosing function async), keep pushing to this.modelList only after successful insert, and propagate/handle errors (reject or return an error response) before calling return getResponseData({ records: this.modelList }).
345-350:⚠️ Potential issue | 🟡 Minor
this.modelListcache is not updated after deletion.After
removeAsync, the in-memorymodelListstill contains the deleted record. Subsequent calls tocreate()will computemockIdfrom stale data, and iflist()hasn't been called, the cache remains inconsistent.♻️ Suggested fix
async delete(id) { const result = await this.db.findOneAsync({ id: Number(id) }) await this.db.removeAsync({ id: Number(id) }) + this.modelList = await this.db.findAsync() return getResponseData(result) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mockServer/src/services/model.js` around lines 345 - 350, The delete(id) method removes the record from the DB but never updates the in-memory cache this.modelList, causing stale data (and wrong mockId in create()); after calling this.db.removeAsync(...) update this.modelList by removing the matching entry (or invalidate it so list() reloads) so that subsequent create() and list() use consistent state—modify the delete method (and any related cache-invalidation logic) to either splice out the item from this.modelList or set this.modelList = null/[] after successful deletion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mockServer/src/services/model.js`:
- Line 13: The import has a typo: change the default import name from DateStore
to Datastore to match the `@seald-io/nedb` export, and update any instantiation
sites (e.g., where new DateStore(...) is used) to use Datastore instead (refer
to the import statement and the constructor call around the model
initialization/instantiation). Ensure the import reads "import Datastore from
'@seald-io/nedb'" and all references (constructor calls and variables) use the
exact symbol "Datastore".
---
Duplicate comments:
In `@mockServer/src/services/model.js`:
- Around line 358-363: The update(id, params) method currently updates the DB
but does not refresh the in-memory cache this.modelList and also allows
params.id to overwrite the primary key; modify update(id, params) to
remove/ignore params.id before calling db.updateAsync, then after retrieving the
updated row via findOneAsync update the corresponding entry in this.modelList
(replace the object with the fresh result or rebuild the list entry with the new
fields) so subsequent create() and other methods see the latest data, and
finally return getResponseData(result).
- Around line 339-342: Await the DB insert so the response waits for
persistence: replace the fire-and-forget this.db.insert(newModel) with an
awaited call (use this.db.insertAsync(newModel) if available, or await
this.db.insert(...) and make the enclosing function async), keep pushing to
this.modelList only after successful insert, and propagate/handle errors (reject
or return an error response) before calling return getResponseData({ records:
this.modelList }).
- Around line 345-350: The delete(id) method removes the record from the DB but
never updates the in-memory cache this.modelList, causing stale data (and wrong
mockId in create()); after calling this.db.removeAsync(...) update
this.modelList by removing the matching entry (or invalidate it so list()
reloads) so that subsequent create() and list() use consistent state—modify the
delete method (and any related cache-invalidation logic) to either splice out
the item from this.modelList or set this.modelList = null/[] after successful
deletion.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 92d7e13a-4936-4dca-a100-df5ec363e644
📒 Files selected for processing (1)
mockServer/src/services/model.js
chilingling
previously approved these changes
Mar 31, 2026
hexqi
approved these changes
Mar 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
New Features
Improvements
Style