feat: agregar lógica para evitar duplicados de clientes por email y vincular usuarios existentes
This commit is contained in:
@@ -0,0 +1,8 @@
|
|||||||
|
db.clients.aggregate([
|
||||||
|
{ $group: {
|
||||||
|
_id: { companyId: "$companyId", email: { $toLower: "$email" } },
|
||||||
|
count: { $sum: 1 },
|
||||||
|
emails: { $addToSet: "$email" }
|
||||||
|
} },
|
||||||
|
{ $match: { count: { $gt: 1 }, $expr: { $gt: [{ $size: "$emails" }, 1] } } }
|
||||||
|
])
|
||||||
@@ -32,6 +32,14 @@ export class ClientsAdapterMongoose implements IClientsAdapter {
|
|||||||
banned: { type: Boolean, required: false, default: false },
|
banned: { type: Boolean, required: false, default: false },
|
||||||
});
|
});
|
||||||
|
|
||||||
|
//Unicidad por email dentro de la organización. La unicidad por usuario es
|
||||||
|
//parcial: excluye los clientes cuyo userId fue removido con $unset.
|
||||||
|
this.schema.index({ companyId: 1, email: 1 }, { unique: true });
|
||||||
|
this.schema.index(
|
||||||
|
{ companyId: 1, userId: 1 },
|
||||||
|
{ unique: true, partialFilterExpression: { userId: { $type: "oid" } } }
|
||||||
|
);
|
||||||
|
|
||||||
this.clientList = model<IClientDocument>("Client", this.schema);
|
this.clientList = model<IClientDocument>("Client", this.schema);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -266,6 +266,27 @@ class ClientManager implements IClientsManager {
|
|||||||
throw new Error("El cliente ya pertenece a la compañia");
|
throw new Error("El cliente ya pertenece a la compañia");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
//Si la organización ya tiene un cliente con el email del usuario, se
|
||||||
|
//vincula el usuario a ese cliente en lugar de crear un duplicado.
|
||||||
|
const existingByEmail = await this.clients.findOne({
|
||||||
|
companyId: data.companyId,
|
||||||
|
email: userCheck.email,
|
||||||
|
});
|
||||||
|
|
||||||
|
if (existingByEmail) {
|
||||||
|
existingByEmail.userId = data.userId;
|
||||||
|
await existingByEmail.save();
|
||||||
|
|
||||||
|
await NotificationsManager.sendSystemNotification({
|
||||||
|
userId: String(userCheck._id),
|
||||||
|
subject: "Ha sido dado de alta como cliente",
|
||||||
|
message: `Su usuario ha sido dado de alta como cliente en la organización ${companyCheck.name}`,
|
||||||
|
type: NotificationType.BILLING
|
||||||
|
});
|
||||||
|
|
||||||
|
return existingByEmail;
|
||||||
|
}
|
||||||
|
|
||||||
const canAdd = await MetricsList.canAddClient(companyCheck.ownerId);
|
const canAdd = await MetricsList.canAddClient(companyCheck.ownerId);
|
||||||
|
|
||||||
if (!canAdd) {
|
if (!canAdd) {
|
||||||
@@ -274,17 +295,36 @@ class ClientManager implements IClientsManager {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
const newClient = await this.clients.create({
|
let newClient: IClientDocument;
|
||||||
...data,
|
|
||||||
...{
|
try {
|
||||||
email: userCheck.email,
|
newClient = await this.clients.create({
|
||||||
firstName: userCheck.firstName,
|
...data,
|
||||||
lastName: userCheck.lastName,
|
...{
|
||||||
phoneAreaCode: userCheck.phoneAreaCode,
|
email: userCheck.email,
|
||||||
phoneCountryCode: userCheck.phoneCountryCode,
|
firstName: userCheck.firstName,
|
||||||
phoneNumber: userCheck.phoneNumber,
|
lastName: userCheck.lastName,
|
||||||
},
|
phoneAreaCode: userCheck.phoneAreaCode,
|
||||||
});
|
phoneCountryCode: userCheck.phoneCountryCode,
|
||||||
|
phoneNumber: userCheck.phoneNumber,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
} catch (error) {
|
||||||
|
//Carrera de concurrencia: el índice único rechazó el duplicado y la
|
||||||
|
//petición ganadora ya creó el cliente, lo contabilizó y notificó.
|
||||||
|
if ((error as { code?: number })?.code === 11000) {
|
||||||
|
const racedClient = await this.clients.findOne({
|
||||||
|
companyId: data.companyId,
|
||||||
|
userId: data.userId,
|
||||||
|
});
|
||||||
|
|
||||||
|
if (racedClient) {
|
||||||
|
return racedClient;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
|
|
||||||
await MetricsList.addClient({
|
await MetricsList.addClient({
|
||||||
userId: String(companyCheck.ownerId),
|
userId: String(companyCheck.ownerId),
|
||||||
|
|||||||
@@ -0,0 +1,284 @@
|
|||||||
|
// Unit tests for ClientManager.createClientByUser: adopt-by-email path, create
|
||||||
|
// path, the existing userId guard, and the E11000 concurrent-create race.
|
||||||
|
|
||||||
|
// Mock all external dependencies before importing the module under test
|
||||||
|
jest.mock("../../Users/Users", () => ({
|
||||||
|
__esModule: true,
|
||||||
|
default: {
|
||||||
|
users: {
|
||||||
|
findOne: jest.fn(),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
jest.mock("../../Companies/Companies", () => ({
|
||||||
|
__esModule: true,
|
||||||
|
default: {
|
||||||
|
companies: {
|
||||||
|
findOne: jest.fn(),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
jest.mock("../../Employees/Employee", () => ({
|
||||||
|
__esModule: true,
|
||||||
|
default: {
|
||||||
|
checkPermission: jest.fn(),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
jest.mock("../../Metrics/Metrics", () => ({
|
||||||
|
__esModule: true,
|
||||||
|
default: {
|
||||||
|
canAddClient: jest.fn(),
|
||||||
|
addClient: jest.fn(),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
jest.mock("../../Notifications/Notifications", () => ({
|
||||||
|
__esModule: true,
|
||||||
|
NotificationsManager: {
|
||||||
|
sendSystemNotification: jest.fn(),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
jest.mock("../../Appointments/Appointments", () => ({
|
||||||
|
__esModule: true,
|
||||||
|
default: {
|
||||||
|
Appointments: {
|
||||||
|
find: jest.fn().mockResolvedValue([]),
|
||||||
|
findOne: jest.fn(),
|
||||||
|
delete: jest.fn(),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
jest.mock("../../Repeats/Repeats", () => ({
|
||||||
|
__esModule: true,
|
||||||
|
default: {
|
||||||
|
deleteRepeatsByClient: jest.fn(),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
jest.mock("../../ClientAccounts/ClientAccount", () => ({
|
||||||
|
__esModule: true,
|
||||||
|
default: {
|
||||||
|
getAvailableAmount: jest.fn(),
|
||||||
|
deleteClientAccount: jest.fn(),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
jest.mock("../../Payments/Payments", () => ({
|
||||||
|
__esModule: true,
|
||||||
|
default: {
|
||||||
|
deletePaymentsByClient: jest.fn(),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
import ClientList from "../Clients";
|
||||||
|
import UsersManager from "../../Users/Users";
|
||||||
|
import CompaniesManager from "../../Companies/Companies";
|
||||||
|
import MetricsList from "../../Metrics/Metrics";
|
||||||
|
import { NotificationsManager } from "../../Notifications/Notifications";
|
||||||
|
import { NotificationType } from "../../SystemNotifications/SystemNotification.Interface";
|
||||||
|
|
||||||
|
const request = {
|
||||||
|
companyId: "company-001",
|
||||||
|
userId: "user-001",
|
||||||
|
sessionUser: "session-user",
|
||||||
|
};
|
||||||
|
|
||||||
|
const sessionUser = { _id: "session-user" };
|
||||||
|
|
||||||
|
const company = {
|
||||||
|
_id: "company-001",
|
||||||
|
ownerId: "owner-user",
|
||||||
|
name: "Acme",
|
||||||
|
};
|
||||||
|
|
||||||
|
const user = {
|
||||||
|
_id: "user-001",
|
||||||
|
email: "client@example.com",
|
||||||
|
firstName: "John",
|
||||||
|
lastName: "Doe",
|
||||||
|
phoneAreaCode: "11",
|
||||||
|
phoneCountryCode: "54",
|
||||||
|
phoneNumber: "12345678",
|
||||||
|
};
|
||||||
|
|
||||||
|
// Helper to build a mock client document with save()
|
||||||
|
function buildClientDoc(overrides: Record<string, any> = {}): Record<string, any> {
|
||||||
|
return {
|
||||||
|
_id: "client-001",
|
||||||
|
id: "client-001",
|
||||||
|
companyId: "company-001",
|
||||||
|
email: "client@example.com",
|
||||||
|
firstName: "Admin Entered Name",
|
||||||
|
lastName: "Admin Entered Lastname",
|
||||||
|
save: jest.fn().mockResolvedValue(true),
|
||||||
|
...overrides,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("createClientByUser", () => {
|
||||||
|
// Keep the real adapter to restore it after each test and to inspect indexes
|
||||||
|
const realAdapter = ClientList.clients;
|
||||||
|
let mockFindOne: jest.Mock;
|
||||||
|
let mockCreate: jest.Mock;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
jest.clearAllMocks();
|
||||||
|
|
||||||
|
mockFindOne = jest.fn();
|
||||||
|
mockCreate = jest.fn();
|
||||||
|
|
||||||
|
(ClientList as any).clients = {
|
||||||
|
findOne: mockFindOne,
|
||||||
|
create: mockCreate,
|
||||||
|
};
|
||||||
|
|
||||||
|
// First call: session user validation. Second call: target user lookup.
|
||||||
|
(UsersManager.users.findOne as jest.Mock)
|
||||||
|
.mockResolvedValueOnce(sessionUser)
|
||||||
|
.mockResolvedValueOnce(user);
|
||||||
|
|
||||||
|
(CompaniesManager.companies.findOne as jest.Mock).mockResolvedValue(company);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
(ClientList as any).clients = realAdapter;
|
||||||
|
});
|
||||||
|
|
||||||
|
// --- Adopt path: existing client by email ---
|
||||||
|
|
||||||
|
it("adopts the existing client found by email instead of creating a duplicate", async () => {
|
||||||
|
const existingClient = buildClientDoc();
|
||||||
|
|
||||||
|
mockFindOne
|
||||||
|
.mockResolvedValueOnce(null) // guard by companyId + userId
|
||||||
|
.mockResolvedValueOnce(existingClient); // lookup by companyId + email
|
||||||
|
|
||||||
|
const result = await ClientList.createClientByUser(request);
|
||||||
|
|
||||||
|
expect(mockCreate).not.toHaveBeenCalled();
|
||||||
|
expect(existingClient.userId).toBe("user-001");
|
||||||
|
expect(existingClient.save).toHaveBeenCalled();
|
||||||
|
// Admin-entered data must not be overwritten by the adopt path
|
||||||
|
expect(existingClient.firstName).toBe("Admin Entered Name");
|
||||||
|
expect(existingClient.lastName).toBe("Admin Entered Lastname");
|
||||||
|
// The client was already counted, so metrics are untouched
|
||||||
|
expect(MetricsList.canAddClient).not.toHaveBeenCalled();
|
||||||
|
expect(MetricsList.addClient).not.toHaveBeenCalled();
|
||||||
|
expect(NotificationsManager.sendSystemNotification).toHaveBeenCalledWith({
|
||||||
|
userId: "user-001",
|
||||||
|
subject: "Ha sido dado de alta como cliente",
|
||||||
|
message: "Su usuario ha sido dado de alta como cliente en la organización Acme",
|
||||||
|
type: NotificationType.BILLING,
|
||||||
|
});
|
||||||
|
expect(result).toBe(existingClient);
|
||||||
|
});
|
||||||
|
|
||||||
|
// --- Create path: no client by userId nor email ---
|
||||||
|
|
||||||
|
it("creates a new client when no client exists by userId nor email", async () => {
|
||||||
|
const newClient = buildClientDoc({ userId: "user-001" });
|
||||||
|
|
||||||
|
mockFindOne
|
||||||
|
.mockResolvedValueOnce(null) // guard by companyId + userId
|
||||||
|
.mockResolvedValueOnce(null); // lookup by companyId + email
|
||||||
|
|
||||||
|
(MetricsList.canAddClient as jest.Mock).mockResolvedValue(true);
|
||||||
|
mockCreate.mockResolvedValue(newClient);
|
||||||
|
|
||||||
|
const result = await ClientList.createClientByUser(request);
|
||||||
|
|
||||||
|
expect(mockCreate).toHaveBeenCalledWith({
|
||||||
|
companyId: "company-001",
|
||||||
|
userId: "user-001",
|
||||||
|
sessionUser: "session-user",
|
||||||
|
email: user.email,
|
||||||
|
firstName: user.firstName,
|
||||||
|
lastName: user.lastName,
|
||||||
|
phoneAreaCode: user.phoneAreaCode,
|
||||||
|
phoneCountryCode: user.phoneCountryCode,
|
||||||
|
phoneNumber: user.phoneNumber,
|
||||||
|
});
|
||||||
|
expect(MetricsList.addClient).toHaveBeenCalledWith({
|
||||||
|
userId: "owner-user",
|
||||||
|
quantity: 1,
|
||||||
|
});
|
||||||
|
expect(NotificationsManager.sendSystemNotification).toHaveBeenCalledWith({
|
||||||
|
userId: "user-001",
|
||||||
|
subject: "Ha sido dado de alta como cliente",
|
||||||
|
message: "Su usuario ha sido dado de alta como cliente en la organización Acme",
|
||||||
|
type: NotificationType.BILLING,
|
||||||
|
});
|
||||||
|
expect(result).toBe(newClient);
|
||||||
|
});
|
||||||
|
|
||||||
|
// --- Existing guard preserved ---
|
||||||
|
|
||||||
|
it("throws when the client already belongs to the company", async () => {
|
||||||
|
mockFindOne.mockResolvedValueOnce(buildClientDoc()); // guard by companyId + userId
|
||||||
|
|
||||||
|
await expect(ClientList.createClientByUser(request)).rejects.toThrow(
|
||||||
|
/ya pertenece a la compan/
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(mockCreate).not.toHaveBeenCalled();
|
||||||
|
expect(NotificationsManager.sendSystemNotification).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
// --- E11000 concurrent-create race ---
|
||||||
|
|
||||||
|
it("returns the winning client when create hits the unique index (E11000)", async () => {
|
||||||
|
const racedClient = buildClientDoc({ userId: "user-001" });
|
||||||
|
|
||||||
|
mockFindOne
|
||||||
|
.mockResolvedValueOnce(null) // guard by companyId + userId
|
||||||
|
.mockResolvedValueOnce(null) // lookup by companyId + email
|
||||||
|
.mockResolvedValueOnce(racedClient); // re-fetch after E11000
|
||||||
|
|
||||||
|
(MetricsList.canAddClient as jest.Mock).mockResolvedValue(true);
|
||||||
|
mockCreate.mockRejectedValue({ code: 11000 });
|
||||||
|
|
||||||
|
const result = await ClientList.createClientByUser(request);
|
||||||
|
|
||||||
|
expect(result).toBe(racedClient);
|
||||||
|
// The winning request already counted and notified
|
||||||
|
expect(MetricsList.addClient).not.toHaveBeenCalled();
|
||||||
|
expect(NotificationsManager.sendSystemNotification).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rethrows non-duplicate-key errors from create", async () => {
|
||||||
|
mockFindOne
|
||||||
|
.mockResolvedValueOnce(null) // guard by companyId + userId
|
||||||
|
.mockResolvedValueOnce(null); // lookup by companyId + email
|
||||||
|
|
||||||
|
(MetricsList.canAddClient as jest.Mock).mockResolvedValue(true);
|
||||||
|
mockCreate.mockRejectedValue(new Error("database down"));
|
||||||
|
|
||||||
|
await expect(ClientList.createClientByUser(request)).rejects.toThrow("database down");
|
||||||
|
});
|
||||||
|
|
||||||
|
// --- Schema indexes ---
|
||||||
|
|
||||||
|
it("declares unique indexes for companyId+email and companyId+userId", () => {
|
||||||
|
const indexes = realAdapter.schema.indexes();
|
||||||
|
|
||||||
|
const emailIndex = indexes.find(
|
||||||
|
([fields]) => fields.companyId === 1 && fields.email === 1
|
||||||
|
);
|
||||||
|
expect(emailIndex).toBeDefined();
|
||||||
|
expect(emailIndex?.[1]).toMatchObject({ unique: true });
|
||||||
|
|
||||||
|
const userIdIndex = indexes.find(
|
||||||
|
([fields]) => fields.companyId === 1 && fields.userId === 1
|
||||||
|
);
|
||||||
|
expect(userIdIndex).toBeDefined();
|
||||||
|
expect(userIdIndex?.[1]).toMatchObject({
|
||||||
|
unique: true,
|
||||||
|
partialFilterExpression: { userId: { $type: "oid" } },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user