Skip to content

Commit

Permalink
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
Browse files Browse the repository at this point in the history
…ss/selectable-planning-constraints
  • Loading branch information
jessicamcinchak committed Nov 25, 2024
2 parents b575d1d + 64e398f commit 4147b44
Show file tree
Hide file tree
Showing 18 changed files with 417 additions and 98 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/cron-deploy-to-production.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
# Requires a Personal Access Token generated by an OSL GitHub org admin
# Allows this action to trigger the push-production.yml action
# PAT requires the "Read and write" permissions on repository "Contents"
# XXX: Expires 23/11/24, see docs here on generating a new one - https://bit.ly/3YNr5sM
# Note: Jess generated latest token, expires 23/11/25, see docs here on generating a new one - https://bit.ly/3YNr5sM
PERSONAL_ACCESS_TOKEN: ${{ secrets.PROD_DEPLOY_PAT }}
run: |
set -xe
Expand Down
4 changes: 4 additions & 0 deletions api.planx.uk/.env.test.example
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,7 @@ ORDNANCE_SURVEY_API_KEY=👻

IDOX_NEXUS_TOKEN_URL=👻
IDOX_NEXUS_SUBMISSION_URL=👻

# Analytics
METABASE_API_KEY=👻
METABASE_URL_EXT=metabase.example.com
51 changes: 50 additions & 1 deletion doc/how-to/how-to-add-a-secret.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ This guide will demonstrate how to -
2. Add to your local `.env` file for local development
- Note: This file is never checked into our public repository and is listed in our `.gitignore` config
3. Document the secret in `.env.example`
4. If a secret is required for a unit test, add dummy values to corresponding `.env.test` files e.g `YOUR_NEW_SECRET=test`

### Docker Environments (Local development + Pizza environments)
To pass a secret into our Docker Compose setup you will need to map it into the relevant container in `docker-compose.yml`. For example -
Expand All @@ -35,6 +36,39 @@ When building Pizza environments for testing, GitHub actions access secrets via
> Please be aware that if you are rotating secrets this may affect existing Pizzas which will need to be rebuilt. This can be done manually in GitHub by re-running the latest action associated with affected PRs.


## Diagram - Docker environments
```mermaid
flowchart LR
subgraph "Local Environment"
localEnv[".env file(s)"] --> Docker["Docker Compose"]
Docker --> API
Docker --> Hasura
Docker --> Frontend
end
subgraph Staging AWS S3 bucket
S3
end
subgraph GitHub actions
pizzaEnv[".env file"]
end
subgraph "Pizza Environment"
pizzaEnv[".env file"] --> PizzaDocker["Docker Compose"]
PizzaDocker --> PizzaAPI["API"]
PizzaDocker --> PizzaHasura["Hasura"]
PizzaDocker --> PizzaFrontend["Frontend"]
end
%% Scripts reading and writing
S3 --"Pull/Push scripts"--> localEnv
%% CI
S3 --> pizzaEnv
```


### AWS / Pulumi Environments (Staging + Production environments)
Secrets for Staging and Production environment are not handled in `.env` files, and are set directly in Pulumi, our Infrastruture as Code (IaC) platform.

Expand Down Expand Up @@ -68,4 +102,19 @@ const apiService = new awsx.ecs.FargateService("api", {
});
```

> Pulumi uses our Docker images to construct Fargate services. This means that the "name" value above must match that used in Docker.
> Pulumi uses our Docker images to construct Fargate services. This means that the "name" value above must match that used in Docker.

## Diagram - AWS / Pulumi Environments
```mermaid
flowchart LR
subgraph "Staging & Production"
direction LR
PulumiService["Pulumi Service"] --"Private key"--> Pulumi
PulumiFile["pulumi.staging.yaml"] --"Public key"--> Pulumi
Pulumi["Pulumi IaC code"] --Decrypted--> Fargate
Fargate --> FargateAPI["API"]
Fargate --> FargateHasura["Hasura"]
Fargate --> FargateFrontend["Frontend"]
end
```
2 changes: 2 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ services:
IDOX_NEXUS_TOKEN_URL: ${IDOX_NEXUS_TOKEN_URL}
JWT_SECRET: ${JWT_SECRET}
MAPBOX_ACCESS_TOKEN: ${MAPBOX_ACCESS_TOKEN}
METABASE_API_KEY: ${METABASE_API_KEY}
METABASE_URL_EXT: ${METABASE_URL_EXT}
MINIO_PORT: ${MINIO_PORT}
ORDNANCE_SURVEY_API_KEY: ${ORDNANCE_SURVEY_API_KEY}
PORT: ${API_PORT}
Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/ui-driven/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
},
"packageManager": "[email protected]",
"devDependencies": {
"@playwright/test": "^1.40.1",
"@playwright/test": "^1.49.0",
"@types/node": "18.16.1",
"eslint-plugin-playwright": "^0.20.0"
}
Expand Down
27 changes: 13 additions & 14 deletions e2e/tests/ui-driven/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import React from "react";
import { act } from "react-dom/test-utils";
import { ErrorBoundary } from "react-error-boundary";
import swr from "swr";
import useSWR from "swr";
import { setup } from "testUtils";
import { vi } from "vitest";
import { axe } from "vitest-axe";
Expand All @@ -21,15 +22,13 @@ const { setState } = useStore;

beforeEach(() => vi.clearAllMocks());

const swrMock = (swr as jest.Mock).mock;

vi.mock("swr", () => ({
default: vi.fn((url: () => string) => {
const isGISRequest = url()?.startsWith(
`${import.meta.env.VITE_APP_API_URL}/gis`
`${import.meta.env.VITE_APP_API_URL}/gis`,
);
const isRoadsRequest = url()?.startsWith(
`${import.meta.env.VITE_APP_API_URL}/roads`
`${import.meta.env.VITE_APP_API_URL}/roads`,
);

if (isGISRequest) return { data: digitalLandResponseMock };
Expand All @@ -50,7 +49,7 @@ describe("error state", () => {
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={vi.fn()}
/>
</ErrorBoundary>
</ErrorBoundary>,
);

expect(getByTestId("error-summary-invalid-graph")).toBeInTheDocument();
Expand All @@ -66,7 +65,7 @@ describe("error state", () => {
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
/>
</ErrorBoundary>
</ErrorBoundary>,
);
const results = await axe(container);
expect(results).toHaveNoViolations();
Expand All @@ -82,7 +81,7 @@ describe("following a FindProperty component", () => {
teamIntegrations: {
hasPlanningData: true,
},
})
}),
);
});

Expand All @@ -96,11 +95,11 @@ describe("following a FindProperty component", () => {
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={handleSubmit}
/>
/>,
);

expect(
getByRole("heading", { name: "Planning constraints" })
getByRole("heading", { name: "Planning constraints" }),
).toBeInTheDocument();

await user.click(getByTestId("continue-button"));
Expand All @@ -115,7 +114,7 @@ describe("following a FindProperty component", () => {
description="Things that might affect your project"
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
/>
/>,
);
const results = await axe(container);
expect(results).toHaveNoViolations();
Expand All @@ -129,14 +128,14 @@ describe("following a FindProperty component", () => {
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={vi.fn()}
/>
/>,
);

expect(swr).toHaveBeenCalled();

// Planning data is called first
const swrURL = swrMock.calls[0][0]();
const swrResponse = swrMock.results[0].value;
const swrURL = (vi.mocked(useSWR).mock.calls[0][0] as () => {})();
const swrResponse = vi.mocked(useSWR).mock.results[0].value;

expect(swrURL).toContain("/gis");
expect(swrResponse).toEqual({ data: digitalLandResponseMock });
Expand All @@ -150,14 +149,14 @@ describe("following a FindProperty component", () => {
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={vi.fn()}
/>
/>,
);

expect(swr).toHaveBeenCalled();

// Classified roads are called second
const swrURL = swrMock.calls[1][0]();
const swrResponse = swrMock.results[1].value;
const swrURL = (vi.mocked(useSWR).mock.calls[1][0] as () => {})();
const swrResponse = vi.mocked(useSWR).mock.results[1].value;

expect(swrURL).toContain("/roads");
expect(swrResponse).toEqual({ data: classifiedRoadsResponseMock });
Expand All @@ -171,7 +170,7 @@ describe("following a FindProperty component", () => {
teamIntegrations: {
hasPlanningData: true,
},
})
}),
);

setup(
Expand All @@ -181,23 +180,25 @@ describe("following a FindProperty component", () => {
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={vi.fn()}
/>
/>,
);

expect(swr).toHaveBeenCalled();

// Planning constraints API still called
const planingConstraintsURL = swrMock.calls[0][0]();
const planingConstraintsResponse = swrMock.results[0].value;
const planingConstraintsURL = (
vi.mocked(useSWR).mock.calls[0][0] as () => {}
)();
const planingConstraintsResponse = vi.mocked(useSWR).mock.results[0].value;

expect(planingConstraintsURL).toContain("/gis");
expect(planingConstraintsResponse).toEqual({
data: digitalLandResponseMock,
});

// Classified roads API not called due to missing USRN
const swrURL = swrMock.calls[1][0]();
const swrResponse = swrMock.results[1].value;
const swrURL = (vi.mocked(useSWR).mock.calls[1][0] as () => {})();
const swrResponse = vi.mocked(useSWR).mock.results[1].value;

expect(swrURL).toBeNull();
expect(swrResponse).toEqual({ data: null });
Expand All @@ -211,12 +212,12 @@ describe("following a FindProperty component", () => {
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={vi.fn()}
/>
/>,
);

// Positive constraints visible by default
expect(
getByRole("heading", { name: /These are the planning constraints/ })
getByRole("heading", { name: /These are the planning constraints/ }),
).toBeVisible();
expect(getByRole("button", { name: /Parks and gardens/ })).toBeVisible();

Expand All @@ -227,7 +228,7 @@ describe("following a FindProperty component", () => {
expect(showNegativeConstraintsButton).toBeVisible();

const negativeConstraintsContainer = getByTestId(
"negative-constraints-list"
"negative-constraints-list",
);
expect(negativeConstraintsContainer).not.toBeVisible();

Expand All @@ -248,12 +249,12 @@ describe("following a FindProperty component", () => {
description="Things that might affect your project"
fn="property.constraints.planning"
handleSubmit={vi.fn()}
/>
/>,
);
expect(
queryByText(
"This page does not include information about historic planning conditions that may apply to this property."
)
"This page does not include information about historic planning conditions that may apply to this property.",
),
).toBeVisible();
});
});
Expand All @@ -268,7 +269,7 @@ describe("demo state", () => {
hasPlanningData: false,
},
teamSlug: "demo",
})
}),
);
});
it("should render an error when teamSlug is demo", async () => {
Expand All @@ -282,21 +283,21 @@ describe("demo state", () => {
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={handleSubmit}
/>
</ErrorBoundary>
</ErrorBoundary>,
);

const errorMessage = queryByText(
"Planning Constraints are not enabled for demo users"
"Planning Constraints are not enabled for demo users",
);
expect(errorMessage).toBeVisible();

// Check planning constraints has not rendered
// reused positive constraints from basic layout test
expect(
queryByRole("heading", { name: /These are the planning constraints/ })
queryByRole("heading", { name: /These are the planning constraints/ }),
).not.toBeInTheDocument();
expect(
queryByRole("button", { name: /Parks and gardens/ })
queryByRole("button", { name: /Parks and gardens/ }),
).not.toBeInTheDocument();

// Ensure a demo user can continue on in the application
Expand Down
Loading

0 comments on commit 4147b44

Please sign in to comment.