From 4cdc93ff86c3e1cfc29ff7baecf497ea1c8b8b46 Mon Sep 17 00:00:00 2001 From: Mouhajer-CO Date: Tue, 20 Feb 2024 07:48:15 +0000 Subject: [PATCH 01/14] Add Project Structure and Code Style docs --- docs/Project Structure and Code Style.md | 212 +++++++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 docs/Project Structure and Code Style.md diff --git a/docs/Project Structure and Code Style.md b/docs/Project Structure and Code Style.md new file mode 100644 index 0000000..937bd12 --- /dev/null +++ b/docs/Project Structure and Code Style.md @@ -0,0 +1,212 @@ +# Project Structure and Code Style + +## Github Request App + +Github Request App is a simple app with a few endpoints, each endpoint represents a possible action to the Github account (eg. add member to org/team, remove team ...). +All information filled by the user will be saved on a browser cookie which is configured to sign the cookie with the `COOKIE_SESSION_SECRET` environment variable. The payload stored in the cookie is also protected by the `crypto` module meaning the client side data is both encrypted and signed. + +Generally we will fetch information from the session, on the GET controller, to be used to populate data on the view. Instead the saving of user data into the session is done on the POST controller of each page. +It is important to remember that there is a mapping between the saved data on the session and the user data passed to the view for visualization. + +The compiled/transpiled project is copied into the dist folder used later to bootstrap the application from `dist/server.js`. All static files will be fetched from a CDN on aws cloudfront service. + +## Files Structure + +Directory Path | Description +--- | --- +`./.github` | Github folder, includes `PULL_REQUEST_TEMPLATE.md` on how to make a pull request to the project and `dependabot.yml` configuration options for dependency updates. +`./.husky` | Add pre check script, includes `pre-commit` and `pre-push` checks +`./src` | Contains all Typescript code +`./src/app.ts` | Application entry point +`./src/server.ts` | Server configuration +`./src/config/index.ts` | Contains all the application's configurations +`./src/controller` | Business logic and handlers +`./src/middleware` | Middleware functions (Authentication, validation ...) +`./src/model` | Github Request Session and View Data Model +`./src/routes` | Paths and routes controller (Only GET and POST enabled) +`./src/service` | Interface to the API through SDK +`./src/utils` | Facade for CO services (e.g. logging) and other application utils (cookie, application data ...) +`./src/validation` | Sets of express validator middlewares for each page +`./test` | Jest Test files (`*.spec.ts`, `setup.ts`, and `*.mocks.ts`) +`./view` | Contains all the html nunjucks structure files +`./docs` | Contains documentation files +Others files | Other files related to modules dependency, CI/CD, *git, dockerization, lint, test/typescript configs … + +## MVC + +Each page or user interface, defined by an endpoint, is divided into three components (MVC) and as a best practice the names for the model, view and controller have, when possible, the same start name of the endpoints (e.g. for the `/confirmation` page we have the: `confirmation.controller.ts` and `confirmation.html` files. If models were present, we would have `confirmation.model.ts`) + +### Model + +In the model we define the interface, the data structure used to represent the data for that particular page and an array used to map back and forth information between the `session` data and the `nunjucks` html view data. + +```js +// Remove Member Page Model +export const RemoveMemberKey = "remove-member"; + +export const RemoveMemberKeys: string[] = ["github_handle", "description"]; + +export interface Presenter { + github_handle?: string + description?: string +} +``` + +For each interface we have a key used to represent the object on the application data model, and the `ApplicationData` represents the object that will be saved in the session, as a subfield of our session data. + +```js +// Github Request Application Data model +export interface ApplicationData { + remove-member?: RemoveMember; + ​...​ +} +``` + +### View + +We utilize both `Nunjucks` and `GDS` style/components. To streamline the construction of pages, we employ common components that are utilized across the UI, which are stored in an includes directory within the `view` directory. This directory contains all the useful shared chunks of HTML code. This approach ensures consistency in error messaging, input formats, and other aspects. + +The data value is passed by the `GET` controller using the `res.render(templateUrl, {...data})` method. If the value is not set, the input field will remain empty. Additionally, if specificity is required, we can include a variable using the `set` command, as demonstrated in the example below. + +```js +// Nunjucks HTML inputs field for removing a github member +{% extends "layout.html" %} + +{% block beforeContent %} + {% include "include/back-link.html" %} +{% endblock %} + +{% block content %} +
+
+ ... + {% include "include/error-list.html" %} + +
+ + {% include "include/github-handle.html" %} + + {% set descriptionText = "Description (optional)" %} + {% set descriptionHint = "Include a description on why this user is getting removed." %} + + {% include "include/description.html" %} + + {% include "include/save-button.html" %} + +
+
+
+{% endblock %} +``` + +### Controller + +Generally only POST and GET http methods are allowed, and therefore we will have mainly just the get and post controllers, and it is literally the last successful middleware of the chain that has the duty to respond to the user. +In the `get` method we fetch possible data and pass it to the view/template to be visualized to the user + +```js +// Github Request for the Remove Member page +export const get = (req: Request, res: Response, next: NextFunction) => { + try { + ... + const appData: ApplicationData = getSessionData(req.session); + const removeMember = appData[RemoveMemberKey]; + + return res.render(config.REMOVE_MEMBER, { + ...removeMember, + ... + }); + } catch (error) { + ... + } +}; + +``` + +and in the `post` method we save the user data every time that a page is submitted. + +```js +// Post controller for the Presenter page +export const post = async (req: Request, res: Response, next: NextFunction) => { + try { + ... + const data = mapData(req.body, RemoveMemberKeys); + setSessionData(req.session, data, RemoveMemberKey); + ... + + return res.redirect(config.HOME); + } catch (error) { + ... + } +}; +``` + +## Authentication + +Authentication is a simple middleware that checks if the user has got a valid signed cookie, otherwise it will be redirected to sign in page `res.redirect(AUTH_SIGN_IN_URL);`. + +```js +export const colaAuthenticationMiddleware = async ( req: Request, res: Response, next: NextFunction) => { + + const cookieSignedValue = getCookieValue(req.signedCookies, COOKIE_ID_NAME); + const unsignedCookie = getUnsignedCookie(cookieSignedValue, COOKIE_PARSER_SECRET); + + if (validateUnsignedCookie(unsignedCookie)) { + log.debug(`Successfully verified signature for ${COOKIE_ID_NAME}, cookie value: ${unsignedCookie}`); + } else { + log.error(`Failed to verify signature for ${COOKIE_ID_NAME}, cookie value: ${cookieSignedValue}, redirect to ${AUTH_SIGN_IN_URL}`); + return res.redirect(AUTH_SIGN_IN_URL); + } + + next(); +}; +``` + +To chain the middleware to the particular endpoint we add it to the router object like `router.METHOD(path, [callback, ...] callback)` as described [here](https://expressjs.com/en/5x/api.html#router.METHOD) + +```js +// Chain middlewares for the `remove-member` endpoints +const removeMemberRouter = Router(); + +removeMemberRouter.get(config.REMOVE_MEMBER_URL, authentication, get); + +removeMemberRouter.post(config.REMOVE_MEMBER_URL, authentication, ...removeMember, checkValidations, post); + +export default removeMemberRouter; +``` + +## Validation + +In each `POST` endpoints for every page we have a sets of middlewares used to validate each fields submitted by the user, if one of the validation middlewares fails the `validationResult` [here](https://github.com/cabinetoffice/github-requests-app/blob/c1923314f23897a809624a8ec208648cab228b4e/src/middleware/validation.middleware.ts#L7) will extracts the validation errors from a request (`req` object) and it will be formatted as an `errors` object [here](https://github.com/cabinetoffice/github-requests-app/blob/c1923314f23897a809624a8ec208648cab228b4e/src/middleware/validation.middleware.ts#L27) and it will be passed to the render page for the correct error visualization. + +```js +// Middlewares validation checks for the remove-member page +import { body } from 'express-validator'; + +import { ErrorMessages } from './error.messages'; +import { descriptionValidation } from './fields/description.validation'; + +export const removeMember = [ + body('github_handle').not().isEmpty({ ignore_whitespace: true }).withMessage(ErrorMessages.GIT_HANDLE), + ...descriptionValidation +]; +``` + +```js +// Inputs field on remove-member page with the errors object +// content file for include/description.html + {{ govukTextarea({ + errorMessage: errors.description if errors, + value: description, + name: "description", + id: "description", + label: { + text: descriptionText, + classes: "govuk-label--m", + isPageHeading: true + }, + hint: { + text: descriptionHint + } + }) }} +``` From 59cb9cf204a88277f5e94df25b86b9378d5b046e Mon Sep 17 00:00:00 2001 From: Mouhajer-CO Date: Tue, 20 Feb 2024 07:52:49 +0000 Subject: [PATCH 02/14] Add GitHub Requests (Application & Terraform) docs --- ...tHub Requests (Application & Terraform).md | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 docs/GitHub Requests (Application & Terraform).md diff --git a/docs/GitHub Requests (Application & Terraform).md b/docs/GitHub Requests (Application & Terraform).md new file mode 100644 index 0000000..34b9a75 --- /dev/null +++ b/docs/GitHub Requests (Application & Terraform).md @@ -0,0 +1,91 @@ +# GitHub Requests (Application & Terraform) + +**GitHub Requests Application** is a key component designed to handle GitHub requests in a secure and consistent manner by incorporating automation with Terraform (repository [here](https://github.com/cabinetoffice/github-requests-terraform)) and implementing security access through the COLA Identity Provider. +For example, when a new member wishes to add their GitHub account to a team, they submit an issue through this app. The IDP team verifies the issues, applies changes, and merges the Pull Request to the main branch. On the next Terraform run, the changes propagate to GitHub, granting access. This entire process occurs with complete visibility within the department, ensuring consistency, instead of relying on a single user to make changes through GitHub's web interface. + +## Workflow + +1. User makes a request through the GitHub application. +2. After authentication with COLA, user's request starts creating the data object. +3. The user fills in all the details and submits the object from the `Check Your Answers` page, triggering: + 1. Terraform Repository: An issue is created. + 2. DynamoDB: User information is saved. + 3. Email Service: An email is sent to the user to confirm the information sent. +4. Platform team assesses the issue in the Terraform repository. + 1. Upon readiness, a plan and apply are triggered. + 2. DynamoDB database is accessed for user information if needed (e.g., to send email to user with expired contract). +5. Notify sends confirmation or any updates to the user. + +## Overview and Design + +The GitHub Requests Application is a Node.js-based web application that provides a simple interface. Internal users (members of the Cognito user pool) fill in forms with requested details. When a request is submitted, an issue is created on the dedicated terraform repository, and an email is sent to the user from our `github-request.idp` email address. The email includes a message containing the user's filled-in information. + +The issue is then reviewed by the team, and further comments may be requested if necessary. Approval must be granted by two members of the team to avoid misconfigurations. + +### Sending an Email + +The Node.js app will send the email with Amazon SES, and you can find the implementation details [here](https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/ses-examples-sending-email.html#ses-examples-sendmail), in particular we will create an object to pass the parameter values that define the email to be sent, including sender and receiver addresses, subject, and email body in plain text (with optional HTML). + +### Github Issue + +The issue will be created by using the CO `@co-digital/api-sdk` module. It has already been tested locally with the following snippets (eg [here](https://github.com/Mouhajer-CO/git-api-calls/blob/14f7bd33e9ac579fead0b2003c35946a5b0cacf7/src/utils.ts#L43)): + +```js +export const createIssue = async (title: string, description: string) => { + try { + const url = ` + const body = { title, body: description, assignees: [ASSIGNEE], labels: [LABEL] }; + + return await fetchCall(url, body); + } catch (error: any) { + console.error('Error:', error); + } +} +``` + +### Terraform Code + +We will utilize Terraform and the GitHub provider to create resources and invite users to the GitHub organization by defining separate JSON files. Possible implementation details can be found [here](https://developer.hashicorp.com/terraform/tutorials/it-saas/github-user-teams). We will use JSON file data with the following function to retrieve information: `jsondecode(file("${path.module}/teams.json"))`. We could use an S3 bucket and push members/teams/repos data before terraform apply. + +Initially, we need to use the `terraform import` command to import existing infrastructure into terraform, utilizing JSON files for members, teams, repositories, members per team, and so on. This configuration scales better and enables the team to follow best practices safely, consistently, and efficiently. + +### Set up + +- Import repos, teams and members details through the api sdk node project +- Set `.tf` configuration file for your organization as json files on datasets folder +- Run basic commands `terraform init`, `terraform fmt` and `terraform validate` +- Set Resources configurations in the root module, it will be removed later after state file generated when resources imported. + +```t + resource "github_repository" "terraform-modules" { + # (resource arguments) + } +``` + +- Import the actual repository using terraform import github_repository.your-repo your-repo +- Run terraform plan to ensure that your configuration matches what is on GitHub, or change the configuration until it matches. +- Run terraform apply. +- Remove ownership to members not part of terraform platform team (at least five people) + +## Benefits + +- **Programmatic Approach**: Implementation of a systematic and automated process. +- **Authorized Users**: All users are part of defined Cognito user pools, ensuring authorized access. +- **Detailed Information**: Comprehensive information from required UI inputs (eg. distinguishing between long and short-term users). +- **Least Privilege Access to GitHub UI**: Minimized access in the GitHub UI to maintain security (limiting OWNER roles to at least two members, no more then five). +- **Decreased Risk of Errors**: Reduction in errors compared to manual UI interactions, minimizing the potential for mistakes. +- **Increased Visibility**: All changes are reviewed and tracked in version control, involving at least two team members. +- **Increased Consistency**: Uniform application of changes across all repositories, addressing issues such as the absence of branch protection and team maintainers. +- **Single Source of Truth**: Changes are exclusively applied through Terraform, eliminating the need for manual UI alterations. + +## Possible issues + +- The GitHub API appears to have slow performance, and the terraform apply process can take 30 minutes or more for large organizations, as mentioned in this [issue](https://github.com/integrations/terraform-provider-github/issues/567). To address this, we could consider using a Node.js script to fetch repository data ahead of time to optimize terraform timing. + +- How can we ensure that the user pool contains only the required users? All internal members are allowed, and there must be at least one maintainer per team, as well as per repository. External team, contractor, or any other outsourcing company have to be part of the Cognito Users pool, and they need to request access ahead of time. Update to new entry wiki page has to be done. + +- The Terraform configuration serves as the single source of truth and policy for most of Github requests. No manual changes should be made to prevent configuration inconsistencies. Owners have to (and will) be reduced. + +- To reduce seats, maintainers for all users need to be identified. + +- Importing existing infrastructure might be time-consuming, especially if we need to remove unused teams, members, or repositories, and the terraform apply process must be carefully defined the first time to avoid issues (backup in place?). From f55908289fdc63939330093ba6c95d3df7bd92d2 Mon Sep 17 00:00:00 2001 From: Mouhajer-CO Date: Tue, 20 Feb 2024 07:54:04 +0000 Subject: [PATCH 03/14] Update Readme --- README.md | 123 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 69 insertions(+), 54 deletions(-) diff --git a/README.md b/README.md index 82df8b6..1601925 100644 --- a/README.md +++ b/README.md @@ -1,81 +1,96 @@ -# GitHub requests app +# GitHub Requests App + ![Static Badge](https://img.shields.io/badge/test_coverage-%E2%89%A595%25-green) +The GitHub request application is a tool designed to streamline and automate the process of managing and tracking request for GitHub. This includes the adding, removal or editing of members. + ## Overview -The GitHub request application is a tool designed to streamline and automate the process of managing and tracking request for GitHub. This includes the adding, removal or editing of members. -Each page or user interface, defined by an endpoint, is divided into three components (MVC) and as a best practice the names for the model, view and controller have, when possible, the same start name of the endpoints (e.g. for the `/confirmation` page we have the: `confirmation.controller.ts` and `confirmation.html` files. If models were present, we would have `confirmation.model.ts`) +The GitHub Requests Application is a Node.js-based web application that provides a simple interface. Internal users (members of the Cognito user pool) fill in forms with requested details. When a request is submitted, an issue is created on the dedicated terraform repository, and an email is sent to the user from our `github-request.idp` email address. The email includes a message containing the user's filled-in information. + +The issue is then reviewed by the team, and further comments may be requested if necessary. Approval must be granted by two members of the team to avoid misconfigurations. + +## Frontend Technologies and Utils + +- [NodeJS](https://nodejs.org/) +- [ExpressJS](https://expressjs.com/) +- [Typescript](https://www.typescriptlang.org/) +- [NunJucks](https://mozilla.github.io/nunjucks) +- [GOV.UK Design System](https://design-system.service.gov.uk/) +- [Jest](https://jestjs.io) +- [SuperTest](https://www.npmjs.com/package/supertest) +- [Docker](https://www.docker.com/) +- [Git](https://git-scm.com/downloads) +- [Terraform](https://www.terraform.io/) +- [AWS](https://aws.amazon.com/) + +### Config variables + +Key | Description | Example Value +----------------|--------------------------- |------------------------- +PATH_SSL_PRIVATE_KEY | Path to ssl private key | `./infrastructure/host/test.key` +PATH_SSL_CERTIFICATE | Path to ssl certificate | `./infrastructure/host/test.cert` +BASE_URL | Base application URL | `http://localhost:3000` (dev mode) +NODE_ENV | Node environment | `development` (or `production`) +PORT | Server port number | `3000` +CDN_HOST | CDN host | `cdn domain` +USER_POOL_ID | ID of the user pool in Amazon Cognito | `secret` +USER_POOL_CLIENT_ID | Client ID of an app registered with the user pool in Amazon Cognito | `secret` +AUTH_SIGN_IN_URL | Authentication sign in URL | `https://cola.service.cabinetoffice.gov.uk/v2//login` +COOKIE_ID_NAME | The name of the cookie | `github-requests` +COOKIE_PARSER_SECRET | Secret used in validating/calculating the cookie signature | `secret` +COOKIE_SESSION_SECRET | Secret key for signing the session cookie | `secret` +LANDING_PAGE_URL | Github Requests landing Page | `/home/` +LOG_LEVEL | Logging levels | `/home/` +HUMAN | Formatting messages form (default JSON) | `true` (Enable human formatting for log messages) +## Launching the web-app +### Prerequisites + +1. Install [NodeJS V20.8](https://nodejs.org/en) +2. Install [Docker](https://www.docker.com/get-started) -### The View +### Running local development environment with Docker -We use `Nunjucks` and `GDS` style/components. +Docker is used to run the application in **development** mode, with tooling setup to detect changes in local `src` directory and reload the container's node server. Ensure that `NODE_ENV=development` is set in the `.env` file. -### The controller +### Building the Docker Image -Generally only POST and GET http methods are allowed, and therefore we will have mainly just the get and post controllers, and it is literally the last successful middleware of the chain that has the duty to respond to the user. -In the `get` method we fetch possible data and pass it to the view/template to be visualized to the user and in the `post` method we save the user data everytime that a page is submitted +1. Create a copy of the `.env.example` file and name it `.env`: +Then run: -## Files Structure +```sh +make docker-build +make docker-up +``` -Directory Path | Description ---- | --- -`./.github` | Github folder, includes `PULL_REQUEST_TEMPLATE.md` on how to make a pull request to the project and `dependabot.yml` configuration options for dependency updates. -`./.husky` | Add pre check script, includes `pre-commit` and `pre-push` checks -`./src` | Contains all Typescript code -`./src/app.ts` | Application entry point -`./src/bin/www.ts` | Server configuration -`./src/config/index.ts` | Contains all the application's configurations -`./src/controller` | Business logic and handlers -`./src/middleware` | Middleware functions (Authentication, validation ...) -`./src/model` | OE Session and View Data Model -`./src/routes` | Paths and routes controller (Only GET and POST enabled) -`./src/service` | Interface to the API through SDK -`./src/utils` | Facade for CO services (e.g. logging) and other application utils (navigation, application data ...) -`./src/validation` | Sets of express validator middlewares for each page -`./test` | Jest Test files (`*.spec.ts`, `setup.ts`, and `*.mocks.ts`) -`./view` | Contains all the html nunjucks structure files -`./docs` | Contains documentation files -Others files | Other files related to modules dependency, CI/CD, *git, dockerization, lint, test/typescript configs … +This will then download the necessary dependencies, build the Docker image, and start the application. You will be able to access it on [localhost:3000](localhost:3000). ## ESlint We use ESlint as both a formatter and code quality assurance. Eslint can also be setup to format on save using a VScode extension: 1. Install the [ESlint VScode extenstion](https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint). + 2. Open your user settings (JSON) inside VScode and add the following: -``` + + ```js "editor.formatOnSave": true, "editor.codeActionsOnSave": { "source.fixAll.eslint": true } -``` + ``` 3. Reload VScode. -## Running local development environment with Docker - -Docker is used run the application in development mode, with tooling setup to detect changes in local `src` directory and reload the container's node server. - -Follow the steps in [Launching-the-web-app](#Launching-the-web-app), and ensure that `NODE_ENV=development` is set in the `.env` file. - -## Launching the web-app - -### Prerequisites - -1. Install [NodeJS V20.8](https://nodejs.org/en) - -2. Install [Docker](https://www.docker.com/get-started) - -### Building the Docker Image - -1. Create a copy of the ``.env.example`` file and name it `.env`: - - Then run: - - make docker-build +## Recommendations - make docker-up +1. Use the [Visual Studio Code](https://code.visualstudio.com/) IDE for development. +2. Use the preformatted `PULL_REQUEST_TEMPLATE` by adding meaningful description +3. Make sure test coverage is above `95%` +4. Do not disable husky pre checks locally +5. Use MVC pattern when adding a new page/endpoint, including validation and authentication. Example can be found on the following doc description [here](./docs/Project%20Structure%20and%20Code%20Style.md) +6. **Happy coding** -This will then download the necessary dependencies, build the Docker image, and start the application. -You will be able to access it on [localhost:3000](localhost:3000). +## License +This code is open source software licensed under the [MIT License]("https://opensource.org/licenses/MIT"). From 4606c0da79241ad6e9ad621516dcf6fdec5ac723 Mon Sep 17 00:00:00 2001 From: Harley Harris Date: Tue, 20 Feb 2024 15:25:29 +0000 Subject: [PATCH 04/14] add validation to add-team-member page --- src/routes/add-team-member.ts | 4 +++- src/validation/add-team-member.validation.ts | 9 +++++++++ src/validation/error.messages.ts | 1 + src/views/add-team-member.html | 14 ++++++++------ 4 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 src/validation/add-team-member.validation.ts diff --git a/src/routes/add-team-member.ts b/src/routes/add-team-member.ts index 4466919..358d593 100644 --- a/src/routes/add-team-member.ts +++ b/src/routes/add-team-member.ts @@ -4,10 +4,12 @@ import { authentication } from '../middleware/authentication.middleware'; import { get, post } from '../controller/add-team-member.controller'; import * as config from '../config'; +import { checkValidations } from '../middleware/validation.middleware'; +import { addTeamMember } from '../validation/add-team-member.validation'; const addTeamMemberRouter = Router(); addTeamMemberRouter.get(config.ADD_TEAM_MEMBER_URL, authentication, get); -addTeamMemberRouter.post(config.ADD_TEAM_MEMBER_URL, authentication, post); +addTeamMemberRouter.post(config.ADD_TEAM_MEMBER_URL, authentication, ...addTeamMember, checkValidations, post); export default addTeamMemberRouter; diff --git a/src/validation/add-team-member.validation.ts b/src/validation/add-team-member.validation.ts new file mode 100644 index 0000000..b983d07 --- /dev/null +++ b/src/validation/add-team-member.validation.ts @@ -0,0 +1,9 @@ +import { body } from 'express-validator'; + +import { ErrorMessages } from './error.messages'; +import { teamNameValidation } from './fields/team-name.validation'; + +export const addTeamMember = [ + ...teamNameValidation, + body('team_member_github_handle').not().isEmpty({ ignore_whitespace: true }).withMessage(ErrorMessages.TEAM_MEMBER_GIT_HANDLE) +]; diff --git a/src/validation/error.messages.ts b/src/validation/error.messages.ts index 531a0d0..6bbd9a7 100644 --- a/src/validation/error.messages.ts +++ b/src/validation/error.messages.ts @@ -12,4 +12,5 @@ export enum ErrorMessages { VISIBILITY = 'Select a visibility option', TEAM_MAINTAINER_GITHUB_HANDLE = 'Enter the team maintainer GitHub handle', TEAM_NAME = 'Enter the team name', + TEAM_MEMBER_GIT_HANDLE = 'Enter the username of the member GitHub handle (aka GitHub account)' } diff --git a/src/views/add-team-member.html b/src/views/add-team-member.html index 9340917..d68f3d2 100644 --- a/src/views/add-team-member.html +++ b/src/views/add-team-member.html @@ -13,23 +13,25 @@

Add a GitHub member to a team

When a user has access to our GitHub organisations and they wish to be added to a team within the cabinetoffice GitHub organisation.

+ {% include "include/error-list.html" %} +
{% include "include/inputs/team-name.html" %} {{ govukInput({ + errorMessage: errors.team_member_github_handle if errors, label: { - text: "Member GitHub handle ", + text: "Member GitHub handle", classes: "govuk-label--m" }, classes: "govuk-input--width-10", - id: "team-member-github-handle", - name: "team_member_github_handle" + id: "team_member_github_handle", + name: "team_member_github_handle", + value: team_member_github_handle }) }} - {{ govukButton({ - text: "Save" - }) }} + {% include "include/save-button.html" %}
From 85be7e6d4369e1587133d2a0a4fa3d7158d3bebb Mon Sep 17 00:00:00 2001 From: Harley Harris Date: Tue, 20 Feb 2024 15:33:42 +0000 Subject: [PATCH 05/14] update add-team-member integration test suite --- test/integration/routes/add-team-member.spec.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/integration/routes/add-team-member.spec.ts b/test/integration/routes/add-team-member.spec.ts index 515afdc..f986cc6 100644 --- a/test/integration/routes/add-team-member.spec.ts +++ b/test/integration/routes/add-team-member.spec.ts @@ -15,6 +15,8 @@ import { authentication } from '../../../src/middleware/authentication.middlewar import { MOCK_REDIRECT_MESSAGE, MOCK_POST_ADD_TEAM_MEMBER_RESPONSE, MOCK_GET_ADD_TEAM_MEMBER_RESPONSE as MOCK_GET_ADD_TEAM_MEMBER_RESPONSE } from '../../mock/text.mock'; import { MOCK_POST_ADD_TEAM_MEMBER } from '../../mock/data'; +import { ErrorMessages } from '../../../src/validation/error.messages'; + const mockedLogger = logger as jest.Mock; mockedLogger.mockImplementation((_req: Request, _res: Response, next: NextFunction) => next()); const mockedAuth = authentication as jest.Mock; @@ -45,6 +47,20 @@ describe('add-team-member endpoint integration tests', () => { expect(mockedAuth).toHaveBeenCalledTimes(1); }); + test('Should render the same page with error messages after POST request', async () => { + const res = await request(app).post(config.ADD_TEAM_MEMBER_URL).send({ + team_name: '', + team_member_github_handle: '' + }); + + expect(res.status).toEqual(200); + expect(res.text).toContain(ErrorMessages.TEAM_NAME); + expect(res.text).toContain(ErrorMessages.TEAM_MEMBER_GIT_HANDLE); + expect(res.text).toContain(MOCK_GET_ADD_TEAM_MEMBER_RESPONSE); + expect(mockedLogger).toHaveBeenCalledTimes(1); + expect(mockedAuth).toHaveBeenCalledTimes(1); + }); + test('Should log the Team Name and Team Member GitHub handle on POST request.', async () => { const res = await request(app).post(config.ADD_TEAM_MEMBER_URL).send(MOCK_POST_ADD_TEAM_MEMBER); From e5e6547fcd7c8c50f6d4c4f7a9e074e9d924dc7e Mon Sep 17 00:00:00 2001 From: Mouhajer-CO Date: Wed, 21 Feb 2024 12:02:37 +0000 Subject: [PATCH 06/14] Update documentations --- README.md | 2 +- docs/Project Structure and Code Style.md | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 1601925..7830452 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ ![Static Badge](https://img.shields.io/badge/test_coverage-%E2%89%A595%25-green) -The GitHub request application is a tool designed to streamline and automate the process of managing and tracking request for GitHub. This includes the adding, removal or editing of members. +The GitHub request application is a tool designed to streamline and automate the process of managing and tracking request for GitHub. This includes the adding, removal or editing of members. Further documentation can be found [here](./docs/). ## Overview diff --git a/docs/Project Structure and Code Style.md b/docs/Project Structure and Code Style.md index 937bd12..9f65d6d 100644 --- a/docs/Project Structure and Code Style.md +++ b/docs/Project Structure and Code Style.md @@ -6,9 +6,9 @@ Github Request App is a simple app with a few endpoints, each endpoint represent All information filled by the user will be saved on a browser cookie which is configured to sign the cookie with the `COOKIE_SESSION_SECRET` environment variable. The payload stored in the cookie is also protected by the `crypto` module meaning the client side data is both encrypted and signed. Generally we will fetch information from the session, on the GET controller, to be used to populate data on the view. Instead the saving of user data into the session is done on the POST controller of each page. -It is important to remember that there is a mapping between the saved data on the session and the user data passed to the view for visualization. +It is important to remember that there is a mapping between the saved data on the session and the user data passed to the view for visualisation. -The compiled/transpiled project is copied into the dist folder used later to bootstrap the application from `dist/server.js`. All static files will be fetched from a CDN on aws cloudfront service. +The compiled/transpiled project is copied into the dist folder used later to bootstrap the application from `dist/server.js`. All static files will be fetched from a CDN on AWS CloudFront service. ## Files Structure @@ -44,9 +44,9 @@ In the model we define the interface, the data structure used to represent the d // Remove Member Page Model export const RemoveMemberKey = "remove-member"; -export const RemoveMemberKeys: string[] = ["github_handle", "description"]; +export const RemoveMemberKeys: (keyof RemoveMember)[] = ["github_handle", "description"]; -export interface Presenter { +export interface RemoveMember { github_handle?: string description?: string } @@ -64,7 +64,7 @@ export interface ApplicationData { ### View -We utilize both `Nunjucks` and `GDS` style/components. To streamline the construction of pages, we employ common components that are utilized across the UI, which are stored in an includes directory within the `view` directory. This directory contains all the useful shared chunks of HTML code. This approach ensures consistency in error messaging, input formats, and other aspects. +We utilise both `Nunjucks` and `GDS` style/components. To streamline the construction of pages, we employ common components that are utilised across the UI, which are stored in an includes directory within the `view` directory. This directory contains all the useful shared chunks of HTML code. This approach ensures consistency in error messaging, input formats, and other aspects. The data value is passed by the `GET` controller using the `res.render(templateUrl, {...data})` method. If the value is not set, the input field will remain empty. Additionally, if specificity is required, we can include a variable using the `set` command, as demonstrated in the example below. @@ -126,7 +126,7 @@ export const get = (req: Request, res: Response, next: NextFunction) => { and in the `post` method we save the user data every time that a page is submitted. ```js -// Post controller for the Presenter page +// Post controller for the Remove Member page export const post = async (req: Request, res: Response, next: NextFunction) => { try { ... @@ -177,7 +177,7 @@ export default removeMemberRouter; ## Validation -In each `POST` endpoints for every page we have a sets of middlewares used to validate each fields submitted by the user, if one of the validation middlewares fails the `validationResult` [here](https://github.com/cabinetoffice/github-requests-app/blob/c1923314f23897a809624a8ec208648cab228b4e/src/middleware/validation.middleware.ts#L7) will extracts the validation errors from a request (`req` object) and it will be formatted as an `errors` object [here](https://github.com/cabinetoffice/github-requests-app/blob/c1923314f23897a809624a8ec208648cab228b4e/src/middleware/validation.middleware.ts#L27) and it will be passed to the render page for the correct error visualization. +In each `POST` endpoints for every page we have a sets of middlewares used to validate each fields submitted by the user, if one of the validation middlewares fails the `validationResult` [here](https://github.com/cabinetoffice/github-requests-app/blob/c1923314f23897a809624a8ec208648cab228b4e/src/middleware/validation.middleware.ts#L7) will extracts the validation errors from a request (`req` object) and it will be formatted as an `errors` object [here](https://github.com/cabinetoffice/github-requests-app/blob/c1923314f23897a809624a8ec208648cab228b4e/src/middleware/validation.middleware.ts#L27) and it will be passed to the render page for the correct error visualisation. ```js // Middlewares validation checks for the remove-member page From f72605886b086f1ac41f4638e50dba6435fb198d Mon Sep 17 00:00:00 2001 From: Harley Harris Date: Wed, 21 Feb 2024 10:44:08 +0000 Subject: [PATCH 07/14] add text variable to github-handle component --- src/views/add-member.html | 1 + src/views/include/inputs/github-handle.html | 2 +- src/views/member-request.html | 1 + src/views/remove-member.html | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/views/add-member.html b/src/views/add-member.html index b49a082..a3e97c8 100644 --- a/src/views/add-member.html +++ b/src/views/add-member.html @@ -41,6 +41,7 @@

Add a GitHub Member

value: last_name }) }} + {% set githubHandleText = "GitHub Handle" %} {% include "include/inputs/github-handle.html" %} {{ govukInput({ diff --git a/src/views/include/inputs/github-handle.html b/src/views/include/inputs/github-handle.html index 304be1f..d5e436f 100644 --- a/src/views/include/inputs/github-handle.html +++ b/src/views/include/inputs/github-handle.html @@ -1,7 +1,7 @@ {{ govukInput({ errorMessage: errors.github_handle if errors, label: { - text: "GitHub Handle", + text: githubHandleText, classes: "govuk-label--m" }, classes: "govuk-input--width-10", diff --git a/src/views/member-request.html b/src/views/member-request.html index 21d021f..fcf4935 100644 --- a/src/views/member-request.html +++ b/src/views/member-request.html @@ -17,6 +17,7 @@

Additional Member Requests

+ {% set githubHandleText = "GitHub Handle" %} {% include "include/inputs/github-handle.html" %} {% set descriptionText = "Description - Permission changes (required)" %} diff --git a/src/views/remove-member.html b/src/views/remove-member.html index c4bacc8..153e9b2 100644 --- a/src/views/remove-member.html +++ b/src/views/remove-member.html @@ -17,6 +17,7 @@

Remove a GitHub Member

+ {% set githubHandleText = "GitHub Handle" %} {% include "include/inputs/github-handle.html" %} {% set descriptionText = "Description (optional)" %} From b6f49f008058b6bfa419eb20106885d469cde455 Mon Sep 17 00:00:00 2001 From: Harley Harris Date: Wed, 21 Feb 2024 14:15:51 +0000 Subject: [PATCH 08/14] update add-team-member to use github handle component --- src/controller/add-team-member.controller.ts | 4 ++-- src/validation/add-team-member.validation.ts | 7 ++----- src/validation/error.messages.ts | 1 - src/views/add-team-member.html | 13 ++----------- test/integration/routes/add-team-member.spec.ts | 4 ++-- test/mock/data.ts | 2 +- 6 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/controller/add-team-member.controller.ts b/src/controller/add-team-member.controller.ts index a6c99b2..3391e32 100644 --- a/src/controller/add-team-member.controller.ts +++ b/src/controller/add-team-member.controller.ts @@ -9,11 +9,11 @@ export const get = (_req: Request, res: Response) => { export const post = (req: Request, res: Response) => { const teamName = req.body.team_name; - const teamMemberGithubHandle = req.body.team_member_github_handle; + const githubHandle = req.body.github_handle; // validation middleware and data assignment to be implemented - log.info(`Team Name: ${teamName}, Team Member GitHub Handle: ${teamMemberGithubHandle}`); + log.info(`Team Name: ${teamName}, Team Member GitHub Handle: ${githubHandle}`); return res.redirect(config.HOME); }; diff --git a/src/validation/add-team-member.validation.ts b/src/validation/add-team-member.validation.ts index b983d07..ce6a45e 100644 --- a/src/validation/add-team-member.validation.ts +++ b/src/validation/add-team-member.validation.ts @@ -1,9 +1,6 @@ -import { body } from 'express-validator'; - -import { ErrorMessages } from './error.messages'; import { teamNameValidation } from './fields/team-name.validation'; +import { githubHandleValidation } from './fields/github-handle.validation'; export const addTeamMember = [ - ...teamNameValidation, - body('team_member_github_handle').not().isEmpty({ ignore_whitespace: true }).withMessage(ErrorMessages.TEAM_MEMBER_GIT_HANDLE) + ...teamNameValidation, ...githubHandleValidation ]; diff --git a/src/validation/error.messages.ts b/src/validation/error.messages.ts index 6bbd9a7..531a0d0 100644 --- a/src/validation/error.messages.ts +++ b/src/validation/error.messages.ts @@ -12,5 +12,4 @@ export enum ErrorMessages { VISIBILITY = 'Select a visibility option', TEAM_MAINTAINER_GITHUB_HANDLE = 'Enter the team maintainer GitHub handle', TEAM_NAME = 'Enter the team name', - TEAM_MEMBER_GIT_HANDLE = 'Enter the username of the member GitHub handle (aka GitHub account)' } diff --git a/src/views/add-team-member.html b/src/views/add-team-member.html index d68f3d2..37d335d 100644 --- a/src/views/add-team-member.html +++ b/src/views/add-team-member.html @@ -19,17 +19,8 @@

Add a GitHub member to a team

{% include "include/inputs/team-name.html" %} - {{ govukInput({ - errorMessage: errors.team_member_github_handle if errors, - label: { - text: "Member GitHub handle", - classes: "govuk-label--m" - }, - classes: "govuk-input--width-10", - id: "team_member_github_handle", - name: "team_member_github_handle", - value: team_member_github_handle - }) }} + {% set githubHandleText = "Member GitHub handle" %} + {% include "include/inputs/github-handle.html" %} {% include "include/save-button.html" %} diff --git a/test/integration/routes/add-team-member.spec.ts b/test/integration/routes/add-team-member.spec.ts index f986cc6..68ffc9c 100644 --- a/test/integration/routes/add-team-member.spec.ts +++ b/test/integration/routes/add-team-member.spec.ts @@ -50,12 +50,12 @@ describe('add-team-member endpoint integration tests', () => { test('Should render the same page with error messages after POST request', async () => { const res = await request(app).post(config.ADD_TEAM_MEMBER_URL).send({ team_name: '', - team_member_github_handle: '' + github_handle: '' }); expect(res.status).toEqual(200); expect(res.text).toContain(ErrorMessages.TEAM_NAME); - expect(res.text).toContain(ErrorMessages.TEAM_MEMBER_GIT_HANDLE); + expect(res.text).toContain(ErrorMessages.GIT_HANDLE); expect(res.text).toContain(MOCK_GET_ADD_TEAM_MEMBER_RESPONSE); expect(mockedLogger).toHaveBeenCalledTimes(1); expect(mockedAuth).toHaveBeenCalledTimes(1); diff --git a/test/mock/data.ts b/test/mock/data.ts index 94630d5..6bc7c0e 100644 --- a/test/mock/data.ts +++ b/test/mock/data.ts @@ -9,7 +9,7 @@ export const MOCK_POST_REMOVE_MEMBER = { github_handle: 'example' }; export const MOCK_POST_TEAM_REQUEST = { team_name: 'team1' }; export const MOCK_POST_MEMBER_REQUEST = { github_handle: 'example' }; -export const MOCK_POST_ADD_TEAM_MEMBER = { team_name: 'team1', team_member_github_handle: 'joe' }; +export const MOCK_POST_ADD_TEAM_MEMBER = { team_name: 'team1', github_handle: 'joe' }; export const MOCK_POST_REPO_REQUEST = { repo_name: 'repo1' }; export const MOCK_CORS_VALUE = { From 9046652180735cb2c3059367ee239f6667d48428 Mon Sep 17 00:00:00 2001 From: Harley Harris Date: Wed, 21 Feb 2024 14:23:01 +0000 Subject: [PATCH 09/14] update add-team to use github handle component --- src/controller/add-team.controller.ts | 4 ++-- src/validation/add-team.validation.ts | 8 ++------ src/validation/error.messages.ts | 1 - src/views/add-team.html | 13 ++----------- test/integration/routes/add-team.spec.ts | 4 ++-- test/mock/data.ts | 2 +- 6 files changed, 9 insertions(+), 23 deletions(-) diff --git a/src/controller/add-team.controller.ts b/src/controller/add-team.controller.ts index 4a5d57d..6982800 100644 --- a/src/controller/add-team.controller.ts +++ b/src/controller/add-team.controller.ts @@ -9,11 +9,11 @@ export const get = (_req: Request, res: Response) => { export const post = (req: Request, res: Response) => { const teamName = req.body.team_name; - const teamMaintainerGithubHandle = req.body.team_maintainer_github_handle; + const githubHandle = req.body.github_handle; // validation middleware and data assignment to be implemented - log.info(`Team Name: ${teamName}, Team Maintainer GitHub Handle: ${teamMaintainerGithubHandle}`); + log.info(`Team Name: ${teamName}, Team Maintainer GitHub Handle: ${githubHandle}`); return res.redirect(config.HOME); }; diff --git a/src/validation/add-team.validation.ts b/src/validation/add-team.validation.ts index adfc0a0..d45f12f 100644 --- a/src/validation/add-team.validation.ts +++ b/src/validation/add-team.validation.ts @@ -1,11 +1,7 @@ -import { body } from 'express-validator'; - -import { ErrorMessages } from './error.messages'; import { descriptionValidation } from './fields/description.validation'; +import { githubHandleValidation } from './fields/github-handle.validation'; import { teamNameValidation } from './fields/team-name.validation'; export const addTeam = [ - ...teamNameValidation, - body('team_maintainer_github_handle').not().isEmpty({ ignore_whitespace: true }).withMessage(ErrorMessages.TEAM_MAINTAINER_GITHUB_HANDLE), - ...descriptionValidation + ...teamNameValidation, githubHandleValidation, ...descriptionValidation ]; diff --git a/src/validation/error.messages.ts b/src/validation/error.messages.ts index 531a0d0..971a555 100644 --- a/src/validation/error.messages.ts +++ b/src/validation/error.messages.ts @@ -10,6 +10,5 @@ export enum ErrorMessages { CONTRACT_DATE_TIME= 'The contract end date cannot be in the past', REPO_NAME = 'Enter the repository name', VISIBILITY = 'Select a visibility option', - TEAM_MAINTAINER_GITHUB_HANDLE = 'Enter the team maintainer GitHub handle', TEAM_NAME = 'Enter the team name', } diff --git a/src/views/add-team.html b/src/views/add-team.html index 13cd7b4..0a6e798 100644 --- a/src/views/add-team.html +++ b/src/views/add-team.html @@ -19,17 +19,8 @@

Add a GitHub Team

{% include "include/inputs/team-name.html" %} - {{ govukInput({ - errorMessage: errors.team_maintainer_github_handle if errors, - label: { - text: "Team maintainer GitHub handle", - classes: "govuk-label--m" - }, - classes: "govuk-input--width-10", - id: "team_maintainer_github_handle", - name: "team_maintainer_github_handle", - value: team_maintainer_github_handle - }) }} + {% set githubHandleText = "Team maintainer GitHub handle" %} + {% include "include/inputs/github-handle.html" %} {% set descriptionText = "Description (optional)" %} {% set descriptionHint = "Explain the reason for adding this team." %} diff --git a/test/integration/routes/add-team.spec.ts b/test/integration/routes/add-team.spec.ts index 87d6d1a..c6ab9d9 100644 --- a/test/integration/routes/add-team.spec.ts +++ b/test/integration/routes/add-team.spec.ts @@ -49,12 +49,12 @@ describe('add-team endpoint integration tests', () => { test('Should render the same page with error messages after POST request', async () => { const res = await request(app).post(config.ADD_TEAM_URL).send({ repo_name: '', - team_maintainer_github_handle: '', + github_handle: '', }); expect(res.status).toEqual(200); expect(res.text).toContain(ErrorMessages.TEAM_NAME); - expect(res.text).toContain(ErrorMessages.TEAM_MAINTAINER_GITHUB_HANDLE); + expect(res.text).toContain(ErrorMessages.GIT_HANDLE); expect(res.text).toContain(MOCK_GET_ADD_TEAM_RESPONSE); expect(mockedLogger).toHaveBeenCalledTimes(1); expect(mockedAuth).toHaveBeenCalledTimes(1); diff --git a/test/mock/data.ts b/test/mock/data.ts index 6bc7c0e..15d6df9 100644 --- a/test/mock/data.ts +++ b/test/mock/data.ts @@ -4,7 +4,7 @@ export const GET_REQUEST_MOCK = { method: 'GET', path: '/test' }; export const MOCK_POST_ADD_REPO = { repo_name: 'repo1', visibility: 'public' }; export const MOCK_POST_ADD_MEMBER = { first_name: 'example', last_name: 'example', github_handle: 'example', email_address: 'email@hotmail.com', description: 'description', contract_type: 'permanent' }; -export const MOCK_POST_ADD_TEAM = { team_name: 'team1', team_maintainer_github_handle: 'bob' }; +export const MOCK_POST_ADD_TEAM = { team_name: 'team1', github_handle: 'bob' }; export const MOCK_POST_REMOVE_MEMBER = { github_handle: 'example' }; export const MOCK_POST_TEAM_REQUEST = { team_name: 'team1' }; export const MOCK_POST_MEMBER_REQUEST = { github_handle: 'example' }; From 55c5ad89098d4eda1958831338eb298fbe120387 Mon Sep 17 00:00:00 2001 From: Harley Harris Date: Wed, 21 Feb 2024 14:28:01 +0000 Subject: [PATCH 10/14] depersonalise git handle error message --- src/validation/error.messages.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validation/error.messages.ts b/src/validation/error.messages.ts index 971a555..84c5f78 100644 --- a/src/validation/error.messages.ts +++ b/src/validation/error.messages.ts @@ -1,5 +1,5 @@ export enum ErrorMessages { - GIT_HANDLE = 'Enter the username of your GitHub handle (aka GitHub account)', + GIT_HANDLE = 'Enter the username of the GitHub handle (aka GitHub account)', DESCRIPTION_LENGTH = 'Description must be 1000 characters or less', FIRST_NAME = 'Enter the first name of the GitHub account holder', LAST_NAME = 'Enter the last name of the GitHub account holder', From 458cb29078ef2bfd741c16650fa7102c24ca2ce5 Mon Sep 17 00:00:00 2001 From: DanielMurray97 Date: Thu, 22 Feb 2024 09:05:09 +0000 Subject: [PATCH 11/14] add general mock express app to data mocks and integrate into helmet and cors unit test --- test/mock/data.ts | 5 +++++ test/unit/config/cors.spec.ts | 10 +++------- test/unit/config/helmet.spec.ts | 10 +++------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/test/mock/data.ts b/test/mock/data.ts index 94630d5..cf5170c 100644 --- a/test/mock/data.ts +++ b/test/mock/data.ts @@ -1,4 +1,5 @@ import * as config from '../../src/config'; +import express from 'express'; export const GET_REQUEST_MOCK = { method: 'GET', path: '/test' }; @@ -42,3 +43,7 @@ export const MOCK_HELMET_VALUE = { reportOnly: false } }; + +export const MOCK_EXPRESS_APP = { + use: jest.fn() +} as unknown as express.Application; diff --git a/test/unit/config/cors.spec.ts b/test/unit/config/cors.spec.ts index 57caf5a..b87cf30 100644 --- a/test/unit/config/cors.spec.ts +++ b/test/unit/config/cors.spec.ts @@ -1,11 +1,10 @@ jest.mock('cors'); import { describe, expect, test, jest, afterEach } from '@jest/globals'; -import express from 'express'; import cors from 'cors'; import { configureCors } from '../../../src/config/cors'; -import { MOCK_CORS_VALUE } from '../../mock/data'; +import { MOCK_CORS_VALUE, MOCK_EXPRESS_APP } from '../../mock/data'; describe('CORS Config test suites', () => { afterEach(() => { @@ -14,14 +13,11 @@ describe('CORS Config test suites', () => { test('Should call cors method and express app.use method ', () => { const mockCors = cors as jest.Mock; - const mockApp = { - use: jest.fn() - } as unknown as express.Application; - configureCors(mockApp); + configureCors(MOCK_EXPRESS_APP); expect(mockCors).toHaveBeenCalledTimes(1); expect(mockCors).toHaveBeenCalledWith(MOCK_CORS_VALUE); - expect(mockApp.use).toHaveBeenCalled(); + expect(MOCK_EXPRESS_APP.use).toHaveBeenCalled(); }); }); diff --git a/test/unit/config/helmet.spec.ts b/test/unit/config/helmet.spec.ts index bf3a9aa..c254f2a 100644 --- a/test/unit/config/helmet.spec.ts +++ b/test/unit/config/helmet.spec.ts @@ -1,11 +1,10 @@ jest.mock('helmet'); import { describe, expect, test, jest, afterEach } from '@jest/globals'; -import express from 'express'; import helmet from 'helmet'; import { configureHelmet } from '../../../src/config/helmet'; -import { MOCK_HELMET_VALUE } from '../../mock/data'; +import { MOCK_HELMET_VALUE, MOCK_EXPRESS_APP } from '../../mock/data'; describe('Helmet Config test suites', () => { afterEach(() => { @@ -14,14 +13,11 @@ describe('Helmet Config test suites', () => { test('Should call helmet method and express app.use method', () => { const mockHelmet = helmet as unknown as jest.Mock; - const mockApp = { - use: jest.fn() - } as unknown as express.Application; - configureHelmet(mockApp); + configureHelmet(MOCK_EXPRESS_APP); expect(mockHelmet).toHaveBeenCalledTimes(1); expect(mockHelmet).toHaveBeenCalledWith(MOCK_HELMET_VALUE); - expect(mockApp.use).toHaveBeenCalled(); + expect(MOCK_EXPRESS_APP.use).toHaveBeenCalled(); }); }); From 6b5288270f124c4c3158dd707846985ac06209e7 Mon Sep 17 00:00:00 2001 From: DanielMurray97 Date: Thu, 22 Feb 2024 09:05:27 +0000 Subject: [PATCH 12/14] add nunjucks unit tests --- test/mock/text.mock.ts | 2 ++ test/unit/config/nunjucks.spec.ts | 46 +++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 test/unit/config/nunjucks.spec.ts diff --git a/test/mock/text.mock.ts b/test/mock/text.mock.ts index bff55b1..88d3e89 100644 --- a/test/mock/text.mock.ts +++ b/test/mock/text.mock.ts @@ -33,3 +33,5 @@ export const MOCK_SERVER_ERROR = 'Pipe 3000 requires elevated privileges'; export const MOCK_ERROR_MESSAGE = 'Something has went wrong'; export const MOCK_WRONG_URL = '/infooo'; export const MOCK_REDIRECT_MESSAGE = 'Found. Redirecting to home'; + +export const MOCK_VIEWS_PATH = '/path/to/views'; diff --git a/test/unit/config/nunjucks.spec.ts b/test/unit/config/nunjucks.spec.ts new file mode 100644 index 0000000..7caa1b8 --- /dev/null +++ b/test/unit/config/nunjucks.spec.ts @@ -0,0 +1,46 @@ +jest.mock('nunjucks'); +jest.mock('../../../src/utils/logger'); + +import { describe, expect, test, jest } from '@jest/globals'; + +import * as nunjucks from 'nunjucks'; + +import * as config from '../../../src/config'; +import { log } from '../../../src/utils/logger'; +import { configureNunjucks } from '../../../src/config/nunjucks'; +import { MOCK_VIEWS_PATH } from '../../mock/text.mock'; +import { MOCK_EXPRESS_APP } from '../../mock/data'; + +describe('Nunjucks Configuration test suites', () => { + + afterEach(() => { + jest.resetAllMocks(); + }); + + test('Should configure Nunjucks with the correct parameters and global variables', () => { + + const mockNunjucksEnv = { + addGlobal: jest.fn() + }; + + (nunjucks.configure as jest.Mock).mockReturnValue(mockNunjucksEnv); + + const mockLogInfo = log.info as jest.Mock; + + configureNunjucks(MOCK_EXPRESS_APP, MOCK_VIEWS_PATH); + + expect(nunjucks.configure).toHaveBeenCalledWith( + [MOCK_VIEWS_PATH, 'node_modules/govuk-frontend', 'node_modules/govuk-frontend/components'], + { + autoescape: true, + express: MOCK_EXPRESS_APP + } + ); + + expect(mockNunjucksEnv.addGlobal).toHaveBeenCalledWith('CDN_HOST', config.CDN_HOST); + expect(mockNunjucksEnv.addGlobal).toHaveBeenCalledWith('SERVICE_URL', config.SERVICE_URL); + expect(mockNunjucksEnv.addGlobal).toHaveBeenCalledWith('SERVICE_NAME', config.SERVICE_NAME); + + expect(mockLogInfo).toHaveBeenCalledWith(expect.stringContaining(MOCK_VIEWS_PATH)); + }); +}); From dbf937f94bc2a320a7647b75f6fac333d1f3f0a3 Mon Sep 17 00:00:00 2001 From: DanielMurray97 Date: Thu, 22 Feb 2024 10:20:13 +0000 Subject: [PATCH 13/14] assess pr comments --- test/unit/config/nunjucks.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/unit/config/nunjucks.spec.ts b/test/unit/config/nunjucks.spec.ts index 7caa1b8..1e579ab 100644 --- a/test/unit/config/nunjucks.spec.ts +++ b/test/unit/config/nunjucks.spec.ts @@ -23,7 +23,9 @@ describe('Nunjucks Configuration test suites', () => { addGlobal: jest.fn() }; - (nunjucks.configure as jest.Mock).mockReturnValue(mockNunjucksEnv); + const nunjucksConfigureMock = nunjucks.configure as jest.Mock; + + nunjucksConfigureMock.mockReturnValue(mockNunjucksEnv); const mockLogInfo = log.info as jest.Mock; From 390c817d0d728aae7343cd94187690e34aefea41 Mon Sep 17 00:00:00 2001 From: Harley Harris Date: Thu, 22 Feb 2024 10:21:07 +0000 Subject: [PATCH 14/14] add missing spread to add-team validation --- src/validation/add-team.validation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validation/add-team.validation.ts b/src/validation/add-team.validation.ts index d45f12f..8c6023e 100644 --- a/src/validation/add-team.validation.ts +++ b/src/validation/add-team.validation.ts @@ -3,5 +3,5 @@ import { githubHandleValidation } from './fields/github-handle.validation'; import { teamNameValidation } from './fields/team-name.validation'; export const addTeam = [ - ...teamNameValidation, githubHandleValidation, ...descriptionValidation + ...teamNameValidation, ...githubHandleValidation, ...descriptionValidation ];