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

[UI] fix filter on api explorer #26657

Merged
merged 3 commits into from Apr 29, 2024
Merged

[UI] fix filter on api explorer #26657

merged 3 commits into from Apr 29, 2024

Conversation

noelledaley
Copy link
Contributor

@noelledaley noelledaley commented Apr 25, 2024

πŸ› οΈ Description

Fixes the search filter on the OpenAPI explorer.

πŸ”— Links

πŸ“Έ Screenshots

Screen.Recording.2024-04-26.at.11.51.48.AM.mov

πŸ—οΈ How to Build and Test the Change

Visit the OpenApi explorer page and the search box should work again πŸŽ‰

@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Apr 25, 2024
Copy link

github-actions bot commented Apr 25, 2024

CI Results:
All Go tests succeeded! βœ…

@noelledaley noelledaley changed the title wip - fix filter on api explorer [UI] fix filter on api explorer Apr 26, 2024
@noelledaley noelledaley added this to the 1.17.0-rc milestone Apr 26, 2024
return (
taggedOps
.map((tagObj) => {
const operations = tagObj.operations.filter((operationObj) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this line was failing because the type of tagObj changed to a Map, which does not have an .operations property. instead we need to use the Map API's get & set methods to access or set properties.

// then traverse again and remove the top level item if there are no operations left after filtering
.filter((tagObj) => !!tagObj.operations.size)
);
const filteredOperations = taggedOps.reduce((acc, tagObj) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the reduce function here allows us to combine the previous map, filter and thenmap that we had before. πŸŽ‰

@noelledaley noelledaley marked this pull request as ready for review April 26, 2024 23:56
@noelledaley noelledaley requested a review from a team as a code owner April 26, 2024 23:56
Copy link

Build Results:
All builds succeeded! βœ…

assert.dom('[data-test-swagger-ui]').exists('renders component');
await waitUntil(() => find('.operation-filter-input'));
assert.dom('.opblock-post').exists({ count: 2 }, 'renders two blocks');
// for some reason search results are not rendered immediately in tests,
Copy link
Contributor

Choose a reason for hiding this comment

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

One thought is you could add a waitUntil that looks for .operation-tag-content or a child element's class?

But checking the input has a value feels sufficient because the whole page errors when the function fails. Maybe it's worth including a comment that explains as much?

Screenshot 2024-04-29 at 3 47 51β€―PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, i tried many variations of waitUntil, waitFor and setTimeout to wait for the search results to update to no avail. what's bizarre is that when i add a await this.pauseTest() immediately after fillIn('some search term'), the text input is still empty and the search results don't update, but if i insert a debugger the search input and results work exactly as expected. so there's something going on with timing here.

unfortunately since the execution of the opsFilter function (aka our custom search function) happens outside of our context (within the source code of swagger-ui-dist, it's difficult to debug. my thoughts were exactly what you mentioned -- if our search function errors, the whole page breaks, and so asserting that the search input gets entered correctly tells us that the page is "working" (for the most part). πŸ˜•

i'll update my comment in the code here to make that more clear, though!

Copy link
Contributor

@hellobontempo hellobontempo 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 tracking this down! Thanks for the additional comments and test coverage 🀩

@noelledaley noelledaley enabled auto-merge (squash) April 29, 2024 17:40
@noelledaley noelledaley merged commit eaadfb6 into main Apr 29, 2024
31 checks passed
@noelledaley noelledaley deleted the ui/vault-25838/fix-filter branch April 29, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed pr/no-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants