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.

Sign up for GitHub

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

Jump to bottom

test: [M3-8591, M3-8594] - Add unit tests for AccountActivationLanding component, remove AddNewLink component #10966

Merged
merged 14 commits into from
Sep 20, 2024
Merged

test: [M3-8591, M3-8594] - Add unit tests for AccountActivationLanding component, remove AddNewLink component #10966

merged 14 commits into from
Sep 20, 2024

Conversation

coliu-akamai
Copy link
Contributor

@coliu-akamai coliu-akamai commented Sep 18, 2024 •
edited
Loading

Description 📝

  • adds unit tests to improve our test coverage
  • removed AddNewLink component, replacing it with Button

Changes 🔄

  • New tests for AccountActivationLanding
  • Remove AddNewLink, replacing it with Button

Target release date 🗓️

n/a

How to test 🧪

  • yarn test AccountActivationLanding
  • Confirm that buttons where AddNewLink was replaced still look/function as expected
  • confirm AccountActivationLanding still looks the same

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

@coliu-akamai coliu-akamai self-assigned this Sep 18, 2024
@coliu-akamai coliu-akamai added Storybook Unit tests Improves unit test coverage labels Sep 18, 2024
@coliu-akamai coliu-akamai marked this pull request as ready for review September 18, 2024 16:51
@coliu-akamai coliu-akamai requested a review from a team as a code owner September 18, 2024 16:51
@coliu-akamai coliu-akamai requested review from hana-akamai and carrillo-erik and removed request for a team September 18, 2024 16:51
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 18, 2024 •
edited
Loading

Coverage Report: ✅
Base Coverage: 86.94%
Current Coverage: 87.13%

pmakode-akamai
pmakode-akamai approved these changes Sep 19, 2024
View reviewed changes
Copy link
Contributor

@pmakode-akamai pmakode-akamai left a comment

Choose a reason for hiding this comment

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

Great work! 🎉🎉 Nice clean up with the Styles as well 🙌🏻.

Confirmed:

  • All test cases pass ✅
    Screenshot 2024-09-19 at 12 23 32 PM
  • AddNewLink story looks good ✅
  • AddNewLink still work as expected ✅
  • AccountActivationLanding still looks the same ✅

packages/manager/src/components/AddNewLink/AddNewLink.stories.tsx Outdated Show resolved Hide resolved
packages/manager/src/components/AddNewLink/AddNewLink.stories.tsx Outdated Show resolved Hide resolved
packages/manager/src/components/AddNewLink/AddNewLink.stories.tsx Outdated Show resolved Hide resolved
@pmakode-akamai pmakode-akamai added the Add'tl Approval Needed Waiting on another approval! label Sep 19, 2024
coliu-akamai and others added 2 commits September 19, 2024 08:23
@coliu-akamai @pmakode-akamai
Update packages/manager/src/components/AddNewLink/AddNewLink.stories.tsx
a8eef6f 
Co-authored-by: Purvesh Makode <pmakode@akamai.com>
@coliu-akamai @pmakode-akamai
Update packages/manager/src/components/AddNewLink/AddNewLink.stories.tsx
510766e 
Co-authored-by: Purvesh Makode <pmakode@akamai.com>
@coliu-akamai coliu-akamai marked this pull request as draft September 19, 2024 16:33
@coliu-akamai coliu-akamai changed the title test: [M3-8591, M3-8594] - Add unit tests for AddNewLink and AccountActivationLanding components test: [M3-8591, M3-8594] - Add unit tests for AccountActivationLanding component, remove AddNewLink component Sep 19, 2024
@coliu-akamai
Copy link
Contributor Author

@hana-linode removed AddNewLink! There is a slight difference in the disabled state with a tooltip, but I guess that's more inline with all other buttons now

AddNewLink Button
image image

@coliu-akamai coliu-akamai marked this pull request as ready for review September 19, 2024 18:00
pmakode-akamai
pmakode-akamai approved these changes Sep 20, 2024
View reviewed changes
Copy link
Contributor

@pmakode-akamai pmakode-akamai left a comment

Choose a reason for hiding this comment

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

Looks good! 🎉 Thanks for removing the AddNewLink component and replacing it with the Button! 🙌🏻🧹

packages/manager/src/features/Databases/DatabaseDetail/AccessControls.test.tsx Outdated Show resolved Hide resolved
...ges/manager/src/features/Databases/DatabaseDetail/DatabaseSettings/DatabaseSettings.test.tsx Outdated Show resolved Hide resolved
@hana-akamai hana-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Sep 20, 2024
@coliu-akamai coliu-akamai merged commit 27abb00 into linode:develop Sep 20, 2024
18 of 20 checks passed
This was referenced Sep 25, 2024
Merged
Merged
@coliu-akamai coliu-akamai deleted the m3-8591 branch October 9, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmakode-akamai pmakode-akamai pmakode-akamai approved these changes

@hana-akamai hana-akamai hana-akamai approved these changes

@carrillo-erik carrillo-erik Awaiting requested review from carrillo-erik carrillo-erik is a code owner automatically assigned from linode/frontend

Assignees

@coliu-akamai coliu-akamai

Labels
Approved Multiple approvals and ready to merge! Unit tests Improves unit test coverage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@coliu-akamai @hana-akamai @pmakode-akamai