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() + } +}