Refactor SFD update handling in check-details.controller.js

src/server/common/helpers/logging/log-codes-definition.js

@@ -431,6 +431,11 @@ export const LogCodes = {
       messageFunc: (messageOptions) =>
         `Missing required configuration: ${messageOptions?.missing?.join(', ') || 'unknown'}`
     },
+    CONFIG_INVALID: {
+      level: 'error',
+      messageFunc: (messageOptions) =>
+        `Invalid configuration, key "${messageOptions.key}" is missing or invalid: ${messageOptions.value}`
+    },
     WHITELIST_CONFIG_INCOMPLETE: {
       level: 'error',
       messageFunc: (messageOptions) =>

src/server/details-page/check-details.controller.js

@@ -7,6 +7,7 @@ import {
 import { debug, log, LogCodes } from '../common/helpers/logging/log.js'
 import { mergeAdditionalAnswers } from '../common/helpers/state/additional-answers-helper.js'
 import { ComponentType } from '@defra/forms-model'
+import { config } from '~/src/config/config.js'
 
 const ERROR_TITLE = 'There is a problem'
 
@@ -80,14 +81,14 @@ export default class CheckDetailsController extends QuestionPageController {
   makeGetRouteHandler() {
     return async (request, context, h) => {
       const baseViewModel = super.getViewModel(request, context)
-      const config = this.model.def.metadata?.detailsPage
+      const detailsPageConfig = this.model.def.metadata?.detailsPage
 
-      if (!config) {
+      if (!detailsPageConfig) {
         return this.handleConfigError(baseViewModel, h, request)
       }
 
       try {
-        const { sections, mappedData } = await this.fetchAndProcessData(request, config)
+        const { sections, mappedData } = await this.fetchAndProcessData(request, detailsPageConfig)
         request.app.detailsPageData = mappedData
         return h.view(this.viewName, { ...baseViewModel, sections })
       } catch (error) {
@@ -103,16 +104,16 @@ export default class CheckDetailsController extends QuestionPageController {
       const { collection, viewName, model } = this
       const { state, evaluationState } = context
       const baseViewModel = super.getViewModel(request, context)
-      const config = this.model.def.metadata?.detailsPage
+      const detailsPageConfig = this.model.def.metadata?.detailsPage
 
-      if (!config) {
+      if (!detailsPageConfig) {
         return this.handleConfigError(baseViewModel, h, request)
       }
 
       if (context.errors) {
         const viewModel = this.getViewModel(request, context)
         viewModel.errors = collection.getViewErrors(viewModel.errors)
-        const { sections } = await this.fetchAndProcessData(request, config)
+        const { sections } = await this.fetchAndProcessData(request, detailsPageConfig)
         viewModel.sections = sections
 
         // Filter components based on their conditions using evaluated state
@@ -125,26 +126,31 @@ export default class CheckDetailsController extends QuestionPageController {
       await this.setState(request, state)
 
       if (confirmationValue === false) {
-        return h.redirect(`/${request.params.slug}/update-details`)
+        const redirectTarget = this.getSFDUpdateUrl(request)
+        if (config.get('externalLinks.sfd.enabled') && redirectTarget !== '') {
+          return h.redirect(redirectTarget)
+        } else {
+          return h.redirect(`/${request.params.slug}/update-details`)
+        }
       }
 
-      return this.handleDetailsConfirmed(request, context, config, h)
+      return this.handleDetailsConfirmed(request, context, detailsPageConfig, h)
     }
   }
 
   /**
    * Handle POST when user confirms details are correct
    * @param {AnyFormRequest} request
    * @param {object} context
-   * @param {object} config
+   * @param {object} detailsConfig
    * @param {ResponseToolkit} h
    * @returns {Promise<ResponseObject>}
    */
-  async handleDetailsConfirmed(request, context, config, h) {
+  async handleDetailsConfirmed(request, context, detailsConfig, h) {
     const baseViewModel = super.getViewModel(request, context)
 
     try {
-      const { mappedData } = await this.fetchAndProcessData(request, config)
+      const { mappedData } = await this.fetchAndProcessData(request, detailsConfig)
       await this.setState(
         request,
         mergeAdditionalAnswers(context.state, {
@@ -168,12 +174,12 @@ export default class CheckDetailsController extends QuestionPageController {
   /**
    * Fetch data from consolidated view and process it according to config
    * @param {AnyFormRequest} request
-   * @param {object} config - detailsPage configuration from form metadata
+   * @param {object} detailsConfig - detailsPage configuration from form metadata
    * @returns {Promise<{sections: Array, mappedData: object}>}
    */
-  async fetchAndProcessData(request, config) {
-    const toleratedPaths = config.toleratedFailurePaths ?? this.model.def.metadata?.toleratedFailurePaths
-    const query = buildGraphQLQuery(config.query, request)
+  async fetchAndProcessData(request, detailsConfig) {
+    const toleratedPaths = detailsConfig.toleratedFailurePaths ?? this.model.def.metadata?.toleratedFailurePaths
+    const query = buildGraphQLQuery(detailsConfig.query, request)
     const response = await executeConfigDrivenQuery(request, query, { toleratedPaths })
 
     if (response?.errors?.length > 0) {
@@ -197,8 +203,8 @@ export default class CheckDetailsController extends QuestionPageController {
       )
     }
 
-    const mappedData = mapResponse(config.responseMapping, response)
-    const sections = processSections(config.displaySections, mappedData, request)
+    const mappedData = mapResponse(detailsConfig.responseMapping, response)
+    const sections = processSections(detailsConfig.displaySections, mappedData, request)
     return { sections, mappedData }
   }
 
@@ -238,6 +244,28 @@ export default class CheckDetailsController extends QuestionPageController {
       }
     })
   }
+
+  /**
+   * Get the URL to update business details through SFD
+   * @param request
+   * @returns {string}
+   */
+  getSFDUpdateUrl(request) {
+    const { currentRelationshipId } = request.auth.credentials
+    const updateUrl = config.get('externalLinks.sfd.updateUrl')
+    if (!updateUrl) {
+      return ''
+    }
+
+    try {
+      const url = new URL(updateUrl)
+      url.searchParams.set('ssoOrgId', currentRelationshipId)
+      return url.toString()
+    } catch (error) {
+      debug(LogCodes.SYSTEM.CONFIG_MISSING, { key: 'externalLinks.sfd.updateUrl', value: updateUrl }, request)
+      return ''
+    }
+  }
 }
 
 /**

src/server/details-page/check-details.controller.test.js

@@ -8,6 +8,21 @@ import {
 } from '../common/services/consolidated-view/consolidated-view.service.js'
 import { debug, log, LogCodes } from '../common/helpers/logging/log.js'
 import { setupControllerMocks } from '~/src/__mocks__/controller-mocks.js'
+import { config } from '~/src/config/config.js'
+
+vi.mock('~/src/config/config.js', () => ({
+  config: {
+    get: vi.fn((key) => {
+      if (key === 'externalLinks.sfd.enabled') {
+        return false
+      }
+      if (key === 'externalLinks.sfd.updateUrl') {
+        return 'http://localhost:3000/sfd/update-sbi'
+      }
+      return undefined
+    })
+  }
+}))
 
 vi.mock('@defra/forms-model', () => ({
   ComponentType: {
@@ -129,6 +144,7 @@ describe('CheckDetailsController', () => {
     mockRequest = {
       app: {},
       path: '/test-form/check-details',
+      params: { slug: 'test-form' },
       auth: {
         isAuthenticated: true,
         credentials: {
@@ -220,6 +236,16 @@ describe('CheckDetailsController', () => {
         ])
       )
     })
+
+    it('should handle missing components in pageDef during patching', () => {
+      const pageDefWithoutComponents = {
+        path: '/check-details',
+        title: 'Check your details'
+      }
+      const ctrl = new CheckDetailsController(mockModel, pageDefWithoutComponents)
+      expect(ctrl.pageDef.components).toHaveLength(2)
+      expect(ctrl.pageDef.components[0].name).toBe('placeholder')
+    })
   })
 
   describe('makeGetRouteHandler', () => {
@@ -326,7 +352,13 @@ describe('CheckDetailsController', () => {
     })
 
     describe('confirmationValue === false (user says details are wrong)', () => {
-      it('should save state and redirect to /{slug}/update-details', async () => {
+      it('should save state and redirect to /{slug}/update-details when SFD is disabled', async () => {
+        vi.mocked(config.get).mockImplementation((key) => {
+          if (key === 'externalLinks.sfd.enabled') {
+            return false
+          }
+        })
+
         mockContext.payload = { detailsConfirmed: false }
         mockRequest.params = { slug: 'test-form' }
         mockH.redirect = vi.fn().mockReturnValue('redirected-to-update-details')
@@ -340,6 +372,49 @@ describe('CheckDetailsController', () => {
         expect(controller.proceed).not.toHaveBeenCalled()
         expect(result).toBe('redirected-to-update-details')
       })
+
+      it('should save state and redirect to /{slug}/update-details when SFD is enabled but no SFD URL has been set', async () => {
+        vi.mocked(config.get).mockImplementation((key) => {
+          if (key === 'externalLinks.sfd.enabled') {
+            return true
+          }
+          if (key === 'externalLinks.sfd.updateUrl') {
+            return undefined
+          }
+        })
+
+        mockContext.payload = { detailsConfirmed: false }
+        mockRequest.auth.credentials.currentRelationshipId = 'REL123'
+        mockH.redirect = vi.fn().mockReturnValue('redirected-to-update-details')
+
+        const handler = controller.makePostRouteHandler()
+        const result = await handler(mockRequest, mockContext, mockH)
+
+        expect(mockH.redirect).toHaveBeenCalledWith('/test-form/update-details')
+        expect(result).toBe('redirected-to-update-details')
+      })
+
+      it('should redirect to SFD update URL when SFD is enabled', async () => {
+        vi.mocked(config.get).mockImplementation((key) => {
+          if (key === 'externalLinks.sfd.enabled') {
+            return true
+          }
+          if (key === 'externalLinks.sfd.updateUrl') {
+            return 'http://localhost:3000/sfd/update-sbi'
+          }
+          return undefined
+        })
+
+        mockContext.payload = { detailsConfirmed: false }
+        mockRequest.auth.credentials.currentRelationshipId = 'REL123'
+        mockH.redirect = vi.fn().mockReturnValue('redirected-to-sfd')
+
+        const handler = controller.makePostRouteHandler()
+        const result = await handler(mockRequest, mockContext, mockH)
+
+        expect(mockH.redirect).toHaveBeenCalledWith('http://localhost:3000/sfd/update-sbi?ssoOrgId=REL123')
+        expect(result).toBe('redirected-to-sfd')
+      })
     })
 
     describe('confirmationValue is truthy (user confirms details)', () => {
@@ -517,6 +592,49 @@ describe('CheckDetailsController', () => {
       expect(mapResponse).toHaveBeenCalledWith(mockConfig.responseMapping, partialResponse)
       expect(result).toEqual({ sections: mockSections, mappedData: mockMappedData })
     })
+
+    it('should use toleratedFailurePaths from detailsConfig if present', async () => {
+      const configWithToleratedPaths = {
+        ...mockConfig,
+        toleratedFailurePaths: ['customPath']
+      }
+      vi.mocked(executeConfigDrivenQuery).mockResolvedValue({
+        errors: [{ message: 'Error' }]
+      })
+      vi.mocked(hasOnlyToleratedFailures).mockReturnValue(true)
+
+      await controller.fetchAndProcessData(mockRequest, configWithToleratedPaths)
+
+      expect(executeConfigDrivenQuery).toHaveBeenCalledWith(mockRequest, expect.any(String), {
+        toleratedPaths: ['customPath']
+      })
+      expect(hasOnlyToleratedFailures).toHaveBeenCalledWith(expect.any(Array), ['customPath'])
+    })
+  })
+
+  describe('getSFDUpdateUrl', () => {
+    beforeEach(() => {
+      vi.clearAllMocks()
+      mockRequest.auth.credentials.currentRelationshipId = 'REL123'
+    })
+
+    it('should return correct SFD update URL with ssoOrgId', () => {
+      vi.mocked(config.get).mockReturnValue('https://sfd.example.com/update')
+      const result = controller.getSFDUpdateUrl(mockRequest)
+      expect(result).toBe('https://sfd.example.com/update?ssoOrgId=REL123')
+    })
+
+    it('should return empty string if updateUrl config is missing', () => {
+      vi.mocked(config.get).mockReturnValue(undefined)
+      const result = controller.getSFDUpdateUrl(mockRequest)
+      expect(result).toBe('')
+    })
+
+    it('should log error and return empty string if updateUrl is invalid', () => {
+      vi.mocked(config.get).mockReturnValue('not-a-url')
+      const result = controller.getSFDUpdateUrl(mockRequest)
+      expect(result).toBe('')
+    })
   })
 
   describe('handleError', () => {