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(marketplace): add optional sorting, filtering, pagination to marketplace api #332

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

its-mitesh-kumar
Copy link
Contributor

Hey, I just made a Pull Request!

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

@rhdh-gh-app
Copy link

rhdh-gh-app bot commented Jan 27, 2025

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-marketplace-backend workspaces/marketplace/plugins/marketplace-backend patch v0.0.7
@red-hat-developer-hub/backstage-plugin-marketplace-common workspaces/marketplace/plugins/marketplace-common patch v0.0.7
@red-hat-developer-hub/backstage-plugin-marketplace workspaces/marketplace/plugins/marketplace patch v0.0.7

@its-mitesh-kumar its-mitesh-kumar force-pushed the feat/marketplace-plugin-pagination branch from f80409a to 9885613 Compare January 27, 2025 07:11
Signed-off-by: its-mitesh-kumar <[email protected]>
@its-mitesh-kumar its-mitesh-kumar marked this pull request as ready for review January 27, 2025 07:21
@its-mitesh-kumar its-mitesh-kumar changed the title feat(marketplace): add pagination feat(marketplace): add pagination , sorting and filtering in /plugins endpoint Jan 27, 2025
@its-mitesh-kumar
Copy link
Contributor Author

Resolves : https://issues.redhat.com/browse/RHIDP-5329
@christoph-jerolimov Can you have a look into this PR , I can update unit test once you confirm the functionality changes seems good to you .

Copy link
Member

@christoph-jerolimov christoph-jerolimov left a comment

Choose a reason for hiding this comment

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

Thanks! As discussed some improvement ideas :)

@its-mitesh-kumar
Copy link
Contributor Author

@karthikjeeyar @christoph-jerolimov I have implemented like
We can encode complex query parameter like fullTextFilter , filter and orderFields in frontend and decode it in the backend. Encoding in frontend can be like this . Let me know with this approach we may have any problem .

let search = {
  term: "Backstage",
  fields: ["metadata.description"], // mutiple fields we can add 
}
let orderFields = {
  field: "metadata.name",
  order: "asc",
} // can be array of object also in case of multiple fields
let filter = {
  "spec.type": "backend-plugin",
  "metadata.name": "search",
}
filter= encodeURIComponent(JSON.stringify(filter))
search=encodeURIComponent(JSON.stringify(search))
orderFields=encodeURIComponent(JSON.stringify(orderFields))
const endpoint=`${baseUrl}?fullTextFilter=${search}&filter=${filter}&orderFields=${orderFields}&limit=10&offset=10`

Comment on lines 51 to 58
const filter: EntityFilterQuery = query?.filter
? {
...(typeof query.filter === 'string'
? JSON.parse(query.filter)
: query.filter),
...RequiredFilter,
}
: RequiredFilter;
Copy link
Contributor

@karthikjeeyar karthikjeeyar Jan 31, 2025

Choose a reason for hiding this comment

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

You could reuse decodeFilterParams from /utils/decodeQueryParams.ts here.

return params;
};

export const convertSearchParamsToGetPluginsRequest = (
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it will be good to extend the generic encodeQueryParams function to handle more fields, Instead of having it's own version for GetPluginsRequest. This way we can reuse the function for other API like PluginLists, Packages etc.

Copy link
Member

@christoph-jerolimov christoph-jerolimov left a comment

Choose a reason for hiding this comment

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

Small type questions for now. We should also align that with the other utils. But its a good start so far. Please add unit tests for all utils.

Comment on lines 23 to 27
const DefaultOrderFields: EntityOrderQuery = [
{ field: 'metadata.name', order: 'asc' },
];
const DefaultLimit = 20;
const RequiredFilter = { kind: 'plugin' };

Choose a reason for hiding this comment

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

Don't know if this is needed. But please start consts with lower case characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have changed into lower case . Since we want to get plugins by default we need to pass { kind: 'plugin' } , in queryEntities . I wanted to get only 20 results by default .

Choose a reason for hiding this comment

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

Without a default limit it will return all entities? Let us not have a limit for now since the UI doesn't support pagination at the moment.

@its-mitesh-kumar its-mitesh-kumar force-pushed the feat/marketplace-plugin-pagination branch from 0704928 to d311406 Compare January 31, 2025 12:03
@christoph-jerolimov christoph-jerolimov changed the title feat(marketplace): add pagination , sorting and filtering in /plugins endpoint feat(marketplace): add optional sorting, filtering, pagination to marketplace api Jan 31, 2025
@its-mitesh-kumar its-mitesh-kumar force-pushed the feat/marketplace-plugin-pagination branch from cbe9678 to 1390685 Compare January 31, 2025 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants