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: fetch & flatten the latest published version of external portal data when publishing the parent flow #2783

Merged
merged 30 commits into from
Mar 15, 2024

Conversation

jessicamcinchak
Copy link
Member

@jessicamcinchak jessicamcinchak commented Feb 13, 2024

We're changing the rules of how we "publish" flows:

  • Currently, "publishing" snapshots the draft content of the parent flow, and the draft content from each external portal (regardless of published status) into a single "published version" of that parent flow
  • Now, the parent flow can only be published if each of its' external portals are also published
    • This means "publishing" now snapshots the draft of the parent flow, and the published content of each external portal
  • This also introduces three unique routes for viewing your flow content in the Editor:
    1. /preview (next → /published) - the latest published version of the parent flow
      • Icon disabled and displays NotFound if not published yet
    2. /amber (next → /preview) - the draft of the parent, plus the latest published version of each external portal
      • Reflects exactly what you'll get when you click publish next
      • Displays an error warning and NotFound if any external portals are unpublished (same error as when you click "Check for changes to publish")
    3. /draft - the draft of the parent, plus the draft of each external portal
      • Useful for testing automations and content changes that cross many different flows
      • Hidden unless your user role is platformAdmin

Key code changes:

  • [API & Editor] Updates dataMerged flattening logic (incl tests) in the API ONLY and removes the duplicate definition from the Editor
  • [API] Adds a public GET endpoint /flow/:flowId/flatten-data (incl tests and Swagger doc) that can be called by the Editor views /amber & /draft instead of relying on the duplicate definition of dataMerged
  • [API] Updates existing tests for validateAndDiff & publish to reflect new rules and error handling, and draw from consistent mock data
  • [Editor] Aligns views to 'next' route names (draft, preview, published)
  • [Editor] MVP simplified diff message of changes to publish and notification badges on publish button

Testing notes:

  • publish succeeds when external portals are published
  • publish throws a meaningful error when external portals are not published
  • /published loads published flow and published external portal data
    • /preview redirects to /published
  • /amber loads draft flow and published external portal data
    • /unpublished redirects to /draft
  • /draft loads draft flow and draft external portal data

Future scope:

  • Redirect /amber/preview after ~30 days (will set Slack channel reminder)
  • Addressing blank screens when reaching portals in the editor sidebar
  • Concept of when a flow is "live"

Copy link

github-actions bot commented Feb 13, 2024

Removed vultr server and associated DNS entries

@jessicamcinchak jessicamcinchak marked this pull request as ready for review February 20, 2024 15:25
@jessicamcinchak jessicamcinchak requested a review from a team February 20, 2024 15:25
Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

Just done a code-only review with a few small comments / nits, will test on Pizza now 👍

API testing looks solid 👌

image

editor.planx.uk/src/routes/views/publishedPreview.tsx Outdated Show resolved Hide resolved
editor.planx.uk/src/lib/dataMergedHotfix.ts Outdated Show resolved Hide resolved
api.planx.uk/modules/flows/routes.ts Show resolved Hide resolved
api.planx.uk/modules/flows/flattenFlow/service.ts Outdated Show resolved Hide resolved
api.planx.uk/modules/flows/flattenFlow/service.ts Outdated Show resolved Hide resolved
api.planx.uk/modules/flows/docs.yaml Show resolved Hide resolved
api.planx.uk/modules/flows/flattenFlow/controller.ts Outdated Show resolved Hide resolved
api.planx.uk/helpers.test.ts Outdated Show resolved Hide resolved
api.planx.uk/modules/flows/flattenFlow/flattenFlow.test.ts Outdated Show resolved Hide resolved
@DafyddLlyr
Copy link
Contributor

Had a good play around with this and all is working as expected on Pizza ✅

  • publish succeeds when external portals are published
  • publish throws a meaningful error when external portals are not published
  • /preview loads published flow and published external portal data
  • /publish-preview loads draft flow and published external portal data
  • /unpublished loads draft flow and draft external portal data

Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

Changes look good thanks!

Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

Reviewed routing changes ✅

…latest version is always flattened and not old snapshot (#2839)
@jessicamcinchak jessicamcinchak merged commit ddd665f into main Mar 15, 2024
12 checks passed
@jessicamcinchak jessicamcinchak deleted the jess/fetch-published-portals-on-publish branch March 15, 2024 15:56
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.

2 participants