Skip to content

Remove pubsinstages table #967

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

Draft
wants to merge 2 commits into
base: kalilsn/add-pub-stageid
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions core/actions/_lib/runActionInstance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ const _runActionInstance = async (
// often happens when an action is scheduled but a pub is moved before the action runs
jsonObjectFrom(
eb
.selectFrom("PubsInStages")
.select(["pubId", "stageId"])
.where("pubId", "=", args.pubId)
.selectFrom("pubs")
.select(["pubs.id as pubId", "stageId"])
.where("pubs.id", "=", args.pubId)
.whereRef("stageId", "=", "action_instances.stageId")
).as("pubInStage"),
])
Expand Down
9 changes: 4 additions & 5 deletions core/app/c/[communitySlug]/pubs/[pubId]/components/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export const getPubChildrenTable = (parentId: PubsId, selectedPubTypeId?: PubTyp
eb
.selectFrom("pubs")
.where("parentId", "=", parentId)
.select((eb) => ["id", "pubTypeId", "createdAt"])
.select(["id", "pubTypeId", "createdAt", "stageId"])
.orderBy("createdAt", "desc")
)
.with("children_with_specific_pubtype", (eb) =>
Expand All @@ -85,15 +85,14 @@ export const getPubChildrenTable = (parentId: PubsId, selectedPubTypeId?: PubTyp
.limit(1)
)
)
.leftJoin("PubsInStages", "PubsInStages.pubId", "all_children.id")
.selectAll(["all_children"])
.selectAll("all_children")
.select((eb) => [
pubValuesByRef(
"all_children.id" as "pubs.id"
) as unknown as AliasedRawBuilder<PubValues, "values">,
memberFields(eb.ref("all_children.id")).as("memberFields"),
actionInstances(eb.ref("PubsInStages.stageId")).as("actionInstances"),
stages(eb.ref("PubsInStages.stageId")).as("stages"),
actionInstances(eb.ref("all_children.stageId")).as("actionInstances"),
stages(eb.ref("all_children.stageId")).as("stages"),
])
)
.with("counts_of_other_pub_types", (eb) =>
Expand Down
4 changes: 2 additions & 2 deletions core/app/c/[communitySlug]/stages/manage/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export const deleteStage = defineServerAction(async function deleteStage(stageId
cause: error,
};
} finally {
await revalidateTagsForCommunity(["stages", "PubsInStages"]);
await revalidateTagsForCommunity(["stages", "pubs"]);
}
});

Expand Down Expand Up @@ -266,7 +266,7 @@ export const revalidateStages = defineServerAction(async function revalidateStag
return ApiError.NOT_LOGGED_IN;
}

await revalidateTagsForCommunity(["stages", "PubsInStages"]);
await revalidateTagsForCommunity(["stages", "pubs"]);
});

export const addAction = defineServerAction(async function addAction(
Expand Down
6 changes: 3 additions & 3 deletions core/app/components/pubs/PubEditor/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ export const availableStagesAndCurrentStage = ({
db
.with("currentStageId", (eb) =>
eb
.selectFrom("PubsInStages")
.selectFrom("pubs")
.select(["stageId as currentStageId"])
.where("PubsInStages.pubId", "=", pubId)
.where("pubs.id", "=", pubId)
)
.selectFrom("stages")
.select((eb) => [
Expand All @@ -119,7 +119,7 @@ export const availableStagesAndCurrentStage = ({
.selectFrom("stages")
.select(["id", "name", "order"])
.orderBy("order desc")
.where("stages.communityId", "=", communityId as CommunitiesId)
.where("stages.communityId", "=", communityId)
).as("availableStagesOfCurrentPub"),
])
);
15 changes: 6 additions & 9 deletions core/lib/authorization/capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,11 @@ export const userCan = async <T extends CapabilityTarget>(
) => {
if (target.type === MembershipType.pub) {
const capabilitiesQuery = db
.with("stage", (db) =>
.with("pub", (db) =>
db
.selectFrom("PubsInStages")
.where("PubsInStages.pubId", "=", target.pubId)
.select("PubsInStages.stageId")
)
.with("community", (db) =>
db.selectFrom("pubs").where("pubs.id", "=", target.pubId).select("pubs.communityId")
.selectFrom("pubs")
.where("pubs.id", "=", target.pubId)
.select(["pubs.communityId", "pubs.stageId"])
)
.with("stage_ms", (db) =>
db
Expand All @@ -104,7 +101,7 @@ export const userCan = async <T extends CapabilityTarget>(
// one stage. But we don't actually expect there to be multiple stageIds
// returned (for now)
"in",
eb.selectFrom("stage").select("stageId")
eb.selectFrom("pub").select("stageId")
)
)
.select("role")
Expand All @@ -120,7 +117,7 @@ export const userCan = async <T extends CapabilityTarget>(
db
.selectFrom("community_memberships")
.where("community_memberships.userId", "=", userId)
.whereRef("communityId", "=", db.selectFrom("community").select("communityId"))
.whereRef("communityId", "=", db.selectFrom("pub").select("communityId"))
.select("role")
)

Expand Down
3 changes: 1 addition & 2 deletions core/lib/db/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ export const getStagePubs = cache((stageId: StagesId) => {
.selectAll("pubs")
.select(pubValuesByRef("pubs.id"))
.select((eb) => pubType({ eb, pubTypeIdRef: "pubs.pubTypeId" }))
.innerJoin("PubsInStages", "PubsInStages.pubId", "pubs.id")
.where("PubsInStages.stageId", "=", stageId)
.where("pubs.stageId", "=", stageId)
);
});

Expand Down
7 changes: 3 additions & 4 deletions core/lib/server/cache/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,10 @@ Using this strategy, we can ensure that all data is fresh when it needs to be, b

#### Example

We fetch all data associated with a specific `pub` quite often. All the data of a `pub` is dependend on
We fetch all data associated with a specific `pub` quite often. All the data of a `pub` is dependent on

- the state of `pub_values` and `pub_fields` table for the values associated with a `pub`.
- the state of the `pubs` table for the data of the `pub` itself, as well as its children.
- possibly the `pubsinstages` table for the stage the `pub` is in.
- the state of `pub_values` and `pub_fields` table for the values associated with a `pub`.
- the state of the `pubs` table for the data of the `pub` itself, as well as its children.

When caching this data, we want to have the maximum balance between freshness and having a long cache time, while ideally not having to manually remember every single way we cache things. We can do this by invalidating the cache for a query for a specific `pub` when any of the tables are modified, as then we are assured that the data is fresh when needed.

Expand Down
83 changes: 14 additions & 69 deletions core/lib/server/pub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ type FlatPub = PubNoChildren & {
};

// pubValuesByRef adds a JSON object of pub_values keyed by their field name under the `fields` key to the output of a query
// pubIdRef should be a column name that refers to a pubId in the current query context, such as pubs.parentId or PubsInStages.pubId
// pubIdRef should be a column name that refers to a pubId in the current query context, such as pubs.parentId
// It doesn't seem to work if you've aliased the table or column (although you can probably work around that with a cast)
export const pubValuesByRef = (pubIdRef: StringReference<Database, keyof Database>) => {
return (eb: ExpressionBuilder<Database, keyof Database>) => pubValues(eb, { pubIdRef });
Expand Down Expand Up @@ -200,11 +200,7 @@ const withPubChildren = ({
.$if(!!communityId, (qb) =>
qb.where("pubs.communityId", "=", communityId!).where("pubs.parentId", "is", null)
)
.$if(!!stageId, (qb) =>
qb
.innerJoin("PubsInStages", "pubs.id", "PubsInStages.pubId")
.where("PubsInStages.stageId", "=", stageId!)
)
.$if(!!stageId, (qb) => qb.where("pubs.stageId", "=", stageId!))
.unionAll((eb) => {
return eb
.selectFrom("pubs")
Expand Down Expand Up @@ -248,6 +244,7 @@ const pubColumns = [
"assigneeId",
"parentId",
"title",
"stageId",
] as const satisfies SelectExpression<Database, "pubs">[];

export const getPubBase = (
Expand All @@ -266,26 +263,10 @@ export const getPubBase = (
...pubColumns,
pubType({ eb, pubTypeIdRef: "pubs.pubTypeId" }),
pubAssignee(eb),
jsonArrayFrom(
eb
.selectFrom("PubsInStages")
.select(["PubsInStages.stageId as id"])
.whereRef("PubsInStages.pubId", "=", "pubs.id")
).as("stages"),
jsonArrayFrom(
eb
.selectFrom("children")
.select((eb) => [
...pubColumns,
"children.values",
"children.pubType",
jsonArrayFrom(
eb
.selectFrom("PubsInStages")
.select(["PubsInStages.stageId as id"])
.whereRef("PubsInStages.pubId", "=", "children.id")
).as("stages"),
])
.select([...pubColumns, "children.values", "children.pubType"])
.$narrowType<{ values: PubValues }>()
).as("children"),
])
Expand Down Expand Up @@ -366,11 +347,7 @@ export const _deprecated_getPubs = async (
.$if(Boolean(props.communityId), (eb) =>
eb.where("pubs.communityId", "=", props.communityId!)
)
.$if(Boolean(props.stageId), (eb) =>
eb
.innerJoin("PubsInStages", "pubs.id", "PubsInStages.pubId")
.where("PubsInStages.stageId", "=", props.stageId!)
)
.$if(Boolean(props.stageId), (eb) => eb.where("pubs.stageId", "=", props.stageId!))
.$if(Boolean(params.onlyParents), (eb) => eb.where("pubs.parentId", "is", null))
.limit(limit)
.offset(offset)
Expand Down Expand Up @@ -519,25 +496,11 @@ export const createPubRecursiveNew = async <Body extends CreatePubRequestBodyWit
pubTypeId: body.pubTypeId as PubTypesId,
assigneeId: body.assigneeId as UsersId,
parentId: parentId as PubsId,
stageId,
})
.returningAll()
).executeTakeFirstOrThrow();

let createdStageId: StagesId | undefined;
if (stageId) {
const result = await autoRevalidate(
trx
.insertInto("PubsInStages")
.values((eb) => ({
pubId: newPub.id,
stageId: expect(stageId),
}))
.returningAll()
).executeTakeFirstOrThrow();

createdStageId = result.stageId;
}

if (body.members && Object.keys(body.members).length) {
await trx
.insertInto("pub_memberships")
Expand Down Expand Up @@ -585,7 +548,6 @@ export const createPubRecursiveNew = async <Body extends CreatePubRequestBodyWit
if (!body.children && !body.relatedPubs) {
return {
...pub,
stageId: createdStageId ?? null,
values: hydratedValues,
children: [],
depth,
Expand Down Expand Up @@ -614,7 +576,6 @@ export const createPubRecursiveNew = async <Body extends CreatePubRequestBodyWit
if (!body.relatedPubs) {
return {
...pub,
stageId: createdStageId ?? null,
values: hydratedValues,
children: children.length ? children : [],
depth,
Expand Down Expand Up @@ -642,7 +603,6 @@ export const createPubRecursiveNew = async <Body extends CreatePubRequestBodyWit

return {
...pub,
stageId: createdStageId,
values: [...pubValues, ...relatedPubs],
children,
depth,
Expand Down Expand Up @@ -685,7 +645,7 @@ export const deletePub = async ({
};

export const getPubStage = (pubId: PubsId, trx = db) =>
autoCache(trx.selectFrom("PubsInStages").select("stageId").where("pubId", "=", pubId));
autoCache(trx.selectFrom("pubs").select("stageId").where("id", "=", pubId));

export const getPlainPub = (pubId: PubsId, trx = db) =>
autoCache(trx.selectFrom("pubs").selectAll().where("id", "=", pubId));
Expand Down Expand Up @@ -1350,8 +1310,6 @@ export async function getPubsWithRelatedValuesAndChildren<
// we need to do this weird cast, because kysely does not support typing the selecting from a later CTE
// which is possible only in a with recursive query
.selectFrom("root_pubs_limited as p" as unknown as "pubs as p")
// TODO: can we avoid doing this join again since it's already in root pubs?
.leftJoin("PubsInStages", "p.id", "PubsInStages.pubId")
.$if(Boolean(withRelatedPubs), (qb) =>
qb
.leftJoin("pub_values as pv", (join) =>
Expand All @@ -1371,7 +1329,7 @@ export async function getPubsWithRelatedValuesAndChildren<
"p.createdAt",
"p.updatedAt",
"p.title",
"PubsInStages.stageId",
"p.stageId",
"p.parentId",
sql<number>`1`.as("depth"),
sql<boolean>`false`.as("isCycle"),
Expand Down Expand Up @@ -1408,7 +1366,6 @@ export async function getPubsWithRelatedValuesAndChildren<
])
)
)
.leftJoin("PubsInStages", "pubs.id", "PubsInStages.pubId")
.where((eb) =>
eb.exists(
eb.selectFrom("capabilities" as any).where((ebb) => {
Expand Down Expand Up @@ -1440,7 +1397,7 @@ export async function getPubsWithRelatedValuesAndChildren<
eb(
"capabilities.membId",
"=",
eb.ref("PubsInStages.stageId")
eb.ref("pubs.stageId")
),
]),
eb.and([
Expand Down Expand Up @@ -1490,7 +1447,7 @@ export async function getPubsWithRelatedValuesAndChildren<
"pubs.createdAt",
"pubs.updatedAt",
"pubs.title",
"PubsInStages.stageId",
"pubs.stageId",
"pubs.parentId",
// increment the depth
sql<number>`pub_tree.depth + 1`.as("depth"),
Expand Down Expand Up @@ -1631,8 +1588,6 @@ export async function getPubsWithRelatedValuesAndChildren<
.selectFrom("pubs")
.selectAll("pubs")
.where("pubs.communityId", "=", props.communityId)
.leftJoin("PubsInStages", "pubs.id", "PubsInStages.pubId")
.select("PubsInStages.stageId")
.where((eb) =>
eb.exists(
eb
Expand All @@ -1641,11 +1596,7 @@ export async function getPubsWithRelatedValuesAndChildren<
eb.or([
eb.and([
eb("capabilities.type", "=", MembershipType.stage),
eb(
"capabilities.membId",
"=",
eb.ref("PubsInStages.stageId")
),
eb("capabilities.membId", "=", eb.ref("pubs.stageId")),
]),
eb.and([
eb("capabilities.type", "=", MembershipType.pub),
Expand All @@ -1661,7 +1612,7 @@ export async function getPubsWithRelatedValuesAndChildren<
)
.$if(Boolean(props.pubId), (qb) => qb.where("pubs.id", "=", props.pubId!))
.$if(Boolean(props.stageId), (qb) =>
qb.where("PubsInStages.stageId", "=", props.stageId!)
qb.where("pubs.stageId", "=", props.stageId!)
)
.$if(Boolean(props.pubTypeId), (qb) =>
qb.where("pubs.pubTypeId", "=", props.pubTypeId!)
Expand Down Expand Up @@ -1894,11 +1845,7 @@ export const getPubsCount = async (props: {
const pubs = await db
.selectFrom("pubs")
.where("pubs.communityId", "=", props.communityId)
.$if(Boolean(props.stageId), (qb) =>
qb
.innerJoin("PubsInStages", "pubs.id", "PubsInStages.pubId")
.where("PubsInStages.stageId", "=", props.stageId!)
)
.$if(Boolean(props.stageId), (qb) => qb.where("pubs.stageId", "=", props.stageId!))
.$if(Boolean(props.pubTypeId), (qb) => qb.where("pubs.pubTypeId", "=", props.pubTypeId!))
.select((eb) => eb.fn.countAll<number>().as("count"))
.executeTakeFirstOrThrow();
Expand Down Expand Up @@ -2066,10 +2013,8 @@ export const fullTextSearch = async (
jsonObjectFrom(
eb
.selectFrom("stages")
.leftJoin("PubsInStages", "stages.id", "PubsInStages.stageId")
.select(["stages.id", "stages.name"])
.whereRef("PubsInStages.pubId", "=", "pubs.id")
.limit(1)
.whereRef("stages.id", "=", "pubs.stageId")
).as("stage"),
])
.where("pubs.communityId", "=", communityId)
Expand Down
Loading
Loading