From cef0dbbb8ae5dfd6d6a5cd32c959bc88d94c7b42 Mon Sep 17 00:00:00 2001 From: Bill Haigh Date: Wed, 29 Jul 2026 11:13:16 +0100 Subject: [PATCH] PDJB-NONE: Tag the leave-property commit step for the Transaction metric The leave property ("no longer a joint landlord") journey rendered its commit button from the untagged warningButton fragment, so completed journeys were not counted in the "Total number of transactions" metric. Swap it for transactionWarningButton, matching how property deregistration already does it. The fragment renders the same red destructive button, so there is no visual change. Also record two accepted coverage gaps in MetricsReadMe (cancelling a joint landlord invitation, and landlord address updates) and add a tagging rule to the journey instructions so new journeys do not repeat the omission. --- .github/instructions/journeys.instructions.md | 3 ++ docs/MetricsReadMe.md | 39 +++++++++++++++++-- .../forms/confirmLeavePropertyForm.html | 2 +- .../integration/TransactionEventTests.kt | 18 +++++++++ 4 files changed, 58 insertions(+), 4 deletions(-) diff --git a/.github/instructions/journeys.instructions.md b/.github/instructions/journeys.instructions.md index 971bacddfa..6da6593f9f 100644 --- a/.github/instructions/journeys.instructions.md +++ b/.github/instructions/journeys.instructions.md @@ -50,6 +50,9 @@ class PropertyComplianceJourneyFactory( } ``` +### Transaction Metric Tagging +A journey's final commit step must render its submit button via `transactionSubmitButton` (or `transactionWarningButton` for destructive actions) so the completion is counted in the transaction metric — either hardcoded in a commit-only template, or opted into with `"submitButton" to "transactionSubmitButton"` in `withAdditionalContentProperties` where the template is shared with non-commit steps. Tag exactly one button per completed journey; where that is not possible, record the gap in [MetricsReadMe](../../docs/MetricsReadMe.md) rather than risk double-counting. + ### Page Class Conventions Page class hierarchy: ``` diff --git a/docs/MetricsReadMe.md b/docs/MetricsReadMe.md index de0a6fcb17..2e72a6c463 100644 --- a/docs/MetricsReadMe.md +++ b/docs/MetricsReadMe.md @@ -51,15 +51,22 @@ The summary list contains 19 rows, in display order: ## Transaction counts -Completed transactions — registrations, deregistrations, updates, switch-to-individual, and accepting a -joint landlord invitation — are counted from a dedicated Plausible `Transaction` custom event. The event +Completed transactions — registrations, deregistrations, updates, switch-to-individual, accepting a +joint landlord invitation, and leaving a property as a joint landlord — are counted from a dedicated +Plausible `Transaction` custom event. The event is fired by a button press on each journey's final commit step: the commit button is rendered from a fragment tagged `data-plausible-event="Transaction"` (`transactionSubmitButton` / `transactionWarningButton`). For reporting periods before the configured cutover date (`plausible.transaction-event-start-date`) the legacy Flow event is used instead. See [AnalyticsReadMe](AnalyticsReadMe.md) and `PlausibleMetricsService` for details. -### Known coverage gap: landlord deregistration with no registered properties +### Known coverage gaps + +Three journeys are intentionally not counted. In each case there is no place to fire the event +exactly once per completion, and the project's standing preference is to **under-count rather than +over-count**. + +#### Landlord deregistration with no registered properties A landlord who has **no registered properties** deregisters via the `are-you-sure` page, which goes straight to the internal deregistration step and skips the `reason` page that carries the `Transaction` @@ -72,6 +79,32 @@ minimal (a landlord with no properties leaving the service) and there is no clea over-counting. Landlord deregistrations where the landlord *does* have properties, and all property deregistrations, are counted as normal. +#### Cancelling a joint landlord invitation + +The `cancelJointLandlordInvitation` journey's only button is on its `are-you-sure` page, and the +following `cancelInvitationStep` is internal with no button. + +**Decision:** cancelled invitations are intentionally **not counted**, for the same reason as above — +there is no place to fire the event that does not also fire it when the user answers "no". + +#### Landlord address updates + +The landlord address update journey commits on either `select-address` (a looked-up address was +chosen) or `manual-address` (reached by choosing "Add address manually" on the select-address page, +or when the lookup found no addresses). Tagging a step's button is a render-time decision, taken +before the user's radio selection is known, so tagging both steps would fire `Transaction` twice for +a single update whenever the user reaches `manual-address` *via* `select-address`. + +**Decision:** address updates are intentionally **not counted**. Tagging only `manual-address` would +count an arbitrary subset (missing the common "pick an address from the list" route), and suppressing +the event with JavaScript keyed on the selected radio would introduce a new tagging mechanism for a +single path. Under-counting was preferred to double-counting. The other four landlord detail updates +(name, email, phone number and date of birth) are counted as normal. + +Note that `forms/selectAddressForm.html` and `forms/manualAddressForm.html` are shared with the +landlord registration, property registration, governing body member address and trustee address +journeys, so any future fix must tag the update journey without affecting those. + ## Cost and cost per transaction The deployed `AwsCostExplorerMetricsClient` calls Cost Explorer's `GetCostAndUsage` operation in diff --git a/src/main/resources/templates/forms/confirmLeavePropertyForm.html b/src/main/resources/templates/forms/confirmLeavePropertyForm.html index 1bc85c65ca..8f06168f4a 100644 --- a/src/main/resources/templates/forms/confirmLeavePropertyForm.html +++ b/src/main/resources/templates/forms/confirmLeavePropertyForm.html @@ -25,7 +25,7 @@

- + leaveProperty.confirm.cancelLink diff --git a/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/TransactionEventTests.kt b/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/TransactionEventTests.kt index fa0528d1f1..1ef5d9eee9 100644 --- a/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/TransactionEventTests.kt +++ b/src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/TransactionEventTests.kt @@ -21,6 +21,7 @@ import uk.gov.communities.prsdb.webapp.integration.pageObjects.pages.landlordReg import uk.gov.communities.prsdb.webapp.integration.pageObjects.pages.landlordRegistrationJourneyPages.PhoneNumberFormPageLandlordRegistration import uk.gov.communities.prsdb.webapp.integration.pageObjects.pages.landlordRegistrationJourneyPages.PrivacyNoticePageLandlordRegistration import uk.gov.communities.prsdb.webapp.integration.pageObjects.pages.landlordRegistrationJourneyPages.SelectAddressFormPageLandlordRegistration +import uk.gov.communities.prsdb.webapp.integration.pageObjects.pages.leavePropertyJourneyPages.ConfirmPageLeaveProperty import uk.gov.communities.prsdb.webapp.integration.pageObjects.pages.propertyDetailsUpdateJourneyPages.NumberOfBedroomsFormPagePropertyDetailsUpdate import uk.gov.communities.prsdb.webapp.models.dataModels.VerifiedIdentityDataModel import uk.gov.communities.prsdb.webapp.models.viewModels.emailModels.LandlordRegistrationConfirmationEmail @@ -157,3 +158,20 @@ class AcceptJointLandlordInvitationTransactionEventTests : assertThat(page.locator(TAGGED_BUTTON_SELECTOR)).isVisible() } } + +class LeavePropertyTransactionEventTests : + IntegrationTestWithImmutableData("data-mockuser-landlord-with-sole-and-joint-properties.sql") { + private val jointPropertyOwnershipId = 2L + + @Test + fun `the leave property commit button is tagged for the Plausible Transaction event`(page: Page) { + navigator.goToLeavePropertyConfirmPage(jointPropertyOwnershipId) + + assertPageIs( + page, + ConfirmPageLeaveProperty::class, + mapOf("propertyOwnershipId" to jointPropertyOwnershipId.toString()), + ) + assertThat(page.locator(TAGGED_BUTTON_SELECTOR)).isVisible() + } +}