Skip to content

fix(ia): rerender subscription lists on esp provider update#3815

Merged
chickenn00dle merged 4 commits into
epic/iafrom
fix/reload-subscription-lists-on-esp-save
Mar 18, 2025
Merged

fix(ia): rerender subscription lists on esp provider update#3815
chickenn00dle merged 4 commits into
epic/iafrom
fix/reload-subscription-lists-on-esp-save

Conversation

@chickenn00dle

@chickenn00dle chickenn00dle commented Mar 11, 2025

Copy link
Copy Markdown
Contributor

All Submissions:

Changes proposed in this Pull Request:

Closes https://app.asana.com/1/26890605006346/project/1205919985867982/task/1209464932054166?focus=true

This PR fixes an issue where updating a provider does not always refresh the subscriptions lists associated with that provider until a page reload. This happened because subscriptions lists would rerender when the provider was updated, but before api credentials were stored and settings were saved. As a result updating the provider would cause the subscriptions lists to fetch using the outdated provider details.

We fix this by:

  • Only rerendering subscriptions lists when provider updates AND lists are not locked
  • Correctly locking lists depending on provider update conditions

Screenshot 2025-03-12 at 11 47 40

How to test the changes in this Pull Request:

Onboarding

  1. Reset newspack via the reset newspack link at the bottom of any wizard
    Screenshot 2025-03-12 at 11 48 42
  2. Navigate to Audience > Setup > Email Service Provider, and click the button to do the ESP step
  3. Confirm the subscriptions lists section is not visible
  4. Select an ESP provider and enter valid credentials
  5. Confirm the subscriptions lists section appears and renders the expected lists

Switching Providers

  1. Select a different provider
  2. Confirm the subscriptions lists section renders a warning to save before editing subscriptions lists
  3. Enter valid credentials for the new ESP
  4. Confirm the subscriptions lists section renders the expected lists after loading

Bonus: Smoke test the ESP and Subscriptions Lists settings with invalid credentials, etc.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@chickenn00dle chickenn00dle force-pushed the fix/reload-subscription-lists-on-esp-save branch from 1d7ab90 to 02cd846 Compare March 12, 2025 15:45
@chickenn00dle chickenn00dle marked this pull request as ready for review March 12, 2025 15:53
@chickenn00dle chickenn00dle requested a review from a team as a code owner March 12, 2025 15:53
@chickenn00dle chickenn00dle added the [Status] Needs Review The issue or pull request needs to be reviewed label Mar 12, 2025

@miguelpeixe miguelpeixe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fixes the issue!

The whole lockedLists idea is not great (my bad) and the regex for provider key detection is a bit fragile. I don't think we should refactor this right now, though.

@github-actions github-actions Bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Mar 17, 2025
@chickenn00dle chickenn00dle merged commit ce73d20 into epic/ia Mar 18, 2025
@chickenn00dle chickenn00dle deleted the fix/reload-subscription-lists-on-esp-save branch March 18, 2025 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Information Architecture [Status] Approved The pull request has been reviewed and is ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants