Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure reconciliation behaves as expected and does not result in orphaned breadcrumbs #4218

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jessicamcinchak
Copy link
Member

@jessicamcinchak jessicamcinchak commented Jan 28, 2025

In-progress investigation based on https://opensystemslab.slack.com/archives/C088K9ZL8EA/p1737533997179459

What I still want to check:

  • Specifics of our orphaned breadcrumb cases:
    • e5d22126-98f9-4e47-99f3-9c86265d6ae3 was invite-to-pay which would have "skipped" reconciliation - should reconciliation have a basic version of reconciliation to at minimum remove now-orphaned breadcrumbs or should the payload generation be adjusted to join to the version of the published flow when the session was locked_at ?
    • 229f0f51-7101-4c5a-adaf-7c159b8d7b20 was not invite-to-pay, but had a near-full-month lapse between starting & submitting
  • Longstanding TODO comments at the top of api.planx.uk/modules/saveAndReturn/service/validateSession.ts
    • Confirm that reconciliation correctly handles flags/filters/results
    • Confirm that reconciliation correctly handles dependent component types like FindProp, Draw, PlanningConstraints

// remove all auto-answered breadcrumbs
// (auto-answers are reconstructed in the editor by `upcomingCards`)
// remove all auto-answered breadcrumbs because their automation may rely on the same data values as an altered node
// (auto-answers can be reconstructed on forwards navigation if they are still auto-answerable on resume)
Copy link
Member Author

Choose a reason for hiding this comment

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

See existing test('auto-answered breadcrumbs with matching "data.val" values are also removed') in validateSession.test.ts to better understand this. I ultimately believe this is correct behavior

Copy link

Pizza

Deployed 63b5e85 to https://4218.planx.pizza.

Useful links:

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.

1 participant