fix(core): Administrator soft delete constraint error (#1523) #3890

mirena-iud · 2025-10-23T03:17:43Z

Description

Fixes administrator soft delete constraint error by adding combined unqiue index. #1523

Breaking changes

None.

Screenshots

None.

Checklist

📌 Always:

I have set a clear title
My PR is small and contains a single feature
I have checked my own PR

👍 Most of the time:

I have added or updated test cases
I have updated the README if needed

Summary by CodeRabbit

Bug Fixes
- Improved administrator record deletion handling to properly clear status when removing administrators.
Chores
- Enhanced administrator tracking system to distinguish between active and inactive records with refined uniqueness constraints.

…rce#1523)

vercel · 2025-10-23T03:17:48Z

Someone is attempting to deploy a commit to the Vendure Team on Vercel.

A member of the Team first needs to authorize it.

vercel · 2025-10-23T03:17:51Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Updated (UTC)
docs	Ready	Preview	Oct 23, 2025 3:20am

coderabbitai · 2025-10-23T03:17:58Z

Walkthrough

Modified the Administrator entity to use a composite unique constraint on emailAddress and isCurrent instead of a unique constraint on emailAddress alone. Updated the soft delete operation to null the isCurrent field alongside deletedAt, enabling email address reuse when an administrator is soft-deleted.

Changes

Cohort / File(s)	Summary
Administrator Entity Schema `packages/core/src/entity/administrator/administrator.entity.ts`	Added `Unique` import from TypeORM. Applied `@Unique(['emailAddress', 'isCurrent'])` class decorator. Removed `unique: true` constraint from `emailAddress` column. Introduced new `isCurrent` boolean column (default `true`, nullable).
Administrator Service Soft Delete `packages/core/src/service/services/administrator.service.ts`	Modified soft delete operation to set both `isCurrent` to `null` and `deletedAt` when soft-deleting an Administrator record.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "fix(core): Administrator soft delete constraint error (#1523)" directly aligns with the primary changes in the changeset. The modifications to the Administrator entity (adding a composite unique constraint on `emailAddress` and `isCurrent`, and updating the soft delete operation to clear the `isCurrent` field) are specifically designed to resolve the soft delete constraint error referenced in the title. The title is concise, follows conventional commit format, includes the issue reference, and clearly communicates the main purpose of the changes to reviewers scanning the commit history.
Description Check	✅ Passed	The pull request description follows the required template structure, with all main sections present and completed: the Description section provides a summary linking the fix to the combined unique index and issue #1523, Breaking changes are explicitly stated as "None", Screenshots are noted as "None" (appropriate since not applicable), and all checklist items are addressed. While the description section is brief, it adequately summarizes what was changed and why, and the template does not mandate extensive detail in this section. The author has properly completed all required elements of the template.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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

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

github-actions · 2025-10-23T03:17:59Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

sonarqubecloud · 2025-10-23T03:18:55Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

coderabbitai

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)

packages/core/src/service/services/administrator.service.ts (2)
349-352: Critical: Restore logic must also set isCurrent = true.

When restoring a soft-deleted administrator, the code sets deletedAt = null but does NOT restore isCurrent back to true. This is a bug because:

The restored administrator will have deletedAt = null but isCurrent = null

This breaks the intended pattern where active administrators should have isCurrent = true

This could prevent creating new administrators with the same email if the restored one has isCurrent = null

Apply this diff to fix the restore logic:
 } else if (superAdministrator.deletedAt != null) {
     superAdministrator.deletedAt = null;
+    superAdministrator.isCurrent = true;
     await this.connection.rawConnection.getRepository(Administrator).save(superAdministrator);
 }
256-273: Restore operations missing isCurrent restoration.

The soft-delete sets both isCurrent: null and deletedAt, but restore operations at lines 349–350 and 354–356 only restore deletedAt to null, leaving isCurrent as null. This creates an inconsistent state where the administrator appears deleted for uniqueness enforcement (due to the @Unique(['emailAddress', 'isCurrent']) constraint) while deletedAt is cleared.

Update the restore logic to also set isCurrent = true:
} else if (superAdministrator.deletedAt != null) {
    superAdministrator.deletedAt = null;
    superAdministrator.isCurrent = true;
    await this.connection.rawConnection.getRepository(Administrator).save(superAdministrator);
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 999d3e1 and c0b3acf.

📒 Files selected for processing (2)

packages/core/src/entity/administrator/administrator.entity.ts (3 hunks)
packages/core/src/service/services/administrator.service.ts (1 hunks)

🔇 Additional comments (3)

packages/core/src/entity/administrator/administrator.entity.ts (1)

2-2: LGTM! Import needed for composite constraint.

The Unique import is correctly added to support the class-level composite unique constraint.

packages/core/src/service/services/administrator.service.ts (2)

264-266: LGTM! Soft delete correctly updates both fields.

The soft delete operation now correctly sets both isCurrent: null and deletedAt, which is essential for the composite unique constraint to allow email address reuse.

354-357: No action required - the review comment is based on incorrect assumptions.

The User entity structure is fundamentally different from Administrator and does not have an isCurrent field or composite unique constraints. User only implements soft-deletion via deletedAt, which is already handled correctly by userService.softDelete(). The restoration logic at lines 354-357 properly handles both entities according to their respective designs: Administrator resets deletedAt, and User resets deletedAt (with no isCurrent to manage).

Likely an incorrect or invalid review comment.

packages/core/src/entity/administrator/administrator.entity.ts

coderabbitai · 2025-10-23T03:23:03Z

packages/core/src/entity/administrator/administrator.entity.ts

+    @Column({ type: 'boolean', default: true, nullable: true })
+    isCurrent: true | null;


⚠️ Potential issue | 🟠 Major

Verify type safety: TypeScript true | null vs SQL boolean.

The TypeScript type is true | null, but the column type is boolean. This mismatch could cause issues:

SQL boolean columns can store true, false, or null

If the database ever returns false (through direct DB manipulation or ORM behavior), TypeScript will not expect it

TypeScript will show type errors when assigning boolean values to this field

Consider using boolean | null for better type safety, and enforce the constraint through validation logic or a database CHECK constraint if you truly want to prevent false values.

If you want to keep the current pattern, add a comment explaining why only true and null are valid values.

+ // isCurrent can only be true (for active administrators) or null (for soft-deleted ones) + // This constraint allows multiple soft-deleted administrators with the same email address @Column({ type: 'boolean', default: true, nullable: true }) isCurrent: true | null;

🤖 Prompt for AI Agents

In packages/core/src/entity/administrator/administrator.entity.ts around lines 35-36, the property is declared as TypeScript type `true | null` while the Column is defined as SQL `boolean`; change the TS type to `boolean | null` to match the database semantics (allow true, false, or null) or, if you truly require only true/null, keep `true | null` but add a clear inline comment explaining the restriction and add application-level validation or a DB CHECK constraint to prevent false values; ensure any code that sets/reads this field is updated to the chosen type/validation approach.

mirena-iud · 2025-10-23T03:27:37Z

I have read the CLA Document and I hereby sign the CLA

mirena-iud · 2025-10-23T03:29:08Z

recheck

dlhck

I don't think this is the correct approach. We should rather remove the unique constraint on the emailAddress and do the proper exist-checks in the create and update in the administrator.service.ts. Similar to what we have in customer.service.ts

fix(core): Administrator soft delete constraint error (vendure-ecomme…

c0b3acf

…rce#1523)

vercel bot deployed to Preview – docs October 23, 2025 03:20 View deployment

coderabbitai bot reviewed Oct 23, 2025

View reviewed changes

github-actions bot added a commit that referenced this pull request Oct 23, 2025

@mirena-iud has signed the CLA in #3890

ad7edab

dlhck requested changes Nov 3, 2025

View reviewed changes

casperiv0 pushed a commit to casperiv0/vendure that referenced this pull request Nov 3, 2025

@mirena-iud has signed the CLA in vendure-ecommerce#3890

88d16d2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(core): Administrator soft delete constraint error (#1523) #3890

fix(core): Administrator soft delete constraint error (#1523) #3890

mirena-iud commented Oct 23, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

vercel bot commented Oct 23, 2025

Uh oh!

vercel bot commented Oct 23, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Oct 23, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Oct 23, 2025 •

edited

Loading

Uh oh!

sonarqubecloud bot commented Oct 23, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot Oct 23, 2025

Uh oh!

mirena-iud commented Oct 23, 2025

Uh oh!

mirena-iud commented Oct 23, 2025

Uh oh!

dlhck left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@Column({ type: 'boolean', default: true, nullable: true })
		isCurrent: true \| null;

Uh oh!

fix(core): Administrator soft delete constraint error (#1523) #3890

Are you sure you want to change the base?

fix(core): Administrator soft delete constraint error (#1523) #3890

Conversation

mirena-iud commented Oct 23, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Breaking changes

Screenshots

Checklist

Summary by CodeRabbit

Uh oh!

vercel bot commented Oct 23, 2025

Uh oh!

vercel bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

github-actions bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Oct 23, 2025

Quality Gate passed

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

mirena-iud commented Oct 23, 2025

Uh oh!

mirena-iud commented Oct 23, 2025

Uh oh!

dlhck left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mirena-iud commented Oct 23, 2025 •

edited by coderabbitai bot

Loading

vercel bot commented Oct 23, 2025 •

edited

Loading

coderabbitai bot commented Oct 23, 2025 •

edited

Loading

github-actions bot commented Oct 23, 2025 •

edited

Loading