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

feat: [M3-8883] - Use our search parser on Images Landing #11233

Conversation

bnussman-akamai
Copy link
Member

@bnussman-akamai bnussman-akamai commented Nov 8, 2024

Description 📝

  • Uses our search parser on the Images Landing page to provide users with slightly more flexibility 🔍
  • This is just a "soft-launch". We can add user facing documentation as a follow up once we're confident in the feature and the API has had some time to improve filterability.

Preview 📷

Search by region

This will help power users using Image Service Gen2

Note

Sadly does not support complex operations (+or / +and) due to API limitations. Only available in dev right now.

screenshot

Search by label and/or size

Note

Because the API returns size in MB, the user would have to search in MBs 🫤 Planning to bring this to UX, but not blocking in my opinion

Screenshot 2024-11-08 at 1 02 38 PM

Search by tags (or tag because it is aliased)

Adds a sustainable way to filter by tag 🎉

Note

Searching by tag is partially broken. See ARB-5792

Screenshot 2024-11-08 at 1 05 38 PM

How to test 🧪

  • Test various search queries on the Images landing page 🔍
  • Verify the UI gracefully handles valid and invalid searches ✅ ❌

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

@bnussman-akamai bnussman-akamai self-assigned this Nov 8, 2024
@bnussman-akamai bnussman-akamai requested a review from a team as a code owner November 8, 2024 18:26
@bnussman-akamai bnussman-akamai requested review from hana-akamai and abailly-akamai and removed request for a team November 8, 2024 18:26
Copy link

github-actions bot commented Nov 8, 2024

Coverage Report:
Base Coverage: 87.34%
Current Coverage: 87.34%

Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

This is great and a nice improvement 🎉 Left comments that should be addressed, but are non-blocking.

  • confirmed search parity with search filter and global search ✅
  • confirmed error handling is sound ✅

My only concern in general with the feature (not so now, cause not only is it a parity item but a considerable improvement) is how can we communicate this feature to the user knowing that:

  • every endpoint has its quirks and may or may not support a variety of filters, including filter operators
  • every endpoint has its own handling of errors (sometimes we'll get a proper 400 with a message, sometimes a 500)

This is going to be an intricate UI and I wonder if we should start thinking of a util that's going to help us generate this contextual help, taking as arguments the things it can and can't do (with which we could also perhaps return our own errors)

Screenshot 2024-11-13 at 09 32 08

Screenshot 2024-11-13 at 09 28 29

@bnussman-akamai bnussman-akamai added the Images Relating to Images label Nov 14, 2024
@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🔺 1 failing test on test run #6 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
1 Failing444 Passing2 Skipped101m 37s

Details

Failing Tests
SpecTest
create-linode-region-select.spec.tsLinode Create Region Select » region select

Troubleshooting

Use this command to re-run the failing tests:

yarn cy:run -s "cypress/e2e/core/linodes/create-linode-region-select.spec.ts"

@bnussman-akamai bnussman-akamai merged commit b40d5bd into linode:develop Nov 14, 2024
22 of 23 checks passed
Copy link

cypress bot commented Nov 14, 2024

Cloud Manager E2E    Run #6824

Run Properties:  status check passed Passed #6824  •  git commit b40d5bd530: feat: [M3-8883] - Use our search parser on Images Landing (#11233)
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #6824
Run duration 24m 57s
Commit git commit b40d5bd530: feat: [M3-8883] - Use our search parser on Images Landing (#11233)
Committer Banks Nussman
View all properties for this run ↗︎

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

hasyed-akamai pushed a commit to hasyed-akamai/manager that referenced this pull request Nov 15, 2024
* initial work

* make overriding contains filter shape more generic

* make things more generic

* allow underscore in seaches

* allow `:` in searches

* only disable retries if the user is searching

* add changeset

* add changeset

* use `errorText` prop @hana-akamai

* add comment to summarize what is filterable

---------

Co-authored-by: Banks Nussman <banks@nussman.us>
hasyed-akamai pushed a commit to hasyed-akamai/manager that referenced this pull request Nov 15, 2024
* initial work

* make overriding contains filter shape more generic

* make things more generic

* allow underscore in seaches

* allow `:` in searches

* only disable retries if the user is searching

* add changeset

* add changeset

* use `errorText` prop @hana-akamai

* add comment to summarize what is filterable

---------

Co-authored-by: Banks Nussman <banks@nussman.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Images Relating to Images
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants