Remove free text search on contextURL #8503

laushinka · 2022-03-01T12:50:48Z

Description

This change disables the free text search, and encourages searching on workspace IDs.
It's a minimum change for now - there will be follow-up tasks to improve the search[1][2].
It also includes drive-by changes of removing whitespace from searches[1].

Related Issue(s)

Fixes #8453
Fixes #8454

How to test

Go to https://lm-search-ws-8453.staging.gitpod-dev.com/admin/workspaces
Search part of a contextURL - it should not work.

Release Notes

Free text search on workspace admin dashboard is not enabled anymore.

Documentation

components/dashboard/src/admin/WorkspacesSearch.tsx

codecov · 2022-03-01T12:58:37Z

Codecov Report

Merging #8503 (8992b7c) into main (a4dbc1a) will decrease coverage by 21.64%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             main    #8503       +/-   ##
===========================================
- Coverage   32.82%   11.17%   -21.65%     
===========================================
  Files          33       18       -15     
  Lines        4753      993     -3760     
===========================================
- Hits         1560      111     -1449     
+ Misses       3075      880     -2195     
+ Partials      118        2      -116

Flag	Coverage Δ
components-gitpod-cli-app	`11.17% <ø> (ø)`
components-local-app-app-darwin-amd64	`?`
components-local-app-app-darwin-arm64	`?`
components-local-app-app-linux-amd64	`?`
components-local-app-app-linux-arm64	`?`
components-local-app-app-windows-386	`?`
components-local-app-app-windows-amd64	`?`
components-local-app-app-windows-arm64	`?`
components-ws-manager-app	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
components/ws-manager/pkg/manager/annotations.go
...s/ws-manager/pkg/manager/internal/grpcpool/pool.go
components/ws-manager/pkg/manager/manager.go
components/ws-manager/pkg/clock/clock.go
components/ws-manager/pkg/manager/create.go
components/local-app/pkg/auth/auth.go
components/ws-manager/pkg/manager/manager_ee.go
components/ws-manager/pkg/manager/probe.go
...omponents/ws-manager/pkg/manager/pod_controller.go
components/ws-manager/pkg/manager/imagespec.go
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4dbc1a...8992b7c. Read the comment docs.

gtsiolis · 2022-03-01T20:52:33Z

components/dashboard/src/admin/WorkspacesSearch.tsx

@@ -104,7 +96,9 @@ export function WorkspaceSearch(props: Props) {
                            <path fillRule="evenodd" clipRule="evenodd" d="M6 2a4 4 0 100 8 4 4 0 000-8zM0 6a6 6 0 1110.89 3.477l4.817 4.816a1 1 0 01-1.414 1.414l-4.816-4.816A6 6 0 010 6z" fill="#A8A29E" />
                        </svg>
                    </div>
-                    <input type="search" placeholder="Search Workspaces" onKeyDown={(ke) => ke.key === 'Enter' && search() } onChange={(v) => { setQueryTerm(v.target.value) }} />
+                    <input type="search" placeholder="Search Workspace IDs or Instance IDs" style={{width: "20rem"}}


question: Shall we go with one of these here based on #8453 (comment)? Cc @geropl

workspaceId 👍

@geropl Like this?

laushinka · 2022-03-01T23:53:30Z

Thank you for taking a look @geropl @gtsiolis 🙏🏽 It's now ready for actual review (not a draft anymore).

gtsiolis · 2022-03-02T09:31:59Z

Looking at this now! 👀

gtsiolis

Thanks, @laushinka!

Regarding the input placeholder question in #8503 (comment):

I think @geropl was only suggesting to go with the workspaceId option instead of the instanceId as described in #8453 (comment) but sounds ok to keep the friendlier placeholder.

gtsiolis · 2022-03-02T09:29:36Z

components/dashboard/src/admin/WorkspacesSearch.tsx

@@ -104,7 +96,9 @@ export function WorkspaceSearch(props: Props) {
                            <path fillRule="evenodd" clipRule="evenodd" d="M6 2a4 4 0 100 8 4 4 0 000-8zM0 6a6 6 0 1110.89 3.477l4.817 4.816a1 1 0 01-1.414 1.414l-4.816-4.816A6 6 0 010 6z" fill="#A8A29E" />
                        </svg>
                    </div>
-                    <input type="search" placeholder="Search Workspaces" onKeyDown={(ke) => ke.key === 'Enter' && search() } onChange={(v) => { setQueryTerm(v.target.value) }} />
+                    <input type="search" placeholder="Search Workspace IDs"


thought: I still think that long-term using more descriptive placeholders like Search by name as described in the last point in #8453 (comment) could be better. The ID, besides being a technical term, it's known within the team building the product but could be scary or ignored by users. We also scarcely reference the term workspace or instance ID in our docs.

suggestion: However, let's go with the current change as this looks obviously good and helpful and improve this if needed in a future iteration. ✔️

We also scarcely reference the term workspace or instance ID in our docs.

Initially I thought that Admins surely know that those are workspace IDs, but I realize that that was my biased assumption and the fact that we don't reference the term ID anywhere makes me see your point.
Let's see how this goes - especially when self-hosted users give us feedback, and yes we can improve this anytime. 🙏🏽

gtsiolis · 2022-03-02T09:31:45Z

components/dashboard/src/admin/WorkspacesSearch.tsx

-                    <input type="search" placeholder="Search Workspaces" onKeyDown={(ke) => ke.key === 'Enter' && search() } onChange={(v) => { setQueryTerm(v.target.value) }} />
+                    <input type="search" placeholder="Search Workspace IDs"
+                        onKeyDown={(ke) => ke.key === 'Enter' && search() }
+                        onChange={(v) => { setQueryTerm((v.target.value).trim()) }} />
                </div>
                <button disabled={searching} onClick={search}>Search</button>


question: Although out of the scope of the changes in this PR, what do you think of dropping this search button here and other pages within the admin dashboard?

thinking When there is no button, would the expectation be that the search result gets updated with every key change? Or would the user be expected to hit Enter?

thought: There are cases where searching or filtering on key change is valid and useful like projects or members which usually are a finite and small data set. However, it sounds optimal to not search on key change when filtering large datasets like instance users, workspaces, etc but require enter to perform the search. 💭

Thinking out loud, we added the search button back in #3723 because we require the enter key to perform a search here but seems harmless to drop the button. Your call, feel free to move this discussion in a follow up issue. 🏓

geropl

Code LGTM and tested: works and looks nice! 👍

roboquat added release-note do-not-merge/work-in-progress size/M labels Mar 1, 2022

laushinka commented Mar 1, 2022

View reviewed changes

components/dashboard/src/admin/WorkspacesSearch.tsx Show resolved Hide resolved

laushinka requested a review from geropl March 1, 2022 12:52

gtsiolis reviewed Mar 1, 2022

View reviewed changes

Remove free text search on contextURL

8992b7c

laushinka force-pushed the lm/search-ws-8453 branch from 59f531c to 8992b7c Compare March 1, 2022 23:37

laushinka marked this pull request as ready for review March 1, 2022 23:44

laushinka requested a review from a team March 1, 2022 23:44

roboquat removed the do-not-merge/work-in-progress label Mar 1, 2022

github-actions bot added the team: webapp Issue belongs to the WebApp team label Mar 1, 2022

laushinka requested a review from gtsiolis March 1, 2022 23:53

gtsiolis approved these changes Mar 2, 2022

View reviewed changes

geropl approved these changes Mar 2, 2022

View reviewed changes

roboquat merged commit 31efabc into main Mar 2, 2022

roboquat deleted the lm/search-ws-8453 branch March 2, 2022 11:04

laushinka mentioned this pull request Mar 3, 2022

Workspaces list under user fetches all workspaces #8561

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove free text search on contextURL #8503

Remove free text search on contextURL #8503

laushinka commented Mar 1, 2022 •

edited

Loading

codecov bot commented Mar 1, 2022 •

edited

Loading

gtsiolis Mar 1, 2022

geropl Mar 2, 2022

laushinka Mar 2, 2022

laushinka commented Mar 1, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Mar 2, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment

gtsiolis Mar 2, 2022

laushinka Mar 2, 2022

gtsiolis Mar 2, 2022

laushinka Mar 2, 2022

gtsiolis Mar 2, 2022

geropl left a comment

Remove free text search on contextURL #8503

Remove free text search on contextURL #8503

Conversation

laushinka commented Mar 1, 2022 • edited Loading

Description

Related Issue(s)

How to test

Release Notes

Documentation

codecov bot commented Mar 1, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laushinka commented Mar 1, 2022 • edited by werft-gitpod-dev-com bot Loading

gtsiolis commented Mar 2, 2022 • edited by werft-gitpod-dev-com bot Loading

gtsiolis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geropl left a comment

Choose a reason for hiding this comment

laushinka commented Mar 1, 2022 •

edited

Loading

codecov bot commented Mar 1, 2022 •

edited

Loading

laushinka commented Mar 1, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Mar 2, 2022 •

edited by werft-gitpod-dev-com bot

Loading