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

fix(Strapi staging performance) #1581

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
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
30 changes: 14 additions & 16 deletions app/root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
fetchMeta,
fetchSingleEntry,
fetchErrors,
fetchTranslations,
fetchMultipleTranslations,
} from "~/services/cms/index.server";
import { defaultLocale } from "~/services/cms/models/StrapiLocale";
import { config as configWeb } from "~/services/env/web";
Expand Down Expand Up @@ -94,12 +94,8 @@ export const loader = async ({ request, context }: LoaderFunctionArgs) => {
trackingConsent,
errorPages,
meta,
deleteDataStrings,
translations,
hasAnyUserData,
feedbackTranslations,
pageHeaderTranslations,
videoTranslations,
accessibilityTranslations,
mainSession,
showKopfzeile,
] = await Promise.all([
Expand All @@ -109,12 +105,14 @@ export const loader = async ({ request, context }: LoaderFunctionArgs) => {
trackingCookieValue({ request }),
fetchErrors(),
fetchMeta({ filterValue: "/" }),
fetchTranslations("delete-data"),
fetchMultipleTranslations([
"delete-data",
"feedback",
"pageHeader",
"video",
"accessibility",
]),
anyUserData(request),
fetchTranslations("feedback"),
fetchTranslations("pageHeader"),
fetchTranslations("video"),
fetchTranslations("accessibility"),
mainSessionFromCookieHeader(cookieHeader),
isFeatureFlagEnabled("showKopfzeile"),
]);
Expand All @@ -139,15 +137,15 @@ export const loader = async ({ request, context }: LoaderFunctionArgs) => {
errorPages,
meta,
context,
deletionLabel: deleteDataStrings.footerLinkLabel,
deletionLabel: translations["delete-data"].footerLinkLabel,
hasAnyUserData,
feedbackTranslations,
feedbackTranslations: translations.feedback,
pageHeaderTranslations: extractTranslations(
["leichtesprache", "gebaerdensprache", "mainNavigationAriaLabel"],
pageHeaderTranslations,
translations.pageHeader,
),
videoTranslations,
accessibilityTranslations,
videoTranslations: translations.video,
accessibilityTranslations: translations.accessibility,
bannerState:
getFeedbackBannerState(mainSession, pathname) ?? BannerState.ShowRating,
},
Expand Down
29 changes: 13 additions & 16 deletions app/routes/shared/formular.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import {
fetchFlowPage,
fetchMeta,
fetchTranslations,
fetchMultipleTranslations,
} from "~/services/cms/index.server";
import { isStrapiArraySummary } from "~/services/cms/models/StrapiArraySummary";
import { buildFormularServerTranslations } from "~/services/flow/formular/buildFormularServerTranslations";
Expand Down Expand Up @@ -82,20 +82,15 @@
}
}

const [
formPageContent,
parentMeta,
navigationStrings,
defaultStrings,
flowTranslations,
overviewTranslations,
] = await Promise.all([
const [formPageContent, parentMeta, translations] = await Promise.all([
fetchFlowPage("form-flow-pages", flowId, stepId),
fetchMeta({ filterValue: parentFromParams(pathname, params) }),
fetchTranslations(`${flowId}/menu`),
fetchTranslations("defaultTranslations"),
fetchTranslations(flowId),
fetchTranslations(`${flowId}/summaryPage`),
fetchMultipleTranslations([
`${flowId}/menu`,
"defaultTranslations",
flowId,
`${flowId}/summaryPage`,
]),
]);

const arrayCategories = formPageContent.pre_form
Expand All @@ -118,16 +113,16 @@
const { stringTranslations, cmsContent } =
await buildFormularServerTranslations({
currentFlow,
flowTranslations,
flowTranslations: translations[flowId],
migrationData,
arrayCategories,
overviewTranslations,
overviewTranslations: translations[`${flowId}/summaryPage`],
formPageContent,
userDataWithPageData,
});

// Inject heading into <legend> inside radio groups
// TODO: only do for pages with *one* select?

Check warning on line 125 in app/routes/shared/formular.server.ts

View workflow job for this annotation

GitHub Actions / code-quality / npm run lint

Complete the task associated to this "TODO" comment
const formElements = cmsContent.formContent.map((strapiFormElement) => {
if (
isStrapiSelectComponent(strapiFormElement) &&
Expand Down Expand Up @@ -157,6 +152,8 @@
? insertIndexesIntoPath(pathname, backDestination, arrayIndexes)
: backDestination;

const defaultStrings = translations.defaultTranslations;

const buttonNavigationProps = getButtonNavigationProps({
backButtonLabel:
cmsContent.backButtonLabel ?? defaultStrings.backButtonDefaultLabel,
Expand All @@ -170,7 +167,7 @@
navItemsFromStepStates(
stepId,
flowController.stepStates(),
navigationStrings,
translations[`${flowId}/menu`],
) ?? [];

const navigationA11yLabels = {
Expand Down
20 changes: 19 additions & 1 deletion app/services/cms/__test__/getStrapiEntryFromApi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe("services/cms", () => {

describe("getStrapiEntryFromApi", () => {
const dataResponse = [{ testField: "testData" }];
const defaultOptions: GetStrapiEntryOpts = {
const defaultOptions: GetStrapiEntryOpts<"pages"> = {
apiId: "pages",
locale: stagingLocale,
};
Expand Down Expand Up @@ -73,6 +73,24 @@ describe("services/cms", () => {
);
});

test("request url with $in filter", async () => {
await getStrapiEntryFromApi({
...defaultOptions,
filters: [
{
field: "flow_ids",
operation: "$in",
value: ["foobar", "foobar2", "foobar3"],
},
],
});
expect(axiosGetSpy).toHaveBeenNthCalledWith(
1,
`${expectedStagingRequestUrl}&filters[flow_ids][$in][0]=foobar&filters[flow_ids][$in][1]=foobar2&filters[flow_ids][$in][2]=foobar3`,
expect.anything(),
);
});

test("response handling with api returning array", async () => {
axiosGetSpy.mockResolvedValue({ data: { data: dataResponse } });
expect(await getStrapiEntryFromApi(defaultOptions)).toEqual(dataResponse);
Expand Down
29 changes: 28 additions & 1 deletion app/services/cms/__test__/getStrapiEntryFromFile.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import fs from "node:fs";
import { getStrapiFlowPage } from "tests/factories/cmsModels/strapiFlowPage";
import { getStrapiFooter } from "tests/factories/cmsModels/strapiFooter";
import { getStrapiEntryFromFile } from "~/services/cms/getStrapiEntryFromFile";
import { StrapiLocaleSchema } from "~/services/cms/models/StrapiLocale";
import {
defaultLocale,
StrapiLocaleSchema,
} from "~/services/cms/models/StrapiLocale";
import { type StrapiPage } from "~/services/cms/models/StrapiPage";
import type { StrapiSchemas } from "../schemas";

Expand Down Expand Up @@ -52,6 +56,14 @@ describe("services/cms", () => {
{ flowId: "/fluggastrechte/formular" },
],
},
getStrapiFlowPage({
stepId: "/stepId2",
form: [],
}),
getStrapiFlowPage({
stepId: "/stepId3",
form: [],
}),
],
translations: [],
} satisfies StrapiSchemas;
Expand Down Expand Up @@ -107,6 +119,21 @@ describe("services/cms", () => {
).not.toBeUndefined();
});

it("can filter on an array of values ($in operator)", async () => {
const results = await getStrapiEntryFromFile({
apiId: "form-flow-pages",
filters: [
{
field: "stepId",
operation: "$in",
value: ["/stepId", "/stepId2", "/stepId3"],
},
],
locale: defaultLocale,
});
expect(results).toEqual(fileContent["form-flow-pages"]);
});

it("returns empty array without matches", async () => {
expect(
await getStrapiEntryFromFile({
Expand Down
66 changes: 65 additions & 1 deletion app/services/cms/__test__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import {
getStrapiFormComponent,
} from "tests/factories/cmsModels/strapiFlowPage";
import { getStrapiFooter } from "~/../tests/factories/cmsModels/strapiFooter";
import { fetchSingleEntry } from "~/services/cms/index.server";
import {
fetchSingleEntry,
fetchEntries,
fetchMultipleTranslations,
} from "~/services/cms/index.server";
import { StrapiSchemas } from "~/services/cms/schemas";
import { fetchAllFormFields } from "../fetchAllFormFields";
import { getStrapiEntry } from "../getStrapiEntry";

Expand All @@ -21,6 +26,27 @@ describe("services/cms", () => {
});
});

describe("fetchEntries", () => {
test("returns a list of entries", async () => {
const strapiPages = [
getStrapiFlowPage({
stepId: "step1",
form: [getStrapiFormComponent({ name: "formFieldForStep1" })],
}),

getStrapiFlowPage({
stepId: "step2",
form: [getStrapiFormComponent({ name: "formFieldForStep2" })],
}),
];
vi.mocked(getStrapiEntry).mockResolvedValue(strapiPages);

expect(await fetchEntries({ apiId: "form-flow-pages" })).toEqual(
strapiPages,
);
});
});

describe("fetchAllFormFields", () => {
test("returns steps with it's form field names", async () => {
vi.mocked(getStrapiEntry).mockResolvedValue([
Expand Down Expand Up @@ -157,4 +183,42 @@ describe("services/cms", () => {
);
});
});

describe("fetchMultipleTranslations", () => {
test("returns translations for multiple scopes", async () => {
const mockedTranslations: StrapiSchemas["translations"] = [
{
scope: "amtsgericht",
locale: "de",
field: [
{ name: "amtsgerichtKey", value: "amtsgerichtValue" },
{ name: "amtsgerichtKey2", value: "amtsgerichtValue2" },
],
},
{
scope: "ausgaben",
locale: "de",
field: [
{ name: "ausgabenKey", value: "ausgabenValue" },
{ name: "ausgabenKey2", value: "ausgabenValue2" },
],
},
];

vi.mocked(getStrapiEntry).mockResolvedValue(mockedTranslations);

expect(
await fetchMultipleTranslations(["amtsgericht", "ausgaben"]),
).toEqual({
amtsgericht: {
amtsgerichtKey: "amtsgerichtValue",
amtsgerichtKey2: "amtsgerichtValue2",
},
ausgaben: {
ausgabenKey: "ausgabenValue",
ausgabenKey2: "ausgabenValue2",
},
});
});
});
});
13 changes: 10 additions & 3 deletions app/services/cms/filters.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
import type { StrapiLocale } from "./models/StrapiLocale";
import type { ApiId } from "./schemas";

/**
* See https://docs.strapi.io/dev-docs/api/rest/filters-locale-publication#filtering
* for all available filter operations and add them here as necessary
*/
export type FilterOperation = "$eq" | "$in";

export type Filter = {
field: string;
value: string;
value: string | string[];
nestedField?: string;
operation?: FilterOperation;
};

export type GetStrapiEntryOpts = {
apiId: ApiId;
export type GetStrapiEntryOpts<T extends ApiId> = {
apiId: T;
filters?: Filter[];
locale?: StrapiLocale;
populate?: string;
Expand Down
2 changes: 1 addition & 1 deletion app/services/cms/getStrapiEntry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { config } from "../env/env.server";
import { defaultLocale, stagingLocale } from "./models/StrapiLocale";

export type GetStrapiEntry = <T extends ApiId>(
opts: GetStrapiEntryOpts & { apiId: T },
opts: GetStrapiEntryOpts<T> & { apiId: T },
) => Promise<StrapiSchemas[T] | [null]>;

const getterFunction =
Expand Down
20 changes: 13 additions & 7 deletions app/services/cms/getStrapiEntryFromApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,30 @@ import { config } from "../env/env.server";
function buildFilters(filters?: Filter[]) {
if (!filters) return "";
return filters
.map(
({ field, nestedField, value }) =>
.map(({ field, nestedField, operation, value }) => {
if (operation === "$in" && Array.isArray(value)) {
return value
.map((v, idx) => `&filters[${field}][${operation}][${idx}]=${v}`)
.join("");
}
return (
`&filters[${field}]` +
(nestedField ? `[${nestedField}]` : ``) +
`[$eq]=${value}`,
)
`[${operation ?? "$eq"}]=${typeof value === "string" ? value : ""}`
);
})
.join("");
}

const buildUrl = ({
const buildUrl = <T extends ApiId>({
apiId,
pageSize,
filters,
locale,
fields,
populate = "*",
deep = true,
}: GetStrapiEntryOpts) =>
}: GetStrapiEntryOpts<T>) =>
[
config().STRAPI_API,
apiId,
Expand All @@ -45,7 +51,7 @@ const makeStrapiRequest = async <T extends ApiId>(url: string) =>
});

export const getStrapiEntryFromApi: GetStrapiEntry = async <T extends ApiId>(
opts: GetStrapiEntryOpts,
opts: GetStrapiEntryOpts<T>,
) => {
const returnData = (await makeStrapiRequest<T>(buildUrl(opts))).data.data;
return Array.isArray(returnData) ? returnData : [returnData];
Expand Down
Loading
Loading