-
Notifications
You must be signed in to change notification settings - Fork 24
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(bulk-import): adding sorting based on column in /imports #178
feat(bulk-import): adding sorting based on column in /imports #178
Conversation
Changed Packages
|
54ca224
to
ba86c46
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @its-mitesh-kumar, thanks for picking up also the backend work.
I added some comments, that IMHO we should remove the sorting in our backend, and add sorting params to the request calls for GitHub / Catalog API calls.
Let me know if I miss something and its not possible for some reason. And let the backend team and me know if you want continue with this.
workspaces/bulk-import/plugins/bulk-import-backend/src/service/handlers/import/bulkImports.ts
Outdated
Show resolved
Hide resolved
workspaces/bulk-import/plugins/bulk-import-backend/src/service/handlers/import/bulkImports.ts
Outdated
Show resolved
Hide resolved
workspaces/bulk-import/plugins/bulk-import-backend/src/helpers/utils.ts
Outdated
Show resolved
Hide resolved
workspaces/bulk-import/plugins/bulk-import-backend/src/schema/openapi.yaml
Outdated
Show resolved
Hide resolved
workspaces/bulk-import/plugins/bulk-import-backend/src/helpers/utils.ts
Outdated
Show resolved
Hide resolved
ba86c46
to
ea7ffa3
Compare
@christoph-jerolimov @dzemanov I have resolved the comments , Please re-review it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with all sort values, on the backend and frontend. Works as expected except for when sortColumn
is not specified. Please run prettier
to fix failing tests :).
workspaces/bulk-import/plugins/bulk-import-backend/src/service/handlers/import/bulkImports.ts
Outdated
Show resolved
Hide resolved
workspaces/bulk-import/plugins/bulk-import-backend/src/schema/openapi.yaml
Show resolved
Hide resolved
Also, please add changeset. |
workspaces/bulk-import/plugins/bulk-import-backend/src/service/handlers/import/bulkImports.ts
Show resolved
Hide resolved
322faef
to
de3defc
Compare
@dzemanov I have resolved all the comments , CI job failing please try prettier:fix and push it from your system , its not getting resolve from end . |
@its-mitesh-kumar it is failing on type check in |
workspaces/bulk-import/plugins/bulk-import-backend/src/schema/openapi.yaml
Outdated
Show resolved
Hide resolved
workspaces/bulk-import/plugins/bulk-import-backend/src/schema/openapi.yaml
Outdated
Show resolved
Hide resolved
5aacddc
to
817b20a
Compare
@dzemanov Now CI is failing due to prettier issue |
workspaces/bulk-import/plugins/bulk-import-backend/src/generated/openapi.d.ts
Outdated
Show resolved
Hide resolved
cc41673
to
093e4f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@its-mitesh-kumar Thank you for all the changes! Enum key repositoryName
can be now safely deleted. Tested again with the changes and works fine except for one bug I found, but not sure it it is related to this PR:
2025-01-13.10-40-16.mp4
Co-authored-by: Dominika Zemanovicova <[email protected]>
@dzemanov ! Quality gate is failing due to its considering the objects of locationUrls as duplicate of each other . |
/lgtm |
@christoph-jerolimov @debsmita1 can we get this merge ? |
When I click on a column to change the order, it triggers the API with the current column name and order along with the previous column name and order Screen.Recording.2025-01-17.at.1.04.27.PM.mov |
@debsmita1 , That's happening due to some odd behavior of useQuery , What I can see if we updating value of any key of queryKey , its recalling queryFunction once with old data and once with new value . We can fix this later , I have raised bug for the same . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
@its-mitesh-kumar Could you resolve this #178 (comment) |
Quality Gate failedFailed conditions |
Resolves : https://issues.redhat.com/browse/RHIDP-5295