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: send updated data back to client from all update pages endpoints #1193

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

jacobkwan
Copy link
Contributor

@jacobkwan jacobkwan commented Mar 7, 2024

Problem

https://opengovproducts.slack.com/archives/CK68JNFHR/p1709007514442959?thread_ts=1708332491.637599&cid=CK68JNFHR

Fixes issue where users see warning modal even after saving changes

Closes [insert issue #]

Solution

  1. updating/saving/syncing state logic seems quite convoluted on the FE
  2. However, frontend is indeed updating editor content after the POST request returns (see useUpdatePageHook in EditPageLayout)
  3. issue is that /update endpoints are not returning the updated frontmatter and pageContent to the client

This seems like the simplest solution to fixing this bug, i.e returning updated pageContent to the client,
but would be good to refactor/simplify the FE logic next time

Breaking Changes

  • Yes - this PR contains breaking changes
    • Details ...
  • No - this PR is backwards compatible with ALL of the following feature flags in this doc

Features:

  • Details ...

Improvements:

  • Details ...

Bug Fixes:

  • Details ...

Before & After Screenshots

BEFORE:

AFTER:

Tests

  1. Go to Prod CMS, edit a page to leave trailing whitespace then save changes
  2. Click Back on the navbar -> should see the unsaved changes modal
  3. Go to Staging CMS, edit a page to leave trailing whitespace then save changes
  4. After saving, should see that trailing whitespace is removed from the editor (to match backend)
  5. Click Back on the navbar -> should not see the modal

Deploy Notes

New environment variables:

  • env var : env var details
    • added env var to 1PW + SSM script (fetch_ssm_parameters.sh)

New scripts:

  • script : script details

New dependencies:

  • dependency : dependency details

New dev dependencies:

  • dependency : dependency details

@jacobkwan jacobkwan requested a review from a team March 7, 2024 08:41
@jacobkwan jacobkwan changed the title fix: send updated data back to client, from all update pages endpoints fix: send updated data back to client from all update pages endpoints Mar 7, 2024
@kishore03109 kishore03109 self-requested a review March 11, 2024 09:50
Copy link
Contributor

@kishore03109 kishore03109 left a comment

Choose a reason for hiding this comment

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

hmm i dont understand this bug.
is this deterministic?

issue is that /update endpoints are not returning the updated frontmatter and pageContent to the client

hmm what are the changes between frontMatter and new frontmatter?

the ticket is for nlb, which means that it is not required to actually do a post call to gh, so am unclear what the diff here would be?

Copy link
Contributor

@kishore03109 kishore03109 left a comment

Choose a reason for hiding this comment

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

discussed offline, understand this now

@jacobkwan jacobkwan merged commit c243784 into develop Mar 12, 2024
19 checks passed
@mergify mergify bot deleted the fix/update-endpoint-not-returning-new-data branch March 12, 2024 10:15
@dcshzj dcshzj mentioned this pull request Mar 14, 2024
14 tasks
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