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: [DI-20934] - Configurable Max limit on resource selection component #11252

Merged
merged 10 commits into from
Nov 15, 2024

Conversation

ankita-akamai
Copy link
Contributor

Description 📝

Introduction of configurable Max limit on resource selection component and other enhancements.

Changes 🔄

  1. SELECT_ALL option in resources list component will not be available if number of resources are higher than the maximum resources selection limit set in launchDarkly.
  2. In resource selection autocomplete, user will be able to select resources up to maximum resource selection limit, all other resources(options) will be disabled after reaching this limit.
  3. Text 'Select up to limit label' is introduced under resource selection autocomplete.

Target release date 🗓️

12-12-2024

Preview 📷

Before After
image image
image image

How to test 🧪

Verification steps

  1. Login as a mock user.
  2. Select any resource type - linode or databases, and the relevant filters.
  3. If you have resources > max resource selection limit, you will not see any 'Select All' tab in resource selection autocomplete.
  4. Try selecting any resource after you have already selected resources up to max limit, you will not be able to select any additional resource as they are disabled after reaching max selection limit.

As an Author, I have considered 🤔

  • 👀 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

  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@ankita-akamai ankita-akamai requested a review from a team as a code owner November 13, 2024 09:28
@ankita-akamai ankita-akamai requested review from mjac0bs and harsh-akamai and removed request for a team November 13, 2024 09:28
Copy link

github-actions bot commented Nov 13, 2024

Coverage Report:
Base Coverage: 87.37%
Current Coverage: 87.37%

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Verified:

  • Any selections beyond the max limit are disabled
  • The Select All option is only present when the number of resource options is less than the limit
  • Test coverage is present and passing

Thanks @ankitaakamai, approving pending a couple of comments I left are addressed. I also called out one question about buggy UI behavior to see if the team is aware of it.

@mjac0bs mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Nov 13, 2024
Copy link
Contributor

@harsh-akamai harsh-akamai left a comment

Choose a reason for hiding this comment

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

Code review ✅
Verified changes in UI ✅

Thanks for the changes 🎉

@harsh-akamai harsh-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Nov 14, 2024
@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 445 passing tests on test run #7 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing445 Passing2 Skipped99m 42s

@kmuddapo
Copy link

@jaalah Are we good to merge this PR? Thanks!!

@mjac0bs
Copy link
Contributor

mjac0bs commented Nov 15, 2024

CI got hung up on needing an approval for the tests. Now that I've approved those and they've run, this looks good for merging. I'll merge, since Jaalah has already approved.

@mjac0bs mjac0bs merged commit 727dc75 into linode:develop Nov 15, 2024
23 checks passed
Copy link

cypress bot commented Nov 15, 2024

Cloud Manager E2E    Run #6828

Run Properties:  status check failed Failed #6828  •  git commit 727dc753f1: upcoming: [DI-20934] - Configurable Max limit on resource selection component (#...
Project Cloud Manager E2E
Branch Review develop
Run status status check failed Failed #6828
Run duration 28m 28s
Commit git commit 727dc753f1: upcoming: [DI-20934] - Configurable Max limit on resource selection component (#...
Committer ankitaakamai
View all properties for this run ↗︎

Test results
Tests that failed  Failures 1
Tests that were flaky  Flaky 3
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 444
View all changes introduced in this branch ↗︎

Tests for review

Failed  cypress/e2e/core/linodes/smoke-linode-landing-table.spec.ts • 1 failed test

View Output Video

Test Artifacts
linode landing checks for non-empty state with restricted user > checks restricted user with read access has no access to create linode and can see existing linodes Screenshots Video
Flakiness  linodes/linode-config.spec.ts • 1 flaky test

View Output Video

Test Artifacts
Linode Config management > End-to-End > Boots a config Screenshots Video
Flakiness  objectStorageGen2/bucket-object-gen2.spec.ts • 1 flaky test

View Output Video

Test Artifacts
Object Storage Gen2 bucket object tests > can check Object details drawer with E0 endpoint type Screenshots Video
Flakiness  volumes/delete-volume.spec.ts • 1 flaky test

View Output Video

Test Artifacts
volume delete flow > deletes a volume Screenshots Video

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! Cloud Pulse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants