Skip to content

Feat: Add SubscriptionList#80

Open
mariana-caetano wants to merge 5 commits intomainfrom
feat/subscription-list
Open

Feat: Add SubscriptionList#80
mariana-caetano wants to merge 5 commits intomainfrom
feat/subscription-list

Conversation

@mariana-caetano
Copy link
Copy Markdown

@mariana-caetano mariana-caetano commented Mar 31, 2026

Description

To test locally:

  1. Clone the repository.
  2. Run in the terminal yarn install.
  3. Change the branch to feat/subscription-list: git checkout feat/subscription-list.
  4. Run Storybook: yarn storybook.
  5. Go to http://localhost:6006/ and navigate to SubscriptionList in the left sidebar.
Screenshot_255

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Requires change to documentation, which has been updated accordingly.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: ['release-no', 'release-auto', 'release-patch', 'release-minor', 'release-major']

@mariana-caetano mariana-caetano self-assigned this Mar 31, 2026
@mariana-caetano mariana-caetano added the release-auto Automatic version bump label Mar 31, 2026
const emailEncoded = encodeURIComponent(email)
const url = baseURL + emailEncoded + urlEnd

fetch(url, { method: 'POST' })
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should improve error handling.

For fetch(), 4xx and 5xx responses are not errors. They represent an error on the part of the API, but they don't trigger the .catch() (98). This means that if there is any error in zapier, or one of the services it uses, and fetch() returns 500, the user will get a success message, when in fact the email won't be registered to the subscription list.

return
}

const isValid = await checkEmail(email)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The error treatment here is innacurate. checkEmail(email) can return false for reasons other than invalid emails:

  • The RapidAPI request fails.
  • The RapidAPI response is otherwise unusable.
  • NEXT_PUBLIC_NEWSLETTER_API_KEY is missing - (E.g., I tested the component with storybook and entered my VTEX email. I got the 'invalid email' error message and this was probably the reason.) This is probably not an issue in production once the environment is correctly configured.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-auto Automatic version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants