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

feat: update and redirect /preview to /published #2854

Conversation

jessicamcinchak
Copy link
Member

@jessicamcinchak jessicamcinchak commented Mar 2, 2024

Last big functional update for #2783 !

Will rebase this branch once pizza builds against main. Opening as separate PR for easier review - this touches a lot of files.

Testing:

  • CI tests and nightly regression tests passing against this branch ✅
  • Editor 🌐 icon directly opens /published now (still disabled if flow is unpublished)
  • Saving a session & inviting to pay generate magic links which have /published in the URL; you can resume successfully using those magic links
  • After redirecting away to Gov Pay for any payment, you can redirect back to /published successfully
  • /preview redirects to /published and keeps query search params like analytics, sessionId, email, etc

Other considerations:

  • Update any planx-core references to /preview chore: update any Editor /preview URLs to /published planx-core#318 (can be merged later once publishing work is on staging, no direct dependencies here)
  • "Preview" language is still a bit pervasive throughout frontend code and isPreviewOnlyDomain is especially confusing now, I think we'll want to reconsider our internal usage of "preview" environments and related terms like "standalone" - but let's address this separately as there aren't any external communication dependencies that will impact PO testing

Copy link

github-actions bot commented Mar 2, 2024

Removed vultr server and associated DNS entries

@jessicamcinchak jessicamcinchak changed the base branch from main to jess/fetch-published-portals-on-publish March 2, 2024 09:42
@jessicamcinchak jessicamcinchak marked this pull request as ready for review March 4, 2024 07:34
@jessicamcinchak jessicamcinchak requested a review from a team March 4, 2024 07:34
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.

Code-only review looks great, thanks for the clear PR description 👍

Testing Pizza now and will then give a ✅

I've added a comment to this trello ticket to take into consideration the renaming of preview → published throughout the code base more generally.

@@ -92,7 +92,7 @@ export const getTeamFromDomain = async (domain: string) => {
/**
* Prevents accessing a different team than the one associated with the custom domain.
* e.g. Custom domain is for Southwark but URL is looking for Lambeth
* e.g. https://planningservices.southwark.gov.uk/lambeth/some-flow/preview
* e.g. https://planningservices.southwark.gov.uk/lambeth/some-flow
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch! 👍

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.

All checks listed in PR description pass as expected ✅

Also tried a few other things to try and break this - working as expected 🎉

@DafyddLlyr
Copy link
Contributor

One thing for BOPS dev call - seems super unlikely, but we should double-check if they hold any references to our service URLs outside of the new payload.

@jessicamcinchak jessicamcinchak merged commit e964a15 into jess/fetch-published-portals-on-publish Mar 4, 2024
17 checks passed
@jessicamcinchak jessicamcinchak deleted the jess/fetch-published-portals-on-publish-reroute-preview branch March 4, 2024 09:40
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