Skip to content

Commit

Permalink
Fix project controller. (#2609)
Browse files Browse the repository at this point in the history
  • Loading branch information
romain-growthbook committed Jun 7, 2024
1 parent 5821934 commit 291b6c3
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 37 deletions.
3 changes: 2 additions & 1 deletion packages/back-end/src/api/projects/postProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import { auditDetailsCreate } from "../../services/audit";

export const postProject = createApiRequestHandler(postProjectValidator)(
async (req): Promise<PostProjectResponse> => {
const project = await req.context.models.projects.create(req.body);
const payload = req.context.models.projects.createValidator.parse(req.body);
const project = await req.context.models.projects.create(payload);

await req.audit({
event: "project.create",
Expand Down
2 changes: 1 addition & 1 deletion packages/back-end/src/api/projects/putProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const putProject = createApiRequestHandler(putProjectValidator)(

const newProject = await req.context.models.projects.update(
project,
req.body
req.context.models.projects.updateValidator.parse(req.body)
);

await req.audit({
Expand Down
95 changes: 76 additions & 19 deletions packages/back-end/src/models/BaseModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { z } from "zod";
import { isEqual, pick } from "lodash";
import { ApiReqContext } from "../../types/api";
import { ReqContext } from "../../types/organization";
import { CreateProps, UpdateProps } from "../../types/models";
import { logger } from "../util/logger";
import { EntityType, EventTypes, EventType } from "../types/Audit";
import { AuditInterfaceTemplate } from "../../types/audit";
Expand Down Expand Up @@ -36,6 +35,67 @@ export const baseSchema = z

export type BaseSchema = typeof baseSchema;

export type CreateProps<T extends object> = Omit<
T,
"id" | "organization" | "dateCreated" | "dateUpdated"
> & { id?: string };

export type CreateRawShape<T extends z.ZodRawShape> = {
[k in keyof Omit<
T,
"id" | "organization" | "dateCreated" | "dateUpdated"
>]: T[k];
} & {
id: z.ZodOptional<z.ZodString>;
};

export type CreateZodObject<T> = T extends z.ZodObject<
infer RawShape,
infer UnknownKeysParam,
infer ZodTypeAny
>
? z.ZodObject<CreateRawShape<RawShape>, UnknownKeysParam, ZodTypeAny>
: never;

export const createSchema = <T extends BaseSchema>(schema: T) =>
(schema
.omit({
organization: true,
dateCreated: true,
dateUpdated: true,
})
.extend({ id: z.string().optional() })
.strict() as unknown) as CreateZodObject<T>;

export type UpdateProps<T extends object> = Partial<
Omit<T, "id" | "organization" | "dateCreated" | "dateUpdated">
>;

export type UpdateRawShape<T extends z.ZodRawShape> = {
[k in keyof Omit<
T,
"id" | "organization" | "dateCreated" | "dateUpdated"
>]: z.ZodOptional<T[k]>;
};

export type UpdateZodObject<T> = T extends z.ZodObject<
infer RawShape,
infer UnknownKeysParam,
infer ZodTypeAny
>
? z.ZodObject<UpdateRawShape<RawShape>, UnknownKeysParam, ZodTypeAny>
: never;

const updateSchema = <T extends BaseSchema>(schema: T) =>
(schema
.omit({
organization: true,
dateCreated: true,
dateUpdated: true,
})
.partial()
.strict() as unknown) as UpdateZodObject<T>;

type AuditLogConfig<Entity extends EntityType> = {
entity: Entity;
createEvent: EventTypes<Entity>;
Expand Down Expand Up @@ -72,10 +132,18 @@ export abstract class BaseModel<
E extends EntityType,
WriteOptions = never
> {
public validator: T;
public createValidator: CreateZodObject<T>;
public updateValidator: UpdateZodObject<T>;

protected context: Context;

public constructor(context: Context) {
this.context = context;
this.config = this.getConfig();
this.validator = this.config.schema;
this.createValidator = createSchema(this.config.schema);
this.updateValidator = updateSchema(this.config.schema);
this.addIndexes();
}

Expand Down Expand Up @@ -205,21 +273,21 @@ export abstract class BaseModel<
return this._find();
}
public create(
props: unknown | CreateProps<z.infer<T>>,
props: CreateProps<z.infer<T>>,
writeOptions?: WriteOptions
): Promise<z.infer<T>> {
return this._createOne(props, writeOptions);
}
public update(
existing: z.infer<T>,
updates: unknown | UpdateProps<z.infer<T>>,
updates: UpdateProps<z.infer<T>>,
writeOptions?: WriteOptions
): Promise<z.infer<T>> {
return this._updateOne(existing, updates, { writeOptions });
}
public async updateById(
id: string,
updates: unknown | UpdateProps<z.infer<T>>,
updates: UpdateProps<z.infer<T>>,
writeOptions?: WriteOptions
): Promise<z.infer<T>> {
const existing = await this.getById(id);
Expand Down Expand Up @@ -320,13 +388,10 @@ export abstract class BaseModel<
}

protected async _createOne(
rawData: unknown | CreateProps<z.infer<T>>,
rawData: CreateProps<z.infer<T>>,
writeOptions?: WriteOptions
) {
const props = this.config.schema
.omit({ organization: true, dateCreated: true, dateUpdated: true })
.partial({ id: true })
.parse(rawData) as CreateProps<z.infer<T>>;
const props = this.createValidator.parse(rawData);

if (this.config.globallyUniqueIds && "id" in props) {
throw new Error("Cannot set a custom id for this model");
Expand Down Expand Up @@ -397,21 +462,13 @@ export abstract class BaseModel<

protected async _updateOne(
doc: z.infer<T>,
rawUpdates: unknown | UpdateProps<z.infer<T>>,
updates: UpdateProps<z.infer<T>>,
options?: {
auditEvent?: EventType;
writeOptions?: WriteOptions;
}
) {
let updates = this.config.schema
.omit({
organization: true,
dateCreated: true,
dateUpdated: true,
id: true,
})
.partial()
.parse(rawUpdates) as UpdateProps<z.infer<T>>;
updates = this.updateValidator.parse(updates);

// Only consider updates that actually change the value
const updatedFields = Object.entries(updates)
Expand Down
4 changes: 2 additions & 2 deletions packages/back-end/src/models/ProjectModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ export const projectSettingsValidator = z.object({
export const projectValidator = baseSchema
.extend({
name: z.string(),
description: z.string(),
settings: projectSettingsValidator,
description: z.string().default("").optional(),
settings: projectSettingsValidator.default({}).optional(),
})
.strict();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,8 @@ export const postFactMetric = async (
req: AuthRequest<unknown>,
res: Response<{ status: 200; factMetric: FactMetricInterface }>
) => {
const data = req.body;
const context = getContextFromReq(req);
const data = context.models.factMetrics.createValidator.parse(req.body);

const factMetric = await context.models.factMetrics.create(data);

Expand All @@ -351,8 +351,8 @@ export const putFactMetric = async (
req: AuthRequest<unknown, { id: string }>,
res: Response<{ status: 200 }>
) => {
const data = req.body;
const context = getContextFromReq(req);
const data = context.models.factMetrics.updateValidator.parse(req.body);

await context.models.factMetrics.updateById(req.params.id, data);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ export const putProject = async (
await context.models.projects.updateById(id, {
name,
description,
dateUpdated: new Date(),
});

res.status(200).json({
Expand Down
12 changes: 12 additions & 0 deletions packages/back-end/test/api/projects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,15 @@ describe("environements API", () => {
it("can update projects", async () => {
const getByIdMock = jest.fn();
const updateMock = jest.fn();
const updateValidatorMock = jest.fn();
const toApiInterfaceMock = jest.fn();

setReqContext({
models: {
projects: {
getById: getByIdMock,
update: updateMock,
updateValidator: { parse: updateValidatorMock },
toApiInterface: toApiInterfaceMock,
},
},
Expand All @@ -148,6 +150,7 @@ describe("environements API", () => {
...existing,
...updated,
}));
updateValidatorMock.mockImplementation((v) => v);
toApiInterfaceMock.mockImplementation((v) => ({
...v,
id: `${v.id}__interface`,
Expand All @@ -172,6 +175,9 @@ describe("environements API", () => {
{ id: "prj__3", description: "le proj 3" },
{ description: "new description" }
);
expect(updateValidatorMock).toHaveBeenCalledWith({
description: "new description",
});
expect(toApiInterfaceMock).toHaveBeenCalledWith({
id: "prj__3",
description: "new description",
Expand Down Expand Up @@ -222,18 +228,21 @@ describe("environements API", () => {

it("can create projects", async () => {
const createMock = jest.fn();
const createValidatorMock = jest.fn();
const toApiInterfaceMock = jest.fn();

setReqContext({
models: {
projects: {
create: createMock,
createValidator: { parse: createValidatorMock },
toApiInterface: toApiInterfaceMock,
},
},
});

createMock.mockImplementation((v) => ({ ...v, id: "prj__3" }));
createValidatorMock.mockImplementation((v) => v);
toApiInterfaceMock.mockImplementation((v) => ({
...v,
id: `${v.id}__interface`,
Expand All @@ -256,6 +265,9 @@ describe("environements API", () => {
expect(createMock).toHaveBeenCalledWith({
name: "le proj trois",
});
expect(createValidatorMock).toHaveBeenCalledWith({
name: "le proj trois",
});
expect(toApiInterfaceMock).toHaveBeenCalledWith({
id: "prj__3",
name: "le proj trois",
Expand Down
12 changes: 1 addition & 11 deletions packages/back-end/types/models.d.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1 @@
import { z } from "zod";
import { BaseSchema } from "../src/models/BaseModel";

export type CreateProps<T extends z.infer<BaseSchema>> = Omit<
T,
"id" | "organization" | "dateCreated" | "dateUpdated"
> & { id?: string };

export type UpdateProps<T extends z.infer<BaseSchema>> = Partial<
Omit<T, "id" | "organization" | "dateCreated" | "dateUpdated">
>;
export { CreateProps, UpdateProps } from "../src/models/BaseModel";

0 comments on commit 291b6c3

Please sign in to comment.