chore: add analytics tracking to capture PlanningConstraints & PropertyInformation "overrides"
#3510
Conversation
jessicamcinchak
changed the title
PlanningConstraint & PropertyInformation "overrides"PlanningConstraints & PropertyInformation "overrides"
|
Removed vultr server and associated DNS entries |
…ss/planning-constraints-analytics
…ss/planning-constraints-analytics
There was a problem hiding this comment.
All looking good!
Tested manually and hit an issue I previously missed - if you remove all constraints you passports ends up with [] as the value for property.constraints.planning. As this is a truthy value it could cause some issues - more accurately this should be undefined (which I think will then get it stripped out by computePassport() and then match a journey of a user who had no constraints).
| @@ -176,7 +178,13 @@ function Component(props: Props) { | |||
| if (data) _constraints.push(data as GISResponse["constraints"]); | |||
| } | |||
|
|
|||
| const _overrides = inaccurateConstraints; | |||
| const hasInaccurateConstraints = inaccurateConstraints && Object.keys(inaccurateConstraints).length > 0; | |||
| const _overrides = hasInaccurateConstraints ? { ...priorOverrides, [props.fn]: inaccurateConstraints } : undefined; | |||
There was a problem hiding this comment.
An observation / pattern I didn't really notice last time round when inaccurateConstraints were added to this component.
This component has transitioned from being informational only, to now capturing data much like other components. This data is generally captured and validated using formik (unlike the useState() hook here for inaccurateConstraints).
This particular function is essentially handling the "back" behaviour usually managed by getPreviouslySubmittedData().
If / when we come back to handle using formik for the OverrideEntitiesModal instances, it likely makes sense that this state would live at this root component level to match the behaviour of other components as opposed to individual instances within each modal as I think I'd previously suggested.
There was a problem hiding this comment.
Thanks for noting this here so we have record when coming back; will leave unresolved so stays expanded 👍
…ss/planning-constraints-analytics
| @@ -216,7 +216,7 @@ function Component(props: Props) { | |||
| _overrides, | |||
| "planningConstraints.action": userAction, | |||
| _nots: notsAfterOverrides, | |||
| ...intersectingConstraintsAfterOverrides, | |||
| ...(intersectingConstraintsAfterOverrides[props.fn]?.length === 0 ? undefined : intersectingConstraintsAfterOverrides), | |||
There was a problem hiding this comment.
Good catch with this one, thanks !
There was a problem hiding this comment.
Sorry @DafyddLlyr - actually second guessing this now - can you please test on this flow https://3510.planx.pizza/opensystemslab/digital-land-api-search-test/preview
When undefined instead of empty array (including for properties with no applicable constraints), then any future questions asking about property.constraints.planning (even if they have "none" / empty option) are NOT going to be automated because the passport doesn't have the key at all / thinks it's seeing it for the first time! I think previous behavior is intended / necessary?
There was a problem hiding this comment.
Okay sorry full circle here, disregard above comment, was totally my end-of-day confusion 🌀
Agree undefined is consistent with current prod behavior & what we want to keep 👍
Merging this since confident in handleOverrides test coverage and going to carve out some time tmrw morning to write more planning constraints tests for these wiggly/easy to confuse cases 👍
Adds new passport variables that we'll track via our analytics
ALLOW_LIST:propertyInformation.actionwith possible values "Accepted the property type" or "Changed the property type"planningConstraints.actionwith possible values "Accepted all planning constraints" or "Reported at least one inaccurate planning constraint"_overrideswhich is only defined if the user has changed the property type or reported inaccurate planning constraints and will include the specific details about what was changed/overrodeTodo:
analytics_summary&submissions_summaryviews ! Waiting on feat: add analytics tracking to "Did you use FOI service" question in LDCs #3508 to not have competing migrations open for the same thingTesting:
_overridesand the.actionvariables above as you'd expect !