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

Plugins: Add fuzzy search to plugins catalogue #81001

Merged
merged 17 commits into from Feb 14, 2024

Conversation

Ukochka
Copy link
Contributor

@Ukochka Ukochka commented Jan 22, 2024

What is this feature?
Add keywords to plugins from frontend side
Fuzzy search for plugin catalog. User can use name, type, orgName, keywords or Id together to find plugin

Why do we need this feature?
Fuzzy search helps users to find results in more convenient way. User can type name, type, keyword and find what is needed. We add this search into our Plugin catalog.

Who is this feature for?
Everyone who uses plugins from plugins catalog

Which issue(s) does this PR fix?:

Fixes #80699

@Ukochka Ukochka self-assigned this Jan 29, 2024
@Ukochka Ukochka changed the title WIP add fuzzysearch to plugins catalog Plugins: add fuzzysearch to plugins catalog Jan 29, 2024
@Ukochka Ukochka marked this pull request as ready for review January 29, 2024 09:48
@Ukochka Ukochka requested review from a team as code owners January 29, 2024 09:48
@Ukochka Ukochka requested review from oshirohugo, marefr, xnyo and academo and removed request for a team January 29, 2024 09:48
Copy link
Member

@xnyo xnyo 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! 🚀 Took it up for a spin and it works great! I have left some comments but the functionality is good.

There are also some integration tests failing in the backend due to adding keywords to the plugin, and some frontend tests are failing too:

https://drone.grafana.net/grafana/grafana/157266

A future improvement could be sorting the fuzzy search results as well using the ufuzzy library, but that's up for discussion and can be left for a follow-up PR 👍

public/app/features/plugins/admin/pages/Browse.tsx Outdated Show resolved Hide resolved
public/app/features/plugins/admin/helpers.ts Outdated Show resolved Hide resolved
@Ukochka Ukochka requested a review from a team as a code owner February 5, 2024 11:29
@Ukochka Ukochka requested review from academo and xnyo February 5, 2024 11:29
package.json Outdated
@@ -251,7 +251,7 @@
"@grafana/sql": "workspace:*",
"@grafana/ui": "workspace:*",
"@kusto/monaco-kusto": "^7.4.0",
"@leeoniya/ufuzzy": "1.0.14",
"@leeoniya/ufuzzy": "^1.0.14",
Copy link
Member

Choose a reason for hiding this comment

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

Is this change from exact dependency to latest version intended?

}, {});
}
export function filterByKeyword(plugins: CatalogPlugin[], query: string) {
const dataArray = Object.values(getPluginDetailsForFuzzySearch(plugins));
Copy link
Member

Choose a reason for hiding this comment

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

Since you are using only the values from the object that getPluginDetailsForFuzzySearch returns and getPluginDetailsForFuzzySearch is not used anywhere else, you can instead return directly an array from there instead of having to do Object.values here

export function filterByKeyword(plugins: CatalogPlugin[], query: string) {
const dataArray = Object.values(getPluginDetailsForFuzzySearch(plugins));
let opts = {};
let uf = new uFuzzy(opts);
Copy link
Member

Choose a reason for hiding this comment

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

you can pass {} directly to the constructor and save declaring the opts variable since we don't use it for anything else.


if (keyword && !fieldsToSearchIn.some((f) => f.includes(keyword))) {
if (keyword && filteredPluginIds != null && !filteredPluginIds.includes(plugin.id)) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: here we already know filteredPluginIds is not null because is checked in the previous condition, so you can remove the check here.

@Ukochka Ukochka requested a review from academo February 12, 2024 12:33
@Ukochka Ukochka changed the title Plugins: add fuzzysearch to plugins catalog Plugins: Add fuzzy search to plugins catalogue Feb 12, 2024
@Ukochka Ukochka requested a review from a team as a code owner February 14, 2024 11:13
@Ukochka Ukochka requested review from zserge, diegommm and undef1nd and removed request for a team February 14, 2024 11:13
Copy link
Member

@academo academo left a comment

Choose a reason for hiding this comment

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

Nice work!

I tested it locally and it is working as expected.

This is without fuzy search, with words: "Grafana amazon"

image

This is with fuzzy search

image

@Ukochka Ukochka merged commit 9dcb780 into main Feb 14, 2024
16 checks passed
@Ukochka Ukochka deleted the fuzzy-search-for-plugin-catalog branch February 14, 2024 13:30
Ukochka added a commit that referenced this pull request Feb 14, 2024
* WIP add fuzzysearch to plugins catalog

* Add keywords to the plugins listing output

* add fuzzy search to plugin catalog, add keywords to plugins at frontend side

* refactor fuzzysearch function after review

* review changes

* change the version of uFuzzy library

* change reduce result object in getPluginDetailsForFuzzySearch

* fix yarn lock error

* fix helpers tests

* fix frontend searching test

* fix frontend linting issues

* fix tests

---------

Co-authored-by: Esteban Beltran <esteban@academo.me>
Co-authored-by: Giuseppe Guerra <giuseppe@guerra.in>
@aangelisc aangelisc modified the milestones: 10.4.x, 10.4.0 Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Allow the grafana plugin admin catalogue page to fuzzy search plugins by name, types and keywords
4 participants