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

chore: Drop adminClient (1/2) #2356

Merged
merged 10 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from 8 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
4 changes: 2 additions & 2 deletions api.planx.uk/gis/digitalLand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
} from "@opensystemslab/planx-core/types";
import { gql } from "graphql-request";
import fetch from "isomorphic-fetch";
import { adminGraphQLClient as adminClient } from "../hasura";
import { addDesignatedVariable, omitGeometry } from "./helpers";
import { baseSchema } from "./local_authorities/metadata/base";
import { $api } from "../client";

export interface LocalAuthorityMetadata {
planningConstraints: {
Expand Down Expand Up @@ -68,12 +68,12 @@
options,
)}${datasets}`;
const res = await fetch(url)
.then((response: { json: () => any }) => response.json())

Check warning on line 71 in api.planx.uk/gis/digitalLand.ts

View workflow job for this annotation

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
.catch((error: any) => console.log(error));

Check warning on line 72 in api.planx.uk/gis/digitalLand.ts

View workflow job for this annotation

GitHub Actions / Run API Tests

Unexpected any. Specify a different type

// if analytics are "on", store an audit record of the raw response
if (extras?.analytics !== "false") {
const _auditRecord = await adminClient.request(
const _auditRecord = await $api.client.request(
gql`
mutation CreatePlanningConstraintsRequest(
$destination_url: String = ""
Expand Down Expand Up @@ -103,7 +103,7 @@
// check for & add any 'positive' constraints to the formattedResult
let formattedResult: Record<string, Constraint> = {};
if (res && res.count > 0 && res.entities) {
res.entities.forEach((entity: { dataset: any }) => {

Check warning on line 106 in api.planx.uk/gis/digitalLand.ts

View workflow job for this annotation

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
// get the planx variable that corresponds to this entity's 'dataset', should never be null because our initial request is filtered on 'dataset'
const key = Object.keys(baseSchema).find(
(key) =>
Expand Down Expand Up @@ -152,7 +152,7 @@
formattedResult["designated.nationalPark"] &&
formattedResult["designated.nationalPark"].value
) {
formattedResult["designated.nationalPark"]?.data?.forEach((entity: any) => {

Check warning on line 155 in api.planx.uk/gis/digitalLand.ts

View workflow job for this annotation

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
if (
baseSchema[broads]["digital-land-entities"]?.includes(entity.entity)
) {
Expand Down Expand Up @@ -196,7 +196,7 @@

// loop through any intersecting a4 data entities and set granular planx values based on this local authority's schema
if (a4s && formattedResult["article4"].value) {
formattedResult["article4"]?.data?.forEach((entity: any) => {

Check warning on line 199 in api.planx.uk/gis/digitalLand.ts

View workflow job for this annotation

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
Object.keys(a4s)?.forEach((key) => {
if (
// these are various ways we link source data to granular planx values (see local_authorities/metadata for specifics)
Expand Down Expand Up @@ -244,7 +244,7 @@
await Promise.all(
urls.map((url) =>
fetch(url)
.then((response: { json: () => any }) => response.json())

Check warning on line 247 in api.planx.uk/gis/digitalLand.ts

View workflow job for this annotation

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
.catch((error: any) => console.log(error)),
),
)
Expand Down
2 changes: 1 addition & 1 deletion api.planx.uk/inviteToPay/createPaymentSendEvents.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe("Create payment send events webhook", () => {
name: "GetTeamSlugByFlowId",
matchOnVariables: false,
data: {
flows_by_pk: {
flows: {
DafyddLlyr marked this conversation as resolved.
Show resolved Hide resolved
team: {
slug: "southwark",
},
Expand Down
17 changes: 12 additions & 5 deletions api.planx.uk/inviteToPay/createPaymentSendEvents.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { ComponentType } from "@opensystemslab/planx-core/types";
import { NextFunction, Request, Response } from "express";
import { gql } from "graphql-request";
import { $api } from "../client";
import { adminGraphQLClient as adminClient } from "../hasura";
import { $api, $public } from "../client";
import {
ScheduledEventResponse,
createScheduledEvent,
Expand Down Expand Up @@ -121,11 +120,19 @@ const createPaymentSendEvents = async (
}
};

interface GetTeamSlugByFlowId {
flows: {
team: {
slug: string;
};
};
}

const getTeamSlugByFlowId = async (id: Flow["id"]): Promise<Team["slug"]> => {
const data = await adminClient.request(
const data = await $public.client.request<GetTeamSlugByFlowId>(
gql`
query GetTeamSlugByFlowId($id: uuid!) {
flows_by_pk(id: $id) {
flows: flows_by_pk(id: $id) {
team {
slug
}
Expand All @@ -135,7 +142,7 @@ const getTeamSlugByFlowId = async (id: Flow["id"]): Promise<Team["slug"]> => {
{ id },
);

return data.flows_by_pk.team.slug;
return data.flows.team.slug;
};

export { createPaymentSendEvents };
4 changes: 1 addition & 3 deletions api.planx.uk/notify/routeSendEmailRequest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,7 @@ describe("Send Email endpoint", () => {
const softDeleteSessionMock = queryMock
.getCalls()
.find((mock) => mock.id === "SoftDeleteLowcalSession");
expect(
softDeleteSessionMock?.response.data.update_lowcal_sessions_by_pk.id,
).toEqual("123");
expect(softDeleteSessionMock?.response.data.session.id).toEqual("123");
});
});
});
Expand Down
34 changes: 18 additions & 16 deletions api.planx.uk/saveAndReturn/resumeApplication.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,33 @@ import app from "../server";
import { queryMock } from "../tests/graphqlQueryMock";
import { mockLowcalSession, mockTeam } from "../tests/mocks/saveAndReturnMocks";
import { buildContentFromSessions } from "./resumeApplication";
import { PartialDeep } from "type-fest";

const ENDPOINT = "/resume-application";
const TEST_EMAIL = "[email protected]";

type DeepPartial<T> = T extends object
? {
[P in keyof T]?: DeepPartial<T[P]>;
}
: T;

const mockFormatRawProjectTypes = jest
.fn()
.mockResolvedValue(["New office premises"]);

jest.mock("@opensystemslab/planx-core", () => {
const actualCoreDomainClient = jest.requireActual(
"@opensystemslab/planx-core",
).CoreDomainClient;

return {
CoreDomainClient: jest.fn().mockImplementation(() => ({
formatRawProjectTypes: () => mockFormatRawProjectTypes(),
})),
CoreDomainClient: class extends actualCoreDomainClient {
constructor() {
super();
this.formatRawProjectTypes = () => mockFormatRawProjectTypes();
}
},
};
});
Comment on lines 16 to 29
Copy link
Contributor Author

@DafyddLlyr DafyddLlyr Nov 3, 2023

Choose a reason for hiding this comment

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

What's going on here?

CoreDomainClient client is being mocked so that we can control and modify the behaviour of formatRawProjectTypes() and test it.

However, we want to retain the behaviour of CoreDomainClient.client so that real GraphQL queries get made so that graphql-query-test-mock is able to catch and mock these calls. To do this we can use jest.requireActual to maintain this behaviour.

I feel like there's possibly a few more elegant options here - I'd like to revisit this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This structure is repeated a few times throughout this PR. I spent a good bit of time attempting to generalise it so that we could import it with an interface similar to -

function mockCoreDomainClient(customMocks = {}) {
   const actualCoreDomainClient = jest.requireActual(
    "@opensystemslab/planx-core",
  ).CoreDomainClient;

  return {
    CoreDomainClient: class extends actualCoreDomainClient {
      constructor() {
        super();
        Object.assign(this, customMocks)
      }
    }
  }
}

...

mockCoreDomainClient({
  formatRawProjectTypes: () => mockFormatRawProjectTypes(),
});

Tried a few variations without much success - I suspect the problem is related to how Jest is hoisting mocks (through the jest.mock("@opensystemslab/planx-core") notation). I wanted to just move on as opposed to losing more time here so I'll open a GitHub issue describing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue logged here - #2404


describe("buildContentFromSessions function", () => {
it("should return correctly formatted content for a single session", async () => {
const sessions: DeepPartial<LowCalSession>[] = [
const sessions: PartialDeep<LowCalSession>[] = [
{
data: {
passport: {
Expand Down Expand Up @@ -62,7 +64,7 @@ describe("buildContentFromSessions function", () => {
});

it("should return correctly formatted content for multiple session", async () => {
const sessions: DeepPartial<LowCalSession>[] = [
const sessions: PartialDeep<LowCalSession>[] = [
{
data: {
passport: {
Expand Down Expand Up @@ -137,7 +139,7 @@ describe("buildContentFromSessions function", () => {
});

it("should handle an empty address field", async () => {
const sessions: DeepPartial<LowCalSession>[] = [
const sessions: PartialDeep<LowCalSession>[] = [
{
data: {
passport: {
Expand Down Expand Up @@ -170,7 +172,7 @@ describe("buildContentFromSessions function", () => {

it("should handle an empty project type field", async () => {
mockFormatRawProjectTypes.mockResolvedValueOnce("");
const sessions: DeepPartial<LowCalSession>[] = [
const sessions: PartialDeep<LowCalSession>[] = [
{
data: {
passport: {
Expand Down Expand Up @@ -227,7 +229,7 @@ describe("Resume Application endpoint", () => {

queryMock.mockQuery({
name: "ValidateRequest",
data: { teams: null, lowcal_sessions: null },
data: { teams: null, sessions: null },
variables: body.payload,
});

Expand All @@ -249,7 +251,7 @@ describe("Resume Application endpoint", () => {
queryMock.mockQuery({
name: "ValidateRequest",
data: {
lowcal_sessions: [mockLowcalSession],
sessions: [mockLowcalSession],
teams: [mockTeam],
},
variables: body.payload,
Expand All @@ -269,7 +271,7 @@ describe("Resume Application endpoint", () => {

queryMock.mockQuery({
name: "ValidateRequest",
data: { teams: [mockTeam], lowcal_sessions: [] },
data: { teams: [mockTeam], sessions: [] },
variables: body.payload,
});

Expand Down
23 changes: 15 additions & 8 deletions api.planx.uk/saveAndReturn/resumeApplication.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { NextFunction, Request, Response } from "express";
import { gql } from "graphql-request";
import { adminGraphQLClient as adminClient } from "../hasura";
import { LowCalSession, Team } from "../types";
import { convertSlugToName, getResumeLink, calculateExpiryDate } from "./utils";
import { sendEmail } from "../notify";
import type { SiteAddress } from "@opensystemslab/planx-core/types";
import { $public } from "../client";
import { $api, $public } from "../client";

/**
* Send a "Resume" email to an applicant which list all open applications for a given council (team)
Expand Down Expand Up @@ -42,6 +41,11 @@ const resumeApplication = async (
}
};

interface ValidateRequest {
teams: Team[];
sessions: LowCalSession[] | null;
DafyddLlyr marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Validate that there are sessions matching the request
* XXX: Admin role is required here as we are relying on the combination of email
Expand All @@ -57,7 +61,7 @@ const validateRequest = async (
try {
const query = gql`
query ValidateRequest($email: String, $teamSlug: String) {
lowcal_sessions(
sessions(
where: {
email: { _eq: $email }
deleted_at: { _is_null: true }
Expand All @@ -81,16 +85,19 @@ const validateRequest = async (
}
}
`;
const { lowcal_sessions, teams } = await adminClient.request(query, {
teamSlug,
email: email.toLowerCase(),
});
const { sessions, teams } = await $api.client.request<ValidateRequest>(
query,
{
teamSlug,
email: email.toLowerCase(),
},
);

if (!teams?.length) throw Error;

return {
team: teams[0],
sessions: lowcal_sessions,
sessions: sessions || [],
DafyddLlyr marked this conversation as resolved.
Show resolved Hide resolved
};
} catch (error) {
throw Error("Unable to validate request");
Expand Down
21 changes: 13 additions & 8 deletions api.planx.uk/saveAndReturn/utils.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { SiteAddress } from "@opensystemslab/planx-core/types";
import { format, addDays } from "date-fns";
import { gql } from "graphql-request";
import { adminGraphQLClient as adminClient } from "../hasura";
import { LowCalSession, Team } from "../types";
import { Template, getClientForTemplate, sendEmail } from "../notify";
import { $public } from "../client";
import { $api, $public } from "../client";

const DAYS_UNTIL_EXPIRY = 28;
const REMINDER_DAYS_FROM_EXPIRY = [7, 1];
Expand Down Expand Up @@ -201,7 +200,7 @@ const softDeleteSession = async (sessionId: string) => {
}
}
`;
await adminClient.request(mutation, { sessionId });
await $api.client.request(mutation, { sessionId });
} catch (error) {
throw new Error(`Error deleting session ${sessionId}`);
}
Expand All @@ -223,7 +222,7 @@ const markSessionAsSubmitted = async (sessionId: string) => {
}
}
`;
await adminClient.request(mutation, { sessionId });
await $api.client.request(mutation, { sessionId });
} catch (error) {
throw new Error(`Error marking session ${sessionId} as submitted`);
}
Expand All @@ -238,25 +237,31 @@ const getSaveAndReturnPublicHeaders = (sessionId: string, email: string) => ({
"x-hasura-lowcal-email": email.toLowerCase(),
});

interface SetupEmailNotifications {
session: { hasUserSaved: boolean };
}

// Update lowcal_sessions.has_user_saved column to kick-off the setup_lowcal_expiry_events &
// setup_lowcal_reminder_events event triggers in Hasura
// Should only run once on initial save of a session
const setupEmailEventTriggers = async (sessionId: string) => {
try {
const mutation = gql`
mutation SetupEmailNotifications($sessionId: uuid!) {
update_lowcal_sessions_by_pk(
session: update_lowcal_sessions_by_pk(
pk_columns: { id: $sessionId }
_set: { has_user_saved: true }
) {
id
has_user_saved
hasUserSaved: has_user_saved
}
}
`;
const {
update_lowcal_sessions_by_pk: { has_user_saved: hasUserSaved },
} = await adminClient.request(mutation, { sessionId });
session: { hasUserSaved },
} = await $api.client.request<SetupEmailNotifications>(mutation, {
sessionId,
});
return hasUserSaved;
} catch (error) {
throw new Error(
Expand Down
12 changes: 12 additions & 0 deletions api.planx.uk/saveAndReturn/validateSession.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,24 @@ import {
stubUpdateLowcalSessionData,
} from "../tests/mocks/saveAndReturnMocks";
import type { Node, Flow, Breadcrumb } from "../types";
import { userContext } from "../modules/auth/middleware";
import { getJWT } from "../tests/mockJWT";

const validateSessionPath = "/validate-session";
const getStoreMock = jest.spyOn(userContext, "getStore");

describe("Validate Session endpoint", () => {
const reconciledData = omit(mockLowcalSession.data, "passport");

beforeEach(() => {
getStoreMock.mockReturnValue({
user: {
sub: "123",
jwt: getJWT({ role: "teamEditor" }),
},
});
});

afterEach(() => {
queryMock.reset();
});
Expand Down
Loading
Loading