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

upcoming: [M3-8217] - Tax Id Notifications & Icon #10558

Merged
merged 19 commits into from
Jul 15, 2024

Conversation

jaalah-akamai
Copy link
Contributor

@jaalah-akamai jaalah-akamai commented Jun 7, 2024

Description 📝

New notifications for Tax Id

Changes 🔄

  • Add two new notifications when tax id is valid/invalid
  • Add warning icon next to tax id in billing section
    • Changed icon to solid variant for visibility (only used in this location in the app)

Target release date 🗓️

7/8

Preview 📷

worked.mov
Desc SS
Notifications Screenshot 2024-06-26 at 8 56 23 AM
Toast Screenshot 2024-07-10 at 12 09 48 PM
Event Screenshot 2024-07-10 at 3 04 59 PM
Icon Before Icon After (ignore dark mode color changes)
Screenshot 2024-06-07 at 12 57 34 PM Screenshot 2024-07-08 at 9 59 11 AM

How to test 🧪

Prerequisites

  • If you want to test the events part of this, you'll need to setup DevEnv (see me about details)
  • Otherwise, you can locally comment out the rendering conditions for the TooltipIcon

Validation

  • Checkout this branch in devenv
  • Checkout latest develop branch of apinext in devenv
  • Add a row to the TaxValidation table with format_valid set to 1, and tax_registered to yes.
  • curl /account/notifications and notice the tax_id_valid notification.
  • Set format_valid to 0, and curl /account/notifications again, notice tax_id_invalid notification. Try again with format_valid set to 1, and tax_registered set to no.

Related

@jaalah-akamai jaalah-akamai changed the title upcoming: upcoming: [M3-8217] - Tax Id Notifications & Icon Jun 7, 2024
@jaalah-akamai jaalah-akamai self-assigned this Jun 7, 2024
@jaalah-akamai jaalah-akamai marked this pull request as ready for review June 26, 2024 13:04
@jaalah-akamai jaalah-akamai requested a review from a team as a code owner June 26, 2024 13:04
@jaalah-akamai jaalah-akamai requested review from jdamore-linode and hkhalil-akamai and removed request for a team June 26, 2024 13:04
Copy link
Contributor

@hkhalil-akamai hkhalil-akamai left a comment

Choose a reason for hiding this comment

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

I don't have devenv set up but I mocked the notification locally and verified the notification and tooltip appear as expected and display the tax id warning.

Copy link

github-actions bot commented Jun 27, 2024

Coverage Report:
Base Coverage: 82.31%
Current Coverage: 82.31%

@jaalah-akamai jaalah-akamai added Add'tl Approval Needed Waiting on another approval! and removed Missing Changeset labels Jun 27, 2024
@jaalah-akamai
Copy link
Contributor Author

I need to fix some invalidation issues with RQ - please hold

@jaalah-akamai jaalah-akamai requested a review from a team as a code owner June 27, 2024 21:29
@jaalah-akamai jaalah-akamai requested review from AzureLatte and removed request for a team June 27, 2024 21:29
@jaalah-akamai
Copy link
Contributor Author

Devenv has been having issues all day preventing me from e2e testing this - I will wait until things are back up and re-request reviews.

…oPanel/ContactInformation.tsx

Co-authored-by: Banks Nussman <[email protected]>
Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Just a thought 💡

It might be a really nice UX to add toast notifications in packages/manager/src/hooks/useToastNotifications.tsx

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

React Query and event handler changes look great ✅

@bnussman-akamai bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Jul 10, 2024
@jaalah-akamai jaalah-akamai merged commit 90b6625 into linode:develop Jul 15, 2024
19 checks passed
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!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants