Skip to content

fix(home): test isolation — duplicate key error #3359

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.

Sign up for GitHub

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

Closed
PierreBrisorgueil wants to merge 4 commits into from
Closed

fix(home): test isolation — duplicate key error #3359

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d914ee5
docs(feature): add ERRORS.md documentation step to self-review checklist
PierreBrisorgueil Mar 30, 2026
d2f9cdb
fix(billing): replace dot separator with underscore in counter keys
PierreBrisorgueil Mar 30, 2026
33c5038
fix(home): isolate test user to prevent duplicate key errors
PierreBrisorgueil Mar 30, 2026
22c411a
fix(billing): update comment to match underscore key format, align ER…
PierreBrisorgueil Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .claude/skills/feature/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ If an action affects another user:
- [ ] Actions affecting other users trigger email (if configured)
- [ ] Templates created for each email type

**Error documentation:**
- [ ] If a non-obvious bug was fixed, document it in `ERRORS.md` (root of repo) with: symptom, root cause, fix
Comment thread
PierreBrisorgueil marked this conversation as resolved.
Outdated

### 8b. Elegance check

For non-trivial changes: pause and ask yourself "is there a simpler or more elegant approach?" If the current implementation feels hacky, refactor before proceeding.
Expand Down
2 changes: 1 addition & 1 deletion modules/billing/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Increment counters after successful operations:
import BillingUsageService from '../billing/services/billing.usage.service.js';

// After creating a document
await BillingUsageService.increment(organizationId, 'documents.create', 1);
await BillingUsageService.increment(organizationId, 'documents_create', 1);
```

### Usage Endpoint
Expand Down
2 changes: 1 addition & 1 deletion modules/billing/controllers/billing.controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ const getUsage = async (req, res) => {
for (const resource of Object.keys(planQuotas)) {
for (const action of Object.keys(planQuotas[resource])) {
const rawLimit = planQuotas[resource][action];
limits[`${resource}.${action}`] = Number.isFinite(rawLimit) ? rawLimit : null;
limits[`${resource}_${action}`] = Number.isFinite(rawLimit) ? rawLimit : null;
}
Comment thread
PierreBrisorgueil marked this conversation as resolved.
}
}
Expand Down
2 changes: 1 addition & 1 deletion modules/billing/middlewares/billing.requireQuota.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function requireQuota(resource, action) {

// Check current usage
const usage = await BillingUsageService.get(req.organization._id.toString());
const counterKey = `${resource}.${action}`;
const counterKey = `${resource}_${action}`;
const current = usage.counters[counterKey] || 0;
Comment thread
PierreBrisorgueil marked this conversation as resolved.

if (current >= limit) {
Expand Down
14 changes: 7 additions & 7 deletions modules/billing/tests/billing.quota.unit.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('requireQuota middleware:', () => {

test('should allow request when under quota', async () => {
mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 1 } });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 1 } });

await requireQuota('scraps', 'create')(req, res, next);

Expand All @@ -79,7 +79,7 @@ describe('requireQuota middleware:', () => {

test('should return 429 when at quota limit', async () => {
mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 3 } });

await requireQuota('scraps', 'create')(req, res, next);

Expand All @@ -95,7 +95,7 @@ describe('requireQuota middleware:', () => {

test('should return 429 when over quota limit', async () => {
mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 5 } });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 5 } });

await requireQuota('scraps', 'create')(req, res, next);

Expand All @@ -105,7 +105,7 @@ describe('requireQuota middleware:', () => {

test('should treat missing subscription as free plan', async () => {
mockSubscriptionRepository.findByOrganization.mockResolvedValue(null);
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 3 } });

await requireQuota('scraps', 'create')(req, res, next);

Expand All @@ -121,7 +121,7 @@ describe('requireQuota middleware:', () => {
'should treat %s subscription as free plan',
async (status) => {
mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 3 } });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 3 } });

await requireQuota('scraps', 'create')(req, res, next);

Expand All @@ -145,7 +145,7 @@ describe('requireQuota middleware:', () => {

test('should return correct error payload with upgradeUrl', async () => {
mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'free', status: 'active' });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.execute': 100 } });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_execute': 100 } });

await requireQuota('scraps', 'execute')(req, res, next);

Expand Down Expand Up @@ -178,7 +178,7 @@ describe('requireQuota middleware:', () => {

test('should use subscription plan when status is trialing', async () => {
mockSubscriptionRepository.findByOrganization.mockResolvedValue({ plan: 'starter', status: 'trialing' });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps.create': 15 } });
mockBillingUsageService.get.mockResolvedValue({ counters: { 'scraps_create': 15 } });

await requireQuota('scraps', 'create')(req, res, next);

Expand Down
16 changes: 8 additions & 8 deletions modules/billing/tests/billing.usage.endpoint.unit.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('Billing usage endpoint unit tests:', () => {

test('should return usage and limits for active subscription', async () => {
mockBillingService.getSubscription.mockResolvedValue({ plan: 'starter', status: 'active' });
mockBillingUsageService.get.mockResolvedValue({ month: '2026-03', counters: { 'documents.create': 5, 'requests.execute': 42 } });
mockBillingUsageService.get.mockResolvedValue({ month: '2026-03', counters: { 'documents_create': 5, 'requests_execute': 42 } });

const req = { organization: { _id: orgId } };
await billingController.getUsage(req, res);
Expand All @@ -74,8 +74,8 @@ describe('Billing usage endpoint unit tests:', () => {
message: 'billing usage',
data: expect.objectContaining({
plan: 'starter',
usage: { 'documents.create': 5, 'requests.execute': 42 },
limits: { 'documents.create': 20, 'requests.execute': 2000 },
usage: { 'documents_create': 5, 'requests_execute': 42 },
limits: { 'documents_create': 20, 'requests_execute': 2000 },
}),
}));
});
Expand All @@ -92,7 +92,7 @@ describe('Billing usage endpoint unit tests:', () => {
type: 'success',
data: expect.objectContaining({
plan: 'free',
limits: { 'documents.create': 5, 'requests.execute': 100 },
limits: { 'documents_create': 5, 'requests_execute': 100 },
}),
}));
});
Expand All @@ -106,7 +106,7 @@ describe('Billing usage endpoint unit tests:', () => {
'paused',
])('should return free plan when subscription status is %s', async (status) => {
mockBillingService.getSubscription.mockResolvedValue({ plan: 'starter', status });
mockBillingUsageService.get.mockResolvedValue({ month: '2026-03', counters: { 'documents.create': 2 } });
mockBillingUsageService.get.mockResolvedValue({ month: '2026-03', counters: { 'documents_create': 2 } });

const req = { organization: { _id: orgId } };
await billingController.getUsage(req, res);
Expand All @@ -115,7 +115,7 @@ describe('Billing usage endpoint unit tests:', () => {
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
data: expect.objectContaining({
plan: 'free',
limits: { 'documents.create': 5, 'requests.execute': 100 },
limits: { 'documents_create': 5, 'requests_execute': 100 },
}),
}));
});
Expand All @@ -131,7 +131,7 @@ describe('Billing usage endpoint unit tests:', () => {
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
data: expect.objectContaining({
plan: 'starter',
limits: { 'documents.create': 20, 'requests.execute': 2000 },
limits: { 'documents_create': 20, 'requests_execute': 2000 },
}),
}));
});
Expand Down Expand Up @@ -163,7 +163,7 @@ describe('Billing usage endpoint unit tests:', () => {
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
data: expect.objectContaining({
plan: 'pro',
limits: { 'documents.create': null, 'requests.execute': null },
limits: { 'documents_create': null, 'requests_execute': null },
}),
}));
});
Expand Down
16 changes: 12 additions & 4 deletions modules/home/tests/home.integration.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe('Home integration tests:', () => {
let agent;
let HomeService;
let adminToken;
let adminUser;
let originalOrganizationsEnabled;

// init
Expand All @@ -41,15 +42,16 @@ describe('Home integration tests:', () => {

// Create admin user and sign JWT for health endpoint test
const User = mongoose.model('User');
const admin = await User.create({
await User.deleteOne({ email: 'admin-home-health@test.com' });
adminUser = await User.create({
firstName: 'Admin',
lastName: 'Health',
email: 'admin-health@test.com',
email: 'admin-home-health@test.com',
password: 'W@os.jsI$Aw3$0m3',
provider: 'local',
roles: ['admin'],
});
adminToken = jwt.sign({ userId: admin.id }, config.jwt.secret, { expiresIn: config.jwt.expiresIn });
adminToken = jwt.sign({ userId: adminUser.id }, config.jwt.secret, { expiresIn: config.jwt.expiresIn });
} catch (err) {
console.log(err);
expect(err).toBeFalsy();
Expand Down Expand Up @@ -204,10 +206,16 @@ describe('Home integration tests:', () => {
});
});

// Mongoose disconnect
// Cleanup and mongoose disconnect
afterAll(async () => {
jest.restoreAllMocks();
config.organizations.enabled = originalOrganizationsEnabled;
try {
if (adminUser) {
const User = mongoose.model('User');
await User.deleteOne({ _id: adminUser._id });
}
} catch (_) { /* cleanup – ignore errors */ }
try {
await mongooseService.disconnect();
} catch (err) {
Expand Down
Loading