-
Notifications
You must be signed in to change notification settings - Fork 64
Refactor fields handling #1271
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
Refactor fields handling #1271
Conversation
krns
left a comment
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.
I also checked insert and update and noticed that update already gets the fields passed, insert does not. Is that on purpose?
def insert(item, attrs, assigns, live_resource, opts) do
def update(item, attrs, fields, assigns, live_resource, opts \\ [])
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.
Pull Request Overview
This PR refactors fields handling in Backpex by introducing a new centralized fields/3 function that combines field validation, action-based filtering, and permission-based filtering into a single operation. The refactoring updates the resource and adapter APIs to pass fields as an explicit parameter rather than computing them multiple times throughout the codebase.
Key changes:
- Replaced
validated_fields/1withfields/3that handles validation, action filtering, and permission checking - Updated Resource and Adapter function signatures to include fields parameter
- Simplified field access patterns across LiveResource modules
- Removed
:resource_actionfrom field visibility options
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/backpex/resource.ex | Updated function signatures to accept fields parameter and removed internal field computation |
| lib/backpex/live_resource.ex | Added new fields/3 function and refactored field filtering logic |
| lib/backpex/live_resource/show.ex | Simplified field assignment using new centralized approach |
| lib/backpex/live_resource/index.ex | Updated to use pre-computed fields from assigns |
| lib/backpex/live_resource/form.ex | Removed duplicate field assignment logic |
| lib/backpex/adapters/ecto.ex | Updated adapter functions to accept fields parameter |
| lib/backpex/adapters/ash.ex | Updated adapter functions to accept fields parameter |
| lib/backpex/adapter.ex | Enhanced callback documentation with proper type specifications |
Comments suppressed due to low confidence (1)
lib/backpex/resource.ex:1
- The parameters are in the wrong order. According to the function signature at line 265, it should be
Resource.update(item, params, fields, socket.assigns, live_resource, opts)but heresocket.assignsandfieldsare swapped.
defmodule Backpex.Resource do
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Todos