Fix：Error message when creating the same page for different applications

[lichunn]

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our Commit Message Guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)
• Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• Other... Please describe:

Background and solution

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

• Yes
• No

Other information

Summary by CodeRabbit

• New Features

  • Routes are now automatically generated from page names when not explicitly provided.

• Bug Fixes

  • Added validation to prevent duplicate routes; conflicts now return a clear error message.
  • Improved page creation logic with enhanced uniqueness enforcement.

[coderabbitai]

Walkthrough

Modified the pages service to enforce route uniqueness through a pre-insert existence check rather than a database index constraint. Added route defaulting logic that falls back to page name or "Untitled" when route is not provided. Adjusted the unique index from route to _id.

Changes

Cohort / File(s)	Summary
Route Uniqueness & Defaulting Logic mockServer/src/services/pages.js	Changed NeDB unique index from route field to _id field. Added route defaulting in create() (uses pageData.name or 'Untitled' when falsy). Implemented pre-insert existence check to prevent duplicate routes for the same app, returning a 409 ROUTE_CONFLICT error if a match is found.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hops with glee through route-filled fields,
No more duplicate path yields!
Defaults spring forth like clover green,
The finest validation yet seen. 🌿✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main issue being fixed: preventing errors when creating pages with the same name/route in different applications by changing uniqueness enforcement from route to document ID.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

mockServer/src/services/pages.js (2)

34-37: Redundant unique index on _id.

NeDB automatically enforces uniqueness on _id by default. This explicit index declaration has no effect and can be removed.

♻️ Suggested fix

-    this.db.ensureIndex({
-      fieldName: '_id',
-      unique: true
-    })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@mockServer/src/services/pages.js` around lines 34 - 37, Remove the redundant
explicit unique index creation on `_id` by deleting the this.db.ensureIndex({
fieldName: '_id', unique: true }) call in the pages service (where this.db is
initialized). NeDB enforces uniqueness for `_id` automatically, so remove that
ensureIndex invocation (or guard it with a comment if you want to retain
historical context) and keep any other index setup intact.

---

111-120: Consider adding route conflict validation to update as well.

The create method now validates route uniqueness per app, but update does not. If a page's route is updated to conflict with another page in the same app, the duplicate will be allowed.

For consistency, consider adding similar validation when params.route is being changed.

♻️ Suggested fix

   async update(id, params) {
     const updateData = { ...params }
+
+    if (updateData.route) {
+      const current = await this.db.findOneAsync({ _id: id })
+      if (current) {
+        const existing = await this.db.findOneAsync({
+          app: current.app.toString(),
+          route: updateData.route,
+          _id: { $ne: id }
+        })
+        if (existing) {
+          return getResponseData(null, {
+            code: 'ROUTE_CONFLICT',
+            message: `Route "${updateData.route}" already exists in app "${current.app}"`
+          })
+        }
+      }
+    }
+
     if (updateData.page_content && typeof updateData.page_content === 'object') {
       updateData.page_content = JSON.stringify(updateData.page_content)
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@mockServer/src/services/pages.js` around lines 111 - 120, The update method
(async update(id, params)) needs the same route-uniqueness check as create: when
params.route is provided and differs from the current page's route, query the DB
for any other document with the same app and route (e.g., this.db.findOneAsync({
app: existing.app, route: params.route, _id: { $ne: id } })); if such a document
exists, throw/return a conflict error instead of proceeding to
this.db.updateAsync; otherwise proceed to stringify page_content as currently
done, update, and return getResponseData(parsePageContent(result)).

🤖 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/pages.js`:
- Around line 92-98: The getResponseData helper currently ignores an
error.status so the 409 from pages.js is never applied; update getResponseData
(function getResponseData in mockServer/src/tool/Common.js) to read and return a
status field from the error object (alongside code and message) so callers like
the return in mockServer/src/services/pages.js that call getResponseData(...)
receive the status and the HTTP layer can use it, or alternatively set
ctx.status (or the equivalent HTTP response status) in the pages.js controller
before returning getResponseData; prefer modifying getResponseData to include
status for consistent handling across callers.

---

Nitpick comments:
In `@mockServer/src/services/pages.js`:
- Around line 34-37: Remove the redundant explicit unique index creation on
`_id` by deleting the this.db.ensureIndex({ fieldName: '_id', unique: true })
call in the pages service (where this.db is initialized). NeDB enforces
uniqueness for `_id` automatically, so remove that ensureIndex invocation (or
guard it with a comment if you want to retain historical context) and keep any
other index setup intact.
- Around line 111-120: The update method (async update(id, params)) needs the
same route-uniqueness check as create: when params.route is provided and differs
from the current page's route, query the DB for any other document with the same
app and route (e.g., this.db.findOneAsync({ app: existing.app, route:
params.route, _id: { $ne: id } })); if such a document exists, throw/return a
conflict error instead of proceeding to this.db.updateAsync; otherwise proceed
to stringify page_content as currently done, update, and return
getResponseData(parsePageContent(result)).

🪄 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: ade700fe-9126-4641-bc48-70d49e2cd193

📥 Commits

Reviewing files that changed from the base of the PR and between e0821bc and 72656a8.

⛔ Files ignored due to path filters (1)

• mockServer/src/database/pages.db is excluded by !**/*.db

📒 Files selected for processing (1)

• mockServer/src/services/pages.js