Skip to content
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

RFC: pubOp for creating and updating pubs more easily #965

Merged
merged 27 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
934f7cc
feat: new fluent pub mutation api
tefkah Feb 5, 2025
1969dfa
feat: add ability to replace pub relations
tefkah Feb 6, 2025
7b961d0
feat: beautiful better implementation
tefkah Feb 6, 2025
0feeca1
fix: cleanup a bit
tefkah Feb 6, 2025
18b38c6
feat: allow to properly purge orphans
tefkah Feb 10, 2025
db62dda
fix: get rid of the concept of a pubId map and just pregenerate the ids
tefkah Feb 10, 2025
f9567d0
chore: add some comments
tefkah Feb 10, 2025
7deacf6
feat: add ability to move stages
tefkah Feb 10, 2025
7ead642
fix: update test
tefkah Feb 10, 2025
eab252d
feat: allow inline specification of pubops
tefkah Feb 11, 2025
631bc83
refactor: rename override option to 'replaceExisting'
tefkah Feb 11, 2025
39dd06d
test: add test for multiple relate
tefkah Feb 11, 2025
d6d638e
fix: fix type issues
tefkah Feb 11, 2025
0c90e5d
refactor: add slightly better error handling
tefkah Feb 11, 2025
2258024
fix: add back deletePubValuesByValueId
tefkah Feb 11, 2025
1390713
fix: add back pubvalue upsert functions
tefkah Feb 11, 2025
e7aa8ac
fix: sort pubvalues deeply for tests
tefkah Feb 11, 2025
6c42929
fix: fix tests and imports
tefkah Feb 11, 2025
8cdb530
feat: make upsert override by default
tefkah Feb 11, 2025
da0b8eb
docs: add better documentation to orphan
tefkah Feb 11, 2025
b52d745
refactor: simplify createPub logic a bit
tefkah Feb 11, 2025
27dee15
Merge branch 'main' into tfk/pub-op
tefkah Feb 11, 2025
64ebece
chore: clean up
tefkah Feb 11, 2025
6af01c3
Merge branch 'tfk/pub-op' of https://github.com/pubpub/platform into …
tefkah Feb 11, 2025
db1eef2
Merge branch 'main' into tfk/pub-op
tefkah Feb 12, 2025
72fc7d2
fix: update matcher types
tefkah Feb 12, 2025
19842b5
Merge branch 'main' into tfk/pub-op
3mcd Feb 12, 2025
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
2 changes: 1 addition & 1 deletion core/actions/_lib/runActionInstance.db.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const { getTrx, rollback, commit } = createForEachMockedTransaction();

const pubTriggerTestSeed = async () => {
const slugName = `test-server-pub-${new Date().toISOString()}`;
const { createSeed } = await import("~/prisma/seed/seedCommunity");
const { createSeed } = await import("~/prisma/seed/createSeed");

return createSeed({
community: {
Expand Down
3 changes: 2 additions & 1 deletion core/app/api/v0/c/[communitySlug]/site/[...ts-rest]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ const handler = createNextHandler(
};
},
archive: async ({ params }) => {
const { lastModifiedBy } = await checkAuthorization({
const { lastModifiedBy, community } = await checkAuthorization({
token: { scope: ApiAccessScope.pub, type: ApiAccessType.write },
cookies: {
capability: Capabilities.deletePub,
Expand All @@ -383,6 +383,7 @@ const handler = createNextHandler(

const result = await deletePub({
pubId: params.pubId as PubsId,
communityId: community.id,
lastModifiedBy,
});

Expand Down
2 changes: 1 addition & 1 deletion core/app/components/pubs/PubEditor/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export const removePub = defineServerAction(async function removePub({ pubId }:
}

try {
await deletePub({ pubId, lastModifiedBy });
await deletePub({ pubId, lastModifiedBy, communityId: community.id });

return {
success: true,
Expand Down
5 changes: 4 additions & 1 deletion core/globalSetup.ts → core/lib/__tests__/globalSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import { logger } from "logger";

export const setup = async () => {
config({
path: ["./.env.test", "./.env.test.local"],
path: [
new URL("../../.env.test", import.meta.url).pathname,
new URL("../../.env.test.local", import.meta.url).pathname,
],
});

if (process.env.SKIP_RESET) {
Expand Down
54 changes: 43 additions & 11 deletions core/lib/__tests__/live.db.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,49 @@
import { describe, expect, test } from "vitest";

import { CoreSchemaType, MemberRole } from "db/public";

import type { ClientException } from "../serverActions";
import { createSeed } from "~/prisma/seed/createSeed";
import { isClientException } from "../serverActions";
import { mockServerCode } from "./utils";

const { testDb, getLoginData, createForEachMockedTransaction } = await mockServerCode();

const { getTrx, rollback, commit } = createForEachMockedTransaction();

const communitySeed = createSeed({
community: {
name: "test",
slug: "test",
},
pubFields: {
Title: { schemaName: CoreSchemaType.String },
},
pubTypes: {
"Basic Pub": {
Title: { isTitle: true },
},
},
users: {
admin: {
role: MemberRole.admin,
},
},
pubs: [
{
pubType: "Basic Pub",
values: {
Title: "test",
},
},
],
});

const seed = async (trx = testDb) => {
const { seedCommunity } = await import("~/prisma/seed/seedCommunity");
return seedCommunity(communitySeed, undefined, trx);
};

describe("live", () => {
test("should be able to connect to db", async () => {
const result = await testDb.selectFrom("users").selectAll().execute();
Expand All @@ -17,6 +53,8 @@ describe("live", () => {
test("can rollback transactions", async () => {
const trx = getTrx();

const { community, users, pubs } = await seed(trx);

// Insert a user
const user = await trx
.insertInto("users")
Expand Down Expand Up @@ -58,6 +96,7 @@ describe("live", () => {
describe("transaction block example", () => {
test("can add a user that will not persist", async () => {
const trx = getTrx();

await trx
.insertInto("users")
.values({
Expand All @@ -79,6 +118,9 @@ describe("live", () => {

test("createForm needs a logged in user", async () => {
const trx = getTrx();

const { community, users, pubs } = await seed(trx);

getLoginData.mockImplementation(() => {
return undefined;
});
Expand All @@ -87,12 +129,6 @@ describe("live", () => {
(m) => m.createForm
);

const community = await trx
.selectFrom("communities")
.selectAll()
.where("slug", "=", "croccroc")
.executeTakeFirstOrThrow();

const pubType = await trx
.selectFrom("pub_types")
.select(["id"])
Expand All @@ -110,15 +146,11 @@ describe("live", () => {
return { id: "123", isSuperAdmin: true };
});

const { community, users, pubs } = await seed(trx);
const getForm = await import("../server/form").then((m) => m.getForm);
const createForm = await import("~/app/c/[communitySlug]/forms/actions").then(
(m) => m.createForm
);
const community = await trx
.selectFrom("communities")
.selectAll()
.where("slug", "=", "croccroc")
.executeTakeFirstOrThrow();

const forms = await getForm({ slug: "my-form-2", communityId: community.id }).execute();
expect(forms.length).toEqual(0);
Expand Down
76 changes: 76 additions & 0 deletions core/lib/__tests__/matchers.ts
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are helper methods to make matching pubvalues easier.

eg instead of doing

expect(pub.values).toHaveLength(3);
pub.values.sort(/* sorting bc you want the order of the values to be stable over different tests */)
expect(pub.values).toMatchObject([ 
	...
]);

you just do

expect(pub).toHaveValues([...])

Similarly for

expect(pubId).toExist()

just easier than manually looking up the pub yourself

Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { expect } from "vitest";

import type { ProcessedPub } from "contracts";
import type { PubsId } from "db/public";

import type { db } from "~/kysely/database";

const deepSortValues = (pub: ProcessedPub): ProcessedPub => {
pub.values
.sort((a, b) => (a.value as string).localeCompare(b.value as string))
.map((item) => ({
...item,
relatedPub: item.relatedPub?.values ? deepSortValues(item.relatedPub) : item.relatedPub,
}));

return pub;
};

expect.extend({
async toExist(received: PubsId | ProcessedPub, expected?: typeof db) {
if (typeof received !== "string") {
throw new Error("toExist() can only be called with a PubsId");
}
const { getPlainPub } = await import("../server/pub");

const pub = await getPlainPub(received, expected).executeTakeFirst();
const pass = Boolean(pub && pub.id === received);
const { isNot } = this;

return {
pass,
message: () =>
isNot
? `Expected pub with ID ${received} not to exist, but it ${pass ? "does" : "does not"}`
: `Expected pub with ID ${received} to exist, but it ${pass ? "does" : "does not"}`,
};
},

toHaveValues(
received: PubsId | ProcessedPub,
expected: Partial<ProcessedPub["values"][number]>[]
) {
if (typeof received === "string") {
throw new Error("toHaveValues() can only be called with a ProcessedPub");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how come received: PubsId | ProcessedPub up above instead of received: ProcessedPub then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! it was left over bc i think i wanted to be able to pass in both in either case, and was having some trouble with the types. i later decided to just use PubsId for toExist and ProcessedPub for toHaveValues, but never updated it! I'll fix it

}

const pub = received;
const sortedPubValues = deepSortValues(pub);

const expectedLength = expected.length;
const receivedLength = sortedPubValues.values.length;

const isNot = this.isNot;
if (!isNot && !this.equals(expectedLength, receivedLength)) {
return {
pass: false,
message: () =>
`Expected pub to have ${expectedLength} values, but it has ${receivedLength}`,
};
}

// equiv. to .toMatchObject
const pass = this.equals(sortedPubValues.values, expected, [
this.utils.iterableEquality,
this.utils.subsetEquality,
]);

return {
pass,
message: () =>
pass
? `Expected pub ${isNot ? "not" : ""} to have values ${JSON.stringify(expected)}, and it does ${isNot ? "not" : ""}`
: `Expected pub ${isNot ? "not to" : "to"} match values ${this.utils.diff(sortedPubValues.values, expected)}`,
};
},
});
6 changes: 3 additions & 3 deletions core/lib/server/pub-capabilities.db.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { describe, expect, it } from "vitest";

import { CoreSchemaType, MemberRole } from "db/public";

import type { Seed } from "~/prisma/seed/seedCommunity";
import { createSeed } from "~/prisma/seed/createSeed";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc importing createSeed up here did make data persist whereas importing just the type { Seed } didn't?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but i think that was bc they were both imported from /seedCommunity. I separated out the createSeed to a different file, which (I did not explicitly check) i think should fix that issue.
i do think using createSeed is slightly better as that preserves the type info once you pass it to seedCommunity, eg if you have pubTypes: { 'SomepubType': ... } in your seed, the output of seedcommunity will have pubTypes.SomePubType as well

import { mockServerCode } from "../__tests__/utils";

await mockServerCode();

const seed = {
const seed = createSeed({
community: {
name: "test-pub-capabilities",
slug: "test-pub-capabilities",
Expand Down Expand Up @@ -97,7 +97,7 @@ const seed = {
},
},
],
} as Seed;
});

describe("getPubsWithRelatedValuesAndChildren capabilities", () => {
it("should restrict pubs by visibility", async () => {
Expand Down
Loading
Loading