Skip to content

PDJB-1133: Organisation address lookup - #1610

Open
bnjn-mt wants to merge 2 commits into
mainfrom
feat/PDJB-1133-organisation-address
Open

PDJB-1133: Organisation address lookup#1610
bnjn-mt wants to merge 2 commits into
mainfrom
feat/PDJB-1133-organisation-address

Conversation

@bnjn-mt

@bnjn-mt bnjn-mt commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Ticket number

PDJB-1133

Goal of change

Allow organisational landlords to find and select their organisation’s address during registration.

Screenshot 2026-07-20 at 13 53 03 Screenshot 2026-07-20 at 13 54 03 Screenshot 2026-07-20 at 13 53 43 Screenshot 2026-07-20 at 13 53 25 Screenshot 2026-07-20 at 13 53 13 Screenshot 2026-07-20 at 13 55 15

Description of main change(s)

  • Replaces the organisation landlord manual-address step with the shared address lookup/select/manual flow under the organisation-address route.
  • Adds organisation-specific lookup and select-address content so the organisation address journey matches the shared address pattern.
  • Updates navigation, page objects, and integration coverage for the new organisation address flow.
  • Restores org-address single-page coverage for validation errors and the successful lookup-to-email path.

Anything you'd like to highlight to the reviewer?

The the 2028 cohort renders multi-unit median, p90 and p95 durations down to minutes int test on line 117 in MetricsSinglePageTest will fail locally in this branch. It passes when run with GH actions. There is a fix for it here. This shouldn't block this PR from being merged.

Checklist

  • Screenshots of any UI changes have been added
  • Single page integration tests have been added for any unhappy-flow UI features, e.g. validation errors
  • New journey steps have been added to the appropriate journey integration test(s)
  • Branch has been rebased onto main and run locally
  • TODO comments referencing this JIRA ticket have been searched for and removed
  • Test suite has been run in full locally and is passing
  • QA instructions have been added to the ticket
  • This feature is behind a feature flag. I've checked that there will be no change in function if the feature flag is disabled

@bnjn-mt bnjn-mt changed the title PDJB-1133: Route organisation address through shared address task PDJB-1133: Organisation address lookup Jul 17, 2026
@bnjn-mt
bnjn-mt force-pushed the feat/PDJB-1133-organisation-address branch 5 times, most recently from af60006 to 3d93e50 Compare July 17, 2026 18:13
…registration

Co-authored-by: samyou-softwire <108681823+samyou-softwire@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bnjn-mt
bnjn-mt force-pushed the feat/PDJB-1133-organisation-address branch from e4098f4 to 1517743 Compare July 22, 2026 16:35
@bnjn-mt
bnjn-mt marked this pull request as ready for review July 22, 2026 16:36

@samyou-softwire samyou-softwire left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a couple comments

would LGTM but a lot of this is code I drafted up so it feels like I'm marking my own homework 😅

val yourDetailsStep: YourDetailsStep
val orgNameStep: OrgNameStep
val orgAddressStep: OrgAddressStep
val orgAddressTask: AddressTask

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this typing feels weaker than expected; can this be an OrgAddressTask?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is a forms.yml usage, but maybe it's fine for this shared task

fun skipToOrgLandlordRegistrationOrgAddressPage(): OrgAddressFormPageLandlordRegistration {
setJourneyStateInSession(LandlordStateSessionBuilder.beforeOrgAddress().build())
navigateToLandlordRegistrationJourneyStep(OrgAddressStep.ROUTE_SEGMENT)
navigateToLandlordRegistrationJourneyStep("${OrgAddressTask.ORGANISATION_ADDRESS_ROUTE_SEGMENT}/lookup-address")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we be using a hardcoded address here for lookup-address?

}

@Test
fun `submitting without an address line 1 returns the missing address line 1 error`(page: Page) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

have we lost these tests for the manual address page?

ManualAddressFormModel().apply {
addressLineOne = "1 Example Street"
townOrCity = "Exampleton"
val routePrefix = OrgAddressTask.ORGANISATION_ADDRESS_ROUTE_SEGMENT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this does seem a little suspicious - is this what other state session builders do for address?

@Travis-Softwire Travis-Softwire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM subject to Sammy's comments being addressed (and one from me where I think we can start as we mean to on with migrating message keys) :)

propertyRegistration:
fieldSetHeading: What is the property address?
fieldSetHint: This should be an address within England
organisationLandlordRegistration:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In the interest of incrementally moving towards the message files we want, I wonder if it's worth splitting out the address keys into their own file?

"fieldSetHint" to "forms.lookupAddress.govBodyMemberRegistration.fieldSetHint",
)

override val selectAddressContentProperties: Map<String, Any?> = emptyMap()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We're setting the fieldSetHeading via the journey DSL at the moment, we should move that here instead now that we have it

@TomHanmer29 TomHanmer29 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've just merged PDJB-1168 which includes a TODO for this PR, just wanted to flag before you merged this @bnjn-mt :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants