From 684766680b6300daaec17a8f1b9248efae25c079 Mon Sep 17 00:00:00 2001 From: ttingle-ch Date: Thu, 24 Oct 2024 12:27:07 +0100 Subject: [PATCH 1/3] Redirect user directly to payment if they resume a transaction that is closed pending payment --- src/config/index.ts | 4 +- .../common/resumeJourneyController.ts | 44 ++++++++++++++++--- .../transactions/transaction_service.ts | 27 ++++++++++++ src/utils/properties.ts | 2 + test/mocks/payment_mock.ts | 11 +++++ test/mocks/transaction_mock.ts | 15 +++++++ .../common/resumeJourneyController.test.ts | 39 +++++++++++++++- .../transaction/transaction_service.test.ts | 42 +++++++++++++++++- 8 files changed, 173 insertions(+), 11 deletions(-) diff --git a/src/config/index.ts b/src/config/index.ts index 7af15567..fd16a81a 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -30,8 +30,10 @@ export const ACSP_SERVICE_BASE = `${ACSP_SERVICE_HOST}/acsp-api`; export const CREATE_DESCRIPTION = "Application to register a Companies House authorised agent"; export const REFERENCE = "ACSP Registration"; export const transactionStatuses = { - CLOSED: "closed" + CLOSED: "closed", + CLOSED_PENDING_PAYMENT: "closed pending payment" }; +export const PAYMENTS = "/payments"; export const ACCESSIBILITY_STATEMENT = `${BASE_COMMON_URL}/accessibility-statement/accessibility-statement`; export const STOP_NOT_RELEVANT_OFFICER = `${BASE_SOLE_TRADER_URL}/stop-not-relevant-officer/stop-not-relevant-officer`; export const WHAT_IS_YOUR_NAME = `${BASE_COMMON_URL}/name/capture-name`; diff --git a/src/controllers/features/common/resumeJourneyController.ts b/src/controllers/features/common/resumeJourneyController.ts index 0796ffc0..40a27f4d 100644 --- a/src/controllers/features/common/resumeJourneyController.ts +++ b/src/controllers/features/common/resumeJourneyController.ts @@ -1,13 +1,21 @@ -import { Request, Response } from "express"; -import { BASE_URL, TYPE_OF_BUSINESS } from "../../../types/pageURL"; -import { selectLang, addLangToUrl } from "../../../utils/localise"; +import { ApiResponse } from "@companieshouse/api-sdk-node/dist/services/resource"; +import { Transaction } from "@companieshouse/api-sdk-node/dist/services/transaction/types"; +import { Payment } from "@companieshouse/api-sdk-node/dist/services/payment"; import { Session } from "@companieshouse/node-session-handler"; -import { SUBMISSION_ID } from "../../../common/__utils/constants"; +import { Request, Response } from "express"; +import { BASE_URL, RESUME_JOURNEY, TYPE_OF_BUSINESS } from "../../../types/pageURL"; +import { selectLang, addLangToUrl, getLocalesService } from "../../../utils/localise"; import logger from "../../../utils/logger"; +import { NO_PAYMENT_RESOURCE_ERROR, SUBMISSION_ID } from "../../../common/__utils/constants"; +import { getTransactionById } from "../../../services/transactions/transaction_service"; +import { ErrorService } from "../../../services/errorService"; +import { startPaymentsSession } from "../../../services/paymentService"; +import { PAYMENTS_API_URL } from "../../../utils/properties"; +import { PAYMENTS, transactionStatuses } from "../../../config"; export const get = async (req: Request, res: Response) => { const lang = selectLang(req.query.lang); - const transactionId = req.query.transactionId; + const transactionId = req.query.transactionId as string; const acspId = req.query.acspId; const session: Session = req.session as any as Session; const infoMsg = `Transaction ID: ${transactionId}, Acsp Id ID: ${acspId}`; @@ -16,6 +24,28 @@ export const get = async (req: Request, res: Response) => { // eslint-disable-next-line camelcase res.locals.userId = session?.data?.signin_info?.user_profile?.id; res.locals.applicationId = acspId; - const nextPageUrl = addLangToUrl(BASE_URL + TYPE_OF_BUSINESS, lang); - res.redirect(nextPageUrl); + + try { + const transaction: Transaction = await getTransactionById(session, transactionId); + + if (transaction.status === transactionStatuses.CLOSED_PENDING_PAYMENT) { + const paymentUrl = PAYMENTS_API_URL + PAYMENTS; + const paymentResponse: ApiResponse = await startPaymentsSession(session, paymentUrl, + transactionId); + + if (!paymentResponse.resource) { + throw new Error(NO_PAYMENT_RESOURCE_ERROR); + } + + res.redirect(paymentResponse.resource.links.journey); + + } else { + res.redirect(addLangToUrl(BASE_URL + TYPE_OF_BUSINESS, lang)); + } + + } catch (err) { + logger.error("Error resuming journey " + JSON.stringify(err)); + const error = new ErrorService(); + error.renderErrorPage(res, getLocalesService(), lang, BASE_URL + RESUME_JOURNEY); + } }; diff --git a/src/services/transactions/transaction_service.ts b/src/services/transactions/transaction_service.ts index dbd2325f..8ca48f94 100644 --- a/src/services/transactions/transaction_service.ts +++ b/src/services/transactions/transaction_service.ts @@ -105,3 +105,30 @@ export const getSavedApplication = async (session: Session, acspApplicationId: s return Promise.resolve(sdkResponse as Resource); }; + +// get transaction from transaction id +export const getTransactionById = async (session: Session, transactionId: string): Promise => { + const apiClient: ApiClient = createPublicOAuthApiClient(session); + const sdkResponse = await apiClient.transaction.getTransaction(transactionId); + + if (!sdkResponse) { + logger.error(`Transaction API GET request returned no response for transaction id: ${transactionId}`); + return Promise.reject(sdkResponse); + } + + if (!sdkResponse.httpStatusCode || sdkResponse.httpStatusCode >= 400) { + logger.error(`Http status code ${sdkResponse.httpStatusCode} - Failed to GET transaction with id: ${transactionId}`); + return Promise.reject(sdkResponse); + } + + const castedSdkResponse = sdkResponse as Resource; + + if (!castedSdkResponse.resource) { + logger.error(`Transaction API GET request returned no resource`); + return Promise.reject(sdkResponse); + } + + logger.debug(`Received transaction ${JSON.stringify(sdkResponse)}`); + + return Promise.resolve(castedSdkResponse.resource); +}; diff --git a/src/utils/properties.ts b/src/utils/properties.ts index 69e0b34e..eab7ef97 100644 --- a/src/utils/properties.ts +++ b/src/utils/properties.ts @@ -16,6 +16,8 @@ export const POSTCODE_ADDRESSES_LOOKUP_URL = getEnvironmentVariable("POSTCODE_AD export const API_URL = getEnvironmentValue("API_URL", "http://api.chs.local:4001"); +export const PAYMENTS_API_URL = getEnvironmentValue("PAYMENTS_API_URL", "http://api.chs.local:4001"); + export const CHS_API_KEY = getEnvironmentValue("CHS_API_KEY", "chs.api.key"); export const CHS_URL = getEnvironmentValue("CHS_URL", "http://chs.local"); diff --git a/test/mocks/payment_mock.ts b/test/mocks/payment_mock.ts index 69f340f9..6dbc1a7e 100644 --- a/test/mocks/payment_mock.ts +++ b/test/mocks/payment_mock.ts @@ -1,4 +1,5 @@ import { Payment } from "@companieshouse/api-sdk-node/dist/services/payment"; +import { ApiResponse } from "@companieshouse/api-sdk-node/dist/services/resource"; export const PAYMENT_JOURNEY_URL = "journey"; @@ -24,3 +25,13 @@ export const dummyPayment = { reference: "3432", status: "paid" } as Payment; + +const dummyHeaders = { + header1: "45435435" +}; + +export const dummyPaymentResponse: ApiResponse = { + headers: dummyHeaders, + httpStatusCode: 200, + resource: dummyPayment +}; diff --git a/test/mocks/transaction_mock.ts b/test/mocks/transaction_mock.ts index 25fb655e..ee3b5f6a 100644 --- a/test/mocks/transaction_mock.ts +++ b/test/mocks/transaction_mock.ts @@ -18,6 +18,21 @@ export const validTransaction: Transaction = { }, description: "Mandatory transaction description" }; + +export const mockOpenTransaction: Transaction = { + id: "119709-207817-181835", + status: "open", + reference: REFERENCE, + description: "Mandatory transaction description" +}; + +export const mockClosedPendingPaymentTransaction: Transaction = { + id: "119709-207817-181835", + status: "closed pending payment", + reference: REFERENCE, + description: "Mandatory transaction description" +}; + export const validTransactionSDKResource: Resource = { httpStatusCode: StatusCodes.OK, resource: validTransaction diff --git a/test/src/controllers/common/resumeJourneyController.test.ts b/test/src/controllers/common/resumeJourneyController.test.ts index c1d5d1cc..f220112f 100644 --- a/test/src/controllers/common/resumeJourneyController.test.ts +++ b/test/src/controllers/common/resumeJourneyController.test.ts @@ -2,17 +2,54 @@ import mocks from "../../../mocks/all_middleware_mock"; import supertest from "supertest"; import app from "../../../../src/app"; import { BASE_URL, TYPE_OF_BUSINESS } from "../../../../src/types/pageURL"; +import { getTransactionById } from "../../../../src/services/transactions/transaction_service"; +import { dummyPaymentResponse, PAYMENT_JOURNEY_URL } from "../../../mocks/payment_mock"; +import { startPaymentsSession } from "../../../../src/services/paymentService"; +import { mockClosedPendingPaymentTransaction, mockOpenTransaction } from "../../../mocks/transaction_mock"; jest.mock("@companieshouse/api-sdk-node"); jest.mock("../../../../src/services/acspRegistrationService"); +jest.mock("../../../../src/services/transactions/transaction_service"); +jest.mock("../../../../src/services/paymentService"); const router = supertest(app); +const mockGetTransaction = getTransactionById as jest.Mock; +const mockStartPaymentsSession = startPaymentsSession as jest.Mock; + describe("GET resume journey", () => { - it("should return status 302", async () => { + it("should return status 302 and redirect to type of business screen if transaction is open", async () => { + mockGetTransaction.mockResolvedValueOnce(mockOpenTransaction); const res = await router.get(BASE_URL + "/resume?transactionId=119709-207817-181835&acspId=Y2VkZWVlMzhlZWFjY2M4MzQ3MT"); expect(res.status).toBe(302); expect(mocks.mockSessionMiddleware).toHaveBeenCalled(); expect(mocks.mockAuthenticationMiddleware).toHaveBeenCalled(); expect(res.header.location).toBe(BASE_URL + TYPE_OF_BUSINESS + "?lang=en"); }); + + it("should return status 302 and redirect to payment if transaction is closed pending payment", async () => { + mockGetTransaction.mockResolvedValueOnce(mockClosedPendingPaymentTransaction); + mockStartPaymentsSession.mockResolvedValueOnce(dummyPaymentResponse); + const res = await router.get(BASE_URL + "/resume?transactionId=119709-207817-181835&acspId=Y2VkZWVlMzhlZWFjY2M4MzQ3MT"); + expect(res.status).toBe(302); + expect(mocks.mockSessionMiddleware).toHaveBeenCalled(); + expect(mocks.mockAuthenticationMiddleware).toHaveBeenCalled(); + expect(res.header.location).toBe(PAYMENT_JOURNEY_URL); + }); + + // Test for calling getTransaction failure. + it("should return status 500 after calling getTransaction and failing", async () => { + mockGetTransaction.mockRejectedValueOnce(new Error("Error getting transaction")); + const res = await router.get(BASE_URL + "/resume?transactionId=119709-207817-181835&acspId=Y2VkZWVlMzhlZWFjY2M4MzQ3MT"); + expect(res.status).toBe(500); + expect(res.text).toContain("Sorry we are experiencing technical difficulties"); + }); + + // Test for calling startPaymentsSession failure. + it("should return status 500 after calling startPaymentsSession and failing", async () => { + mockGetTransaction.mockResolvedValueOnce(mockClosedPendingPaymentTransaction); + mockStartPaymentsSession.mockRejectedValueOnce(new Error("Error starting payment session")); + const res = await router.get(BASE_URL + "/resume?transactionId=119709-207817-181835&acspId=Y2VkZWVlMzhlZWFjY2M4MzQ3MT"); + expect(res.status).toBe(500); + expect(res.text).toContain("Sorry we are experiencing technical difficulties"); + }); }); diff --git a/test/src/services/transaction/transaction_service.test.ts b/test/src/services/transaction/transaction_service.test.ts index 18ea860c..a16140f4 100644 --- a/test/src/services/transaction/transaction_service.test.ts +++ b/test/src/services/transaction/transaction_service.test.ts @@ -5,14 +5,15 @@ import { closeTransaction, postTransaction, getSavedApplication, - putTransaction + putTransaction, + getTransactionById } from "../../../../src/services/transactions/transaction_service"; import { Transaction } from "@companieshouse/api-sdk-node/dist/services/transaction/types"; import { ApiResponse } from "@companieshouse/api-sdk-node/dist/services/resource"; import { StatusCodes } from "http-status-codes"; import { CompanyProfile } from "@companieshouse/api-sdk-node/dist/services/company-profile/types"; import { REFERENCE } from "../../../../src/config"; -import { HttpResponse } from "@companieshouse/api-sdk-node/dist/http/http-client"; +import { validTransaction } from "../../../mocks/transaction_mock"; jest.mock("@companieshouse/api-sdk-node"); jest.mock("../../../../src/services/apiService"); @@ -226,4 +227,41 @@ describe("transaction service tests", () => { await expect(getSavedApplication(session, USER_ID)).rejects.toBe(mockFailedResponce); }); }); + + describe("getTransaction tests", () => { + it("should return resolved transaction for 200 status code", async () => { + const mockSuccessResponce = { + httpStatusCode: 200, + resource: validTransaction + }; + mockGetTransaction.mockResolvedValueOnce(mockSuccessResponce); + + const transactionResp = await getTransactionById(session, TRANSACTION_ID); + expect(transactionResp).toStrictEqual(validTransaction); + }); + + it("should throw an error when no transaction api response", async () => { + mockGetTransaction.mockResolvedValueOnce(undefined); + + await expect(getTransactionById(session, TRANSACTION_ID)).rejects.toBe(undefined); + }); + + it("should throw an error when status code is >=400", async () => { + const mockFailedResponce = { + httpStatusCode: 500, + resource: { items: [[Object]] } + }; + mockGetTransaction.mockResolvedValueOnce(mockFailedResponce); + + await expect(getTransactionById(session, TRANSACTION_ID)).rejects.toBe(mockFailedResponce); + }); + it("Should throw an error when no status code", async () => { + const mockFailedResponce = { + resource: { items: [[Object]] } + }; + mockGetTransaction.mockResolvedValueOnce(mockFailedResponce); + + await expect(getTransactionById(session, TRANSACTION_ID)).rejects.toBe(mockFailedResponce); + }); + }); }); From 4bdf9a512d9fa0493baacf716223db0e2647c138 Mon Sep 17 00:00:00 2001 From: ttingle-ch Date: Thu, 24 Oct 2024 12:51:44 +0100 Subject: [PATCH 2/3] Increasing test coverage --- test/mocks/payment_mock.ts | 5 +++++ .../common/resumeJourneyController.test.ts | 11 ++++++++++- .../services/transaction/transaction_service.test.ts | 8 ++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/test/mocks/payment_mock.ts b/test/mocks/payment_mock.ts index 6dbc1a7e..9d5adb85 100644 --- a/test/mocks/payment_mock.ts +++ b/test/mocks/payment_mock.ts @@ -35,3 +35,8 @@ export const dummyPaymentResponse: ApiResponse = { httpStatusCode: 200, resource: dummyPayment }; + +export const dummyPaymentResponseNoResource: ApiResponse = { + headers: dummyHeaders, + httpStatusCode: 200 +}; diff --git a/test/src/controllers/common/resumeJourneyController.test.ts b/test/src/controllers/common/resumeJourneyController.test.ts index f220112f..f73d356a 100644 --- a/test/src/controllers/common/resumeJourneyController.test.ts +++ b/test/src/controllers/common/resumeJourneyController.test.ts @@ -3,7 +3,7 @@ import supertest from "supertest"; import app from "../../../../src/app"; import { BASE_URL, TYPE_OF_BUSINESS } from "../../../../src/types/pageURL"; import { getTransactionById } from "../../../../src/services/transactions/transaction_service"; -import { dummyPaymentResponse, PAYMENT_JOURNEY_URL } from "../../../mocks/payment_mock"; +import { dummyPaymentResponse, dummyPaymentResponseNoResource, PAYMENT_JOURNEY_URL } from "../../../mocks/payment_mock"; import { startPaymentsSession } from "../../../../src/services/paymentService"; import { mockClosedPendingPaymentTransaction, mockOpenTransaction } from "../../../mocks/transaction_mock"; @@ -52,4 +52,13 @@ describe("GET resume journey", () => { expect(res.status).toBe(500); expect(res.text).toContain("Sorry we are experiencing technical difficulties"); }); + + // Test for startPaymentsSession returning no resource. + it("should return status 500 after calling getTransaction and failing", async () => { + mockGetTransaction.mockResolvedValueOnce(mockClosedPendingPaymentTransaction); + mockStartPaymentsSession.mockResolvedValueOnce(dummyPaymentResponseNoResource); + const res = await router.get(BASE_URL + "/resume?transactionId=119709-207817-181835&acspId=Y2VkZWVlMzhlZWFjY2M4MzQ3MT"); + expect(res.status).toBe(500); + expect(res.text).toContain("Sorry we are experiencing technical difficulties"); + }); }); diff --git a/test/src/services/transaction/transaction_service.test.ts b/test/src/services/transaction/transaction_service.test.ts index a16140f4..20fb3dfe 100644 --- a/test/src/services/transaction/transaction_service.test.ts +++ b/test/src/services/transaction/transaction_service.test.ts @@ -263,5 +263,13 @@ describe("transaction service tests", () => { await expect(getTransactionById(session, TRANSACTION_ID)).rejects.toBe(mockFailedResponce); }); + it("Should throw an error when responce has no resource", async () => { + const mockResponceNoResource = { + httpStatusCode: 200 + }; + mockGetTransaction.mockResolvedValueOnce(mockResponceNoResource); + + await expect(getTransactionById(session, TRANSACTION_ID)).rejects.toBe(mockResponceNoResource); + }); }); }); From 6342b559d8586da64d114017bd599badf63ac4d5 Mon Sep 17 00:00:00 2001 From: ttingle-ch Date: Thu, 24 Oct 2024 12:54:55 +0100 Subject: [PATCH 3/3] updated default payment url --- src/utils/properties.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/properties.ts b/src/utils/properties.ts index eab7ef97..de6a4f4c 100644 --- a/src/utils/properties.ts +++ b/src/utils/properties.ts @@ -16,7 +16,7 @@ export const POSTCODE_ADDRESSES_LOOKUP_URL = getEnvironmentVariable("POSTCODE_AD export const API_URL = getEnvironmentValue("API_URL", "http://api.chs.local:4001"); -export const PAYMENTS_API_URL = getEnvironmentValue("PAYMENTS_API_URL", "http://api.chs.local:4001"); +export const PAYMENTS_API_URL = getEnvironmentValue("PAYMENTS_API_URL", "http://api-payments.chs.local:4001"); export const CHS_API_KEY = getEnvironmentValue("CHS_API_KEY", "chs.api.key");