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

Add support for filter operators #107

Merged
merged 3 commits into from
Jul 20, 2023

Conversation

AbegaM
Copy link
Collaborator

@AbegaM AbegaM commented Jul 17, 2023

Fixes #105

Purpose of PR

The purpose of this PR is to enhance the filtering feature of Soul. Currently, the _filters field is used for basic filtering, but it does not allow users to filter items by comparing them or by using comparison operators.

Modifications

To address this limitation, I have made modifications to the rows.js file in the core/src/controllers directory. With these changes, users can now filter items using comparison operators such as __lte, __gte, __lt, __gt, __eq, and __neq. With this improved filtering functionality, users can more effectively search and analyze data in the Soul application.

@AbegaM AbegaM requested a review from thevahidal July 17, 2023 17:52
@AbegaM
Copy link
Collaborator Author

AbegaM commented Jul 17, 2023

Hello @thevahidal Please review this PR and let me know if there is anything i have to improve

@@ -66,15 +75,23 @@ const listTableRows = async (req, res) => {
// will filter by name like '%John%' and age like '%20%'

const filters = _filters.split(',').map((filter) => {
const [field, value] = filter.split(':');
return { field, value };
let [key, value] = filter.split(':');
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like this to be more pessimistic, please @AbegaM . I'd like us to handle when a filter doesn't match our expected list, and throw a suitable error message.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@IanMayo Ah, I haven't considered that. I will modify the code to throw errors.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@thevahidal, I have added error handling to the feature request. Please review it and let me know if there is anything I can modify.

@AbegaM AbegaM requested a review from IanMayo July 18, 2023 09:02
Copy link
Collaborator

@IanMayo IanMayo left a comment

Choose a reason for hiding this comment

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

Have tested this functionality using calls from browser, and inspecting results.

Found issues with next/prev links, but these have been captured in #109

@thevahidal
Copy link
Owner

Thanks for the effort guys, it looks good to go

@thevahidal thevahidal merged commit a721df7 into main Jul 20, 2023
@IanMayo IanMayo deleted the 105_add_support_for_filter_operators branch July 20, 2023 09:04
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.

Add support for filter operators
3 participants