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

change: [M3-8607] - Change notification tax_id_invalid to tax_id_verifying #10967

Merged
merged 6 commits into from
Oct 2, 2024

Conversation

jaalah-akamai
Copy link
Contributor

@jaalah-akamai jaalah-akamai commented Sep 18, 2024

Description 📝

Renames the tax_id_invalid notification to tax_id_verifying to represent what it actually does. The event tax_id_invalid remains unchanged.

Changes 🔄

  • Removed the event from the toast since we have a verifying and success toast already in place. No longer need to account for invalid since that's handled immediately during validation in the edit billing contact drawer.

Target release date 🗓️

Note

In conjunction with APINext pr 7001

Preview 📷

No visual changes - ensure when changing tax id you see the loading icon. See recording in this PR for details #10928

How to test 🧪

To test, this requires you to run DevEnv pointing to the branch for apinext PR 7001

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@jaalah-akamai jaalah-akamai changed the title change: [M3-8607] - Replace tax_id_invalid to tax_id_verifying change: [M3-8607] - Change notification tax_id_invalid to tax_id_verifying Sep 19, 2024
@jaalah-akamai jaalah-akamai marked this pull request as ready for review September 20, 2024 20:07
@jaalah-akamai jaalah-akamai requested a review from a team as a code owner September 20, 2024 20:07
@jaalah-akamai jaalah-akamai requested review from carrillo-erik and cpathipa and removed request for a team September 20, 2024 20:07
Copy link

github-actions bot commented Sep 20, 2024

Coverage Report:
Base Coverage: 87.12%
Current Coverage: 87.12%

@coliu-akamai coliu-akamai added the Approved Multiple approvals and ready to merge! label Sep 24, 2024
@jaalah-akamai
Copy link
Contributor Author

jaalah-akamai commented Oct 1, 2024

@jdamore-linode Any idea why I'm getting a failure for plan-selection.spec.ts when using yarn build && yarn start:manager:ci. Hopefully it's not more gecko related issues with regionSelect, although it may be based on findItemByRegionLabel and the way we're using regex.

I think it's safe to merge this, but just looking for confirmation

@jdamore-linode
Copy link
Contributor

@jaalah-akamai Mind merging in the latest from develop to see if that clears up the test issues? These look like test failures that might've been fixed with PRs #10976 and #10978

@jaalah-akamai jaalah-akamai merged commit e9e3495 into linode:develop Oct 2, 2024
20 checks passed
Copy link

cypress bot commented Oct 2, 2024

Cloud Manager E2E    Run #6602

Run Properties:  status check passed Passed #6602  •  git commit e9e3495596: change: [M3-8607] - Change notification `tax_id_invalid` to `tax_id_verifying` (...
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #6602
Run duration 24m 48s
Commit git commit e9e3495596: change: [M3-8607] - Change notification `tax_id_invalid` to `tax_id_verifying` (...
Committer Jaalah Ramos
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 1
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 409
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Ready for Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants