-
Notifications
You must be signed in to change notification settings - Fork 3
Tag update #326
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.
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
base: main
Are you sure you want to change the base?
Tag update #326
Changes from 5 commits
b3a76ca
a28c2ca
ac262e5
e22e4e4
f83ea3c
791ae67
8e22c82
d972adf
60efd6f
c28a60d
33e9adb
1c544b7
14b5c5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,44 @@ | ||||||
| import * as dotenv from 'dotenv'; | ||||||
|
|
||||||
| dotenv.config(); | ||||||
|
|
||||||
| import { Collection, Db, MongoClient } from 'mongodb'; | ||||||
|
|
||||||
| const { MONGO_URL = 'mongodb://localhost:27017/erxes?directConnection=true' } = | ||||||
| process.env; | ||||||
|
|
||||||
| if (!MONGO_URL) { | ||||||
| throw new Error(`Environment variable MONGO_URL not set.`); | ||||||
| } | ||||||
|
|
||||||
| const client = new MongoClient(MONGO_URL); | ||||||
|
|
||||||
| let db: Db; | ||||||
| let Tags: Collection; | ||||||
|
|
||||||
| const command = async () => { | ||||||
| await client.connect(); | ||||||
| db = client.db() as Db; | ||||||
|
|
||||||
| Tags = db.collection('tags'); | ||||||
|
|
||||||
| try { | ||||||
| await Tags.updateMany( | ||||||
| {}, | ||||||
| { | ||||||
| $unset: { objectCount: '', order: '', scopeBrandIds: '' }, | ||||||
| $set: { isGroup: false, parentId: '' }, // flatten parentId: no nested parent tags allowed | ||||||
|
||||||
| $set: { isGroup: false, parentId: '' }, // flatten parentId: no nested parent tags allowed | |
| $set: { isGroup: false, parentId: null }, // flatten parentId: no nested parent tags allowed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure client connection is closed on success too (e.g., in a finally block) to avoid leaving open connections.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ export const documentSchema = new Schema( | |
| name: { type: String }, | ||
| content: { type: String }, | ||
| replacer: { type: String }, | ||
| code: { type: String }, | ||
| code: { type: String, optional: true }, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainUnique index on an optional field will cause duplicate-key errors; use a partial index and drop "optional".
Apply: - code: { type: String, optional: true },
+ code: { type: String, required: false, trim: true },
-documentSchema.index({ code: 1 }, { unique: true });
+documentSchema.index(
+ { code: 1 },
+ { unique: true, partialFilterExpression: { code: { $exists: true } } }
+);Verification (run in mongosh against the target DB): Also applies to: 18-18 🏁 Script executed: db.getCollection('documents').getIndexes()
# Expect the 'code_1' index to have { unique: true, partialFilterExpression: { code: { $exists: true } } }Length of output: 235 🏁 Script executed: #!/bin/bash
# Print the 'code' field definition and surrounding lines
rg -n "code:\s*\{" -C2 backend/core-api/src/modules/documents/db/definitions/documents.ts
# Print all index definitions on documentSchema
rg -n "documentSchema\.index" -C3 backend/core-api/src/modules/documents/db/definitions/documents.tsLength of output: 398 Use required:false (not optional) and add a partial filter to the unique ‘code’ index Replace the field and index definitions as follows: - code: { type: String, optional: true },
+ code: { type: String, required: false, trim: true },
- documentSchema.index({ code: 1 }, { unique: true });
+ documentSchema.index(
+ { code: 1 },
+ { unique: true, partialFilterExpression: { code: { $exists: true } } }
+ );After deployment, in the Mongo shell run: db.getCollection('documents').getIndexes();and confirm that the 🤖 Prompt for AI Agents |
||
| }, | ||
| { | ||
| timestamps: true, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,32 +5,17 @@ export const tagSchema = schemaWrapper( | |
| new Schema( | ||
| { | ||
| _id: mongooseStringRandomId, | ||
| name: { type: String, label: 'Name' }, | ||
| type: { | ||
| type: String, | ||
| label: 'Type', | ||
| index: true, | ||
| }, | ||
| name: { type: String, label: 'Name', unique: true }, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainGlobal unique on name is risky; scope uniqueness. unique: true enforces cross-tenant/global uniqueness and blocks same-name tags under different groups. Prefer a compound unique index (name, parentId) or include tenant scope (e.g., organizationId). - name: { type: String, label: 'Name', unique: true },
+ name: { type: String, label: 'Name' },
@@
-tagSchema.index({ _id: 1, name: 1, parentId: 1 });
+tagSchema.index({ _id: 1, name: 1, parentId: 1 });
+// Enforce uniqueness only within the same parent (adjust to include tenant scope if applicable)
+tagSchema.index({ name: 1, parentId: 1 }, { unique: true });If a unique index on name already exists in prod, we’ll need a migration to drop it before adding the compound index. I can prepare it. 🏁 Script executed: #!/bin/bash
# Locate the tags.ts schema definition
fd -t f tags.ts
# Preview the top of the file to see field definitions
sed -n '1,200p' backend/core-api/src/modules/tags/db/definitions/tags.ts
# Inspect the schema object and index declarations
rg -n "tagSchema" -C3 backend/core-api/src/modules/tags/db/definitions/tags.ts
rg -n "index" -C3 backend/core-api/src/modules/tags/db/definitions/tags.ts
# Check for tenant or hierarchical scope fields
rg -n "parentId" -C3 backend/core-api/src/modules/tags/db/definitions/tags.ts
rg -n "organizationId" -C3 backend/core-api/src/modules/tags/db/definitions/tags.tsLength of output: 2612 Scope tag name uniqueness to its parent (and tenant)
|
||
| colorCode: { type: String, label: 'Color code' }, | ||
| objectCount: { type: Number, label: 'Object count' }, | ||
| order: { type: String, label: 'Order', index: true }, | ||
| parentId: { | ||
| type: String, | ||
| optional: true, | ||
| index: true, | ||
| label: 'Parent', | ||
| }, | ||
| relatedIds: { | ||
| type: [String], | ||
| optional: true, | ||
| label: 'Children tag ids', | ||
| }, | ||
| parentId: { type: String, label: 'Parent' }, | ||
| relatedIds: { type: [String], label: 'Children tag ids' }, | ||
| isGroup: { type: Boolean, label: 'Is group', default: false }, | ||
| type: { type: String, label: 'Content type' }, | ||
| }, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Fix ineffective indexes; add ones matching query patterns The current compound index starts with - tagSchema.index({ _id: 1, name: 1, parentId: 1, type: 1 });
+ // Efficient lookups by hierarchy and content type
+ tagSchema.index({ parentId: 1 });
+ tagSchema.index({ type: 1, isGroup: 1 });
+ // Optional helper for search-by-name within a parent/type scope
+ tagSchema.index({ parentId: 1, type: 1, name: 1 });Also applies to: 21-21 🤖 Prompt for AI Agents |
||
| { | ||
| timestamps: true, | ||
| }, | ||
| ), | ||
| ); | ||
|
|
||
| // for tags query. increases search speed, avoids in-memory sorting | ||
| tagSchema.index({ _id: 1, type: 1, order: 1, name: 1, createdAt: 1 }); | ||
| tagSchema.index({ _id: 1, name: 1, parentId: 1, type: 1 }); | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,9 +1,8 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { escapeRegExp } from 'erxes-api-shared/utils'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { tagSchema } from '@/tags/db/definitions/tags'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { removeRelatedTagIds, setRelatedTagIds } from '@/tags/utils'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { ITag, ITagDocument } from 'erxes-api-shared/core-types'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Model } from 'mongoose'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { IModels } from '~/connectionResolvers'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { removeRelatedTagIds, setRelatedTagIds } from '@/tags/utils'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { tagSchema } from '@/tags/db/definitions/tags'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface ITagModel extends Model<ITagDocument> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getTag(_id: string): Promise<ITagDocument>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| createTag(doc: ITag): Promise<ITagDocument>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -13,107 +12,93 @@ export interface ITagModel extends Model<ITagDocument> { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const loadTagClass = (models: IModels) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class Tag { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Get a tag | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public static async getTag(_id: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tag = await models.Tags.findOne({ _id }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public static async validate(_id: string | null, doc: ITag) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { name, parentId, isGroup } = doc; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!tag) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error('Tag not found'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tag = await models.Tags.findOne({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $or: [{ _id }, { name }], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return tag; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (tag?.name === name) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (tag?.name === name) { | |
| if (tag?.name === name && tag._id.toString() !== _id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix validation: duplicate-name check, parent/group rules, self-parent, and cycle prevention
Current logic mis-detects duplicates, conflates parent/group constraints, and can block benign edits (e.g., when isGroup is omitted). Consolidate validation using the effective next state, exclude self in duplicate checks, improve errors, and prevent cycles and self-parenting.
Apply:
- public static async validate(_id: string | null, doc: ITag) {
- const { name, parentId, isGroup } = doc;
-
- const tag = await models.Tags.findOne({
- $or: [{ _id }, { name }],
- });
-
- if (tag?.name === name) {
- throw new Error(`A tag named ${name} already exists`);
- }
-
- if (tag?.isGroup && isGroup) {
- throw new Error('Nested group is not allowed 1');
- }
-
- if (_id === parentId) {
- throw new Error('Group cannot be itself');
- }
-
- if (parentId) {
- const parentTag = await models.Tags.findOne({ _id: parentId });
-
- if (!parentTag) {
- throw new Error('Group not found');
- }
-
- if (!parentTag.isGroup) {
- throw new Error('Parent tag must be a group');
- }
-
- if ((isGroup || tag?.isGroup) && parentTag?.isGroup) {
- throw new Error('Nested group is not allowed 2 ');
- }
- }
-
- if (tag) {
- const parentTag = await models.Tags.findOne({ _id: tag.parentId });
- const childTags = await models.Tags.find({ parentId: tag._id });
-
- if (parentTag?.isGroup && isGroup) {
- throw new Error('Nested group is not allowed 3');
- }
-
- if (!isGroup && childTags.length) {
- throw new Error('Group has tags');
- }
- }
- }
+ public static async validate(_id: string | null, doc: ITag) {
+ const { name, parentId, isGroup } = doc;
+
+ // 1) Unique name (exclude self on update)
+ const dup = await models.Tags.findOne({
+ name,
+ ...(!!_id ? { _id: { $ne: _id } } : {}),
+ });
+ if (dup) {
+ throw new Error(`A tag named ${name} already exists`);
+ }
+
+ // 2) Load existing on update and compute effective next state
+ const existing = _id ? await models.Tags.findOne({ _id }) : null;
+ const hasIsGroup = Object.prototype.hasOwnProperty.call(doc, 'isGroup');
+ const hasParentId = Object.prototype.hasOwnProperty.call(doc, 'parentId');
+ const nextIsGroup = hasIsGroup ? !!isGroup : !!existing?.isGroup;
+ const nextParentId = hasParentId ? parentId : existing?.parentId;
+
+ // 3) Self-parent
+ if (_id && nextParentId && String(_id) === String(nextParentId)) {
+ throw new Error('A tag cannot be its own parent');
+ }
+
+ // 4) Parent existence + type + cycle
+ if (nextParentId) {
+ const parentTag = await models.Tags.findOne({ _id: nextParentId });
+ if (!parentTag) {
+ throw new Error('Parent tag not found');
+ }
+ if (!parentTag.isGroup) {
+ throw new Error('Parent tag must be a group');
+ }
+ // Prevent cycles: parent cannot be a descendant of current
+ if (_id && (parentTag.relatedIds || []).includes(_id)) {
+ throw new Error('Cannot set a tag’s parent to its descendant');
+ }
+ }
+
+ // 5) Group cannot have a parent
+ if (nextIsGroup && nextParentId) {
+ throw new Error('Group tag cannot have a parent');
+ }
+
+ if (existing) {
+ // 6) Disallow converting a nested tag into a group
+ if (!existing.isGroup && nextIsGroup && !!existing.parentId) {
+ throw new Error('Cannot convert a nested tag into a group');
+ }
+ // 7) Prevent demoting a group that already has children
+ if (existing.isGroup && !nextIsGroup) {
+ const childCount = await models.Tags.countDocuments({ parentId: _id });
+ if (childCount > 0) {
+ throw new Error(
+ 'Cannot convert a group with children into a non-group',
+ );
+ }
+ }
+ }
+ }📝 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.
| public static async validate(_id: string | null, doc: ITag) { | |
| const { name, parentId, isGroup } = doc; | |
| if (!tag) { | |
| throw new Error('Tag not found'); | |
| } | |
| const tag = await models.Tags.findOne({ | |
| $or: [{ _id }, { name }], | |
| }); | |
| return tag; | |
| } | |
| if (tag?.name === name) { | |
| throw new Error(`A tag named ${name} already exists`); | |
| } | |
| /** | |
| * Create a tag | |
| */ | |
| public static async createTag(doc: ITag) { | |
| const isUnique = await this.validateUniqueness(null, doc.name, doc.type); | |
| if (tag?.isGroup && isGroup) { | |
| throw new Error('Nested group is not allowed 1'); | |
| } | |
| if (!isUnique) { | |
| throw new Error('Tag duplicated'); | |
| if (_id === parentId) { | |
| throw new Error('Group cannot be itself'); | |
| } | |
| const parentTag = await this.getParentTag(doc); | |
| if (parentId) { | |
| const parentTag = await models.Tags.findOne({ _id: parentId }); | |
| // Generatingg order | |
| const order = await this.generateOrder(parentTag, doc); | |
| if (!parentTag) { | |
| throw new Error('Group not found'); | |
| } | |
| const tag = await models.Tags.create({ | |
| ...doc, | |
| order, | |
| createdAt: new Date(), | |
| }); | |
| if (!parentTag.isGroup) { | |
| throw new Error('Parent tag must be a group'); | |
| } | |
| await setRelatedTagIds(models, tag); | |
| if ((isGroup || tag?.isGroup) && parentTag?.isGroup) { | |
| throw new Error('Nested group is not allowed 2 '); | |
| } | |
| } | |
| return tag; | |
| } | |
| if (tag) { | |
| const parentTag = await models.Tags.findOne({ _id: tag.parentId }); | |
| const childTags = await models.Tags.find({ parentId: tag._id }); | |
| /** | |
| * Update Tag | |
| */ | |
| public static async updateTag(_id: string, doc: ITag) { | |
| const isUnique = await this.validateUniqueness( | |
| { _id }, | |
| doc.name, | |
| doc.type, | |
| ); | |
| if (parentTag?.isGroup && isGroup) { | |
| throw new Error('Nested group is not allowed 3'); | |
| } | |
| if (!isUnique) { | |
| throw new Error('Tag duplicated'); | |
| if (!isGroup && childTags.length) { | |
| throw new Error('Group has tags'); | |
| } | |
| } | |
| } | |
| public static async validate(_id: string | null, doc: ITag) { | |
| const { name, parentId, isGroup } = doc; | |
| // 1) Unique name (exclude self on update) | |
| const dup = await models.Tags.findOne({ | |
| name, | |
| ...(!!_id ? { _id: { $ne: _id } } : {}), | |
| }); | |
| if (dup) { | |
| throw new Error(`A tag named ${name} already exists`); | |
| } | |
| // 2) Load existing on update and compute effective next state | |
| const existing = _id ? await models.Tags.findOne({ _id }) : null; | |
| const hasIsGroup = Object.prototype.hasOwnProperty.call(doc, 'isGroup'); | |
| const hasParentId = Object.prototype.hasOwnProperty.call(doc, 'parentId'); | |
| const nextIsGroup = hasIsGroup ? !!isGroup : !!existing?.isGroup; | |
| const nextParentId = hasParentId ? parentId : existing?.parentId; | |
| // 3) Self-parent guard | |
| if (_id && nextParentId && String(_id) === String(nextParentId)) { | |
| throw new Error('A tag cannot be its own parent'); | |
| } | |
| // 4) Parent existence, type, and cycle prevention | |
| if (nextParentId) { | |
| const parentTag = await models.Tags.findOne({ _id: nextParentId }); | |
| if (!parentTag) { | |
| throw new Error('Parent tag not found'); | |
| } | |
| if (!parentTag.isGroup) { | |
| throw new Error('Parent tag must be a group'); | |
| } | |
| // Prevent setting a tag’s parent to its own descendant | |
| if (_id && (parentTag.relatedIds || []).includes(_id)) { | |
| throw new Error('Cannot set a tag’s parent to its descendant'); | |
| } | |
| } | |
| // 5) Disallow any parent on a group | |
| if (nextIsGroup && nextParentId) { | |
| throw new Error('Group tag cannot have a parent'); | |
| } | |
| if (existing) { | |
| // 6) Prevent converting a nested tag into a group | |
| if (!existing.isGroup && nextIsGroup && !!existing.parentId) { | |
| throw new Error('Cannot convert a nested tag into a group'); | |
| } | |
| // 7) Prevent demoting a group that already has children | |
| if (existing.isGroup && !nextIsGroup) { | |
| const childCount = await models.Tags.countDocuments({ parentId: _id }); | |
| if (childCount > 0) { | |
| throw new Error( | |
| 'Cannot convert a group with children into a non-group', | |
| ); | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In backend/core-api/src/modules/tags/db/models/Tags.ts around lines 15 to 62,
compute the effective next state (treat undefined isGroup as existing
tag.isGroup when editing) and then: 1) check for duplicate name excluding the
current _id (findOne({ name, _id: { $ne: _id } })); 2) ensure parentId !== _id
and walk the parent chain to prevent cycles (follow parentId up until null and
throw if any ancestor === _id); 3) verify parent exists and parent.isGroup ===
true; 4) forbid nested groups by disallowing a tag with isGroup === true to have
a parent that isGroup === true; 5) if changing/setting isGroup to false, ensure
there are no child tags (childTags.length === 0) before allowing the change; and
improve throw messages to be specific for each failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (bug_risk): Removing parentId from child tags on tag removal may leave orphaned tags.
Currently, child tags become orphaned when their parent is removed. Please review if this is intentional, or if child tags should be deleted or reassigned to maintain data integrity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call client.close() before process.exit() for proper cleanup of the MongoDB connection.