Add support for binary_id search fields
This PR adds support for binary_id search fields in Torch.
Changes:
- Added a new
filter_uuidfunction toTorch.FilterViewthat only supports the "equals" filter type for binary_id fields, as lower(binary_id) is not supported by PostgreSQL. - Added a test for the new
filter_uuidfunction. - Updated the pagination module to handle binary_id fields properly.
- Updated the template generator to use the
filter_uuidfunction for binary_id fields.
Fixes #256
[!IMPORTANT]
Review skipped
Bot user detected.
To trigger a single review, invoke the
@coderabbitai reviewcommand.You can disable this status message by setting the
reviews.review_statustofalsein the CodeRabbit configuration file.
🪧 Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
I pushed a fix in commit <commit_id>, please review it.Explain this complex logic.Open a follow-up GitHub issue for this discussion.
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. Examples:@coderabbitai explain this code block.@coderabbitai modularize this function.
- PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.@coderabbitai read src/utils.ts and explain its main purpose.@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.@coderabbitai help me debug CodeRabbit configuration file.
Support
Need help? Join our Discord community for assistance with any issues or questions.
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.
CodeRabbit Commands (Invoked using PR comments)
@coderabbitai pauseto pause the reviews on a PR.@coderabbitai resumeto resume the paused reviews.@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository.@coderabbitai full reviewto do a full review from scratch and review all the files again.@coderabbitai summaryto regenerate the summary of the PR.@coderabbitai generate sequence diagramto generate a sequence diagram of the changes in this PR.@coderabbitai resolveresolve all the CodeRabbit review comments.@coderabbitai configurationto show the current CodeRabbit configuration for the repository.@coderabbitai helpto get help.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
CodeRabbit Configuration File (.coderabbit.yaml)
- You can programmatically configure CodeRabbit by adding a
.coderabbit.yamlfile to the root of your repository. - Please see the configuration documentation for more information.
- If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation:
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
Documentation and Community
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
✅ Fixed the failing check in this commit.
The issue was a mismatch in the doctest for the filter_uuid function. The doctest expected an option without the selected attribute, but the actual implementation adds the selected attribute to the option. I updated the doctest to match the actual implementation.
This should resolve the failing tests across all Elixir/OTP versions.
💻 View my work • React 👍 or 👎
I've addressed the comments regarding the handling of :binary_id fields in the filtering system. Here's what I've done:
-
Added a new
build_filtrex_configs/2clause that maps the:idcollection (which includes:binary_idfields) to the:textfilter type. This ensures that binary_id fields can be properly filtered using text-based comparisons, which is appropriate for UUID strings. -
The existing implementation already correctly maps
:binary_idfields to the:idcollection in thecollect_attributes_by_type/3function, and now with the addedbuild_filtrex_configs/2clause, the full pipeline for handling binary_id fields is complete.
These changes ensure that binary_id fields are properly categorized, collected, and configured for filtering in the admin interface.
✅ Fixed the failing check in this commit.
The issue was in the doctest for the filter_uuid function. The doctest had incorrect escaping of quotes in the expected output string. I fixed the escaping to match the actual output format of the function.
This should resolve the failing tests across all Elixir/OTP versions.
💻 View my work • React 👍 or 👎
You're absolutely right @cpjolicoeur! I've updated the code to map :binary_id directly to :text instead of :id.
The previous implementation had a circular logic issue:
:idtype is mapped to:number(as seen in the code above)- Then we were mapping
:binary_idto:id, which would ultimately map to:number
This was incorrect since binary_id/UUID fields are string-based and should be treated as text for filtering purposes. The updated implementation now correctly maps :binary_id directly to :text type, which is more appropriate for UUID string comparisons.
Thanks for catching this issue!
I've addressed Craig's latest comment about the potential circular logic in the type mapping for :binary_id fields. Here's what I've updated:
-
Changed the implementation to map
:binary_idfields directly to the:textcollection incollect_attributes_by_type/3instead of the:idcollection. This is more appropriate since binary IDs (UUIDs) are text-based values. -
Removed the unnecessary
build_filtrex_configs/2clause for:idsince we're now using the existing:textmapping pipeline.
This approach simplifies the implementation and avoids any confusion or circular logic where :id was being mapped to :number in one place and then :id collection was being mapped to :text filter type in another. The updated implementation is cleaner and more consistent with how other field types are handled in the codebase.