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

Use new ListResources in the webapi layer for pagination and filtering #11019

Merged
merged 9 commits into from
Apr 20, 2022

Conversation

kimlisa
Copy link
Contributor

@kimlisa kimlisa commented Mar 10, 2022

part of RFD 55

Description

In addition proccess query params sent in with request

Before Merge

  • Requires webassets update with corresponding changes (or else the web UI list views will be inaccurate)
  • Mark RFD 55 state from draft to implemented

@github-actions github-actions bot requested review from alex-kovoy and rudream March 10, 2022 03:35
@kimlisa kimlisa requested review from espadolini and rosstimothy and removed request for alex-kovoy and rudream March 10, 2022 03:35
lib/web/resources.go Outdated Show resolved Hide resolved
lib/web/ui/server.go Outdated Show resolved Hide resolved
lib/web/apiserver_test.go Outdated Show resolved Hide resolved
lib/web/resources_test.go Show resolved Hide resolved
lib/web/resources_test.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@zmb3 zmb3 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 couple minor comments.

lib/web/apiserver_test.go Outdated Show resolved Hide resolved
lib/web/apiserver_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@espadolini espadolini left a comment

Choose a reason for hiding this comment

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

Nothing to add

@kimlisa kimlisa force-pushed the lisa/webapi-use-newapi branch from 6d6f2b0 to bfd192a Compare March 14, 2022 18:31
@kimlisa kimlisa force-pushed the lisa/webapi-use-newapi branch from bfd192a to 584feaa Compare March 24, 2022 18:23
@kimlisa
Copy link
Contributor Author

kimlisa commented Mar 24, 2022

May i get a review for this commit please from either @zmb3, @espadolini, or @rosstimothy: 584feaa

The web UI needed a flag that would tell it if the list is empty from filter results, or if user just didn't have any resource to access. These are the views depending on this flag and list results:

  1. If a list result was empty, and user had no resources to access, and the user has read + write access then the UI will show the user how to get started:
    image

  2. If a list result was empty, and user had no resources to acces, and the user has read only access then we show this screen:
    image

  3. If a list result was empty, and user HAS resources to access, then it means there was no results found after applying filter (we have a bookmarkable url feature that saves the filter queries, so on first load it can be empty b/c of no results):
    image

@espadolini
Copy link
Contributor

@kimlisa in a hypothetical future in which we can do filtering in the backend itself that extra flag would force us to still figure out if something else could've been available - wouldn't it be simpler to only show the getting started UI element if the filter textbox is empty?

@kimlisa
Copy link
Contributor Author

kimlisa commented Mar 25, 2022

@kimlisa in a hypothetical future in which we can do filtering in the backend itself that extra flag would force us to still figure out if something else could've been available - wouldn't it be simpler to only show the getting started UI element if the filter textbox is empty?

i think you're right @espadolini, it's also weird that we are ignoring the filter field too (in the UI b/c it will get replaced with the empty state), reverted

@kimlisa kimlisa enabled auto-merge (squash) April 19, 2022 16:51
@kimlisa kimlisa disabled auto-merge April 19, 2022 16:51
@kimlisa kimlisa enabled auto-merge (squash) April 19, 2022 23:09
@kimlisa kimlisa merged commit 6b3d7eb into master Apr 20, 2022
@kimlisa kimlisa deleted the lisa/webapi-use-newapi branch April 20, 2022 04:16
kimlisa added a commit that referenced this pull request Apr 20, 2022
Supports pagination and filtering for the web UI
Part of RFD 55
kimlisa added a commit that referenced this pull request Apr 20, 2022
Supports pagination and filtering for the web UI
Part of RFD 55
@webvictim webvictim mentioned this pull request Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants