-
-
Notifications
You must be signed in to change notification settings - Fork 242
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
Feature: Display additional fields when attaching a record to another #3048
Changes from 37 commits
670ba0a
f79177e
689d72a
c7ae36c
fd1f441
f6cfb9e
a60de9f
abdd39d
37f9c60
b6b0e85
31dd758
77e2988
cfd4090
ec2f378
109e0f7
30f37cb
950a88e
78bbb18
7e909a1
73d8829
1993546
1a1ec9e
6751d8a
a748541
8d057bf
4412710
edc6f5a
76a01ff
90a64f9
19d5da0
c2aed1b
76a7cc2
12c96e8
981ecee
c1a463a
a40e604
83ffa20
0a8a670
8ce929d
4763aac
e16142b
a00b433
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -12,6 +12,7 @@ class AssociationsController < BaseController | |||
before_action :set_attachment_class, only: [:show, :index, :new, :create, :destroy] | ||||
before_action :set_attachment_resource, only: [:show, :index, :new, :create, :destroy] | ||||
before_action :set_attachment_record, only: [:create, :destroy] | ||||
before_action :set_attach_fields, only: [:new, :create] | ||||
before_action :authorize_index_action, only: :index | ||||
before_action :authorize_attach_action, only: :new | ||||
before_action :authorize_detach_action, only: :destroy | ||||
|
@@ -67,6 +68,7 @@ def create | |||
notice: t("avo.attachment_class_attached", attachment_class: @related_resource.name) | ||||
} | ||||
else | ||||
flash[:error] = t("avo.attachment_failed", attachment_class: @related_resource.name) | ||||
format.turbo_stream { | ||||
render turbo_stream: turbo_stream.append("alerts", partial: "avo/partials/all_alerts") | ||||
} | ||||
|
@@ -78,7 +80,9 @@ def create_association | |||
association_name = BaseResource.valid_association_name(@record, association_from_params) | ||||
|
||||
perform_action_and_record_errors do | ||||
if has_many_reflection? | ||||
if through_reflection? && additional_params.present? | ||||
new_join_record.save | ||||
elsif has_many_reflection? || through_reflection? | ||||
@record.send(association_name) << @attachment_record | ||||
else | ||||
@record.send(:"#{association_name}=", @attachment_record) | ||||
|
@@ -90,7 +94,7 @@ def create_association | |||
def destroy | ||||
association_name = BaseResource.valid_association_name(@record, @field.for_attribute || params[:related_name]) | ||||
|
||||
if @reflection.instance_of? ActiveRecord::Reflection::ThroughReflection | ||||
if through_reflection? | ||||
join_record.destroy! | ||||
elsif has_many_reflection? | ||||
@record.send(association_name).delete @attachment_record | ||||
|
@@ -190,5 +194,36 @@ def has_many_reflection? | |||
ActiveRecord::Reflection::HasAndBelongsToManyReflection | ||||
] | ||||
end | ||||
|
||||
def through_reflection? | ||||
@reflection.instance_of? ActiveRecord::Reflection::ThroughReflection | ||||
end | ||||
|
||||
def additional_params | ||||
params[:fields].permit(@attach_fields.map(&:id)) | ||||
Paul-Bob marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
end | ||||
|
||||
def set_attach_fields | ||||
@attach_fields = if @field.attach_fields.present? | ||||
Avo::FieldsExecutionContext.new(target: @field.attach_fields) | ||||
.detect_fields | ||||
.items_holder | ||||
.items | ||||
end | ||||
end | ||||
|
||||
def new_join_record | ||||
@resource.fill_record( | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @adrianthedev We use Line 448 in 90a64f9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, but let's apply the def additional_params
params[:fields].permit(@attach_fields.map(&:id))
end There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okayy. Got it 😄 |
||||
@reflection.through_reflection.klass.new, | ||||
additional_params.merge( | ||||
{ | ||||
source_foreign_key => @attachment_record.id, | ||||
through_foreign_key => @record.id | ||||
} | ||||
), | ||||
fields: @attach_fields, | ||||
extra_params: [source_foreign_key, through_foreign_key] | ||||
) | ||||
end | ||||
end | ||||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
module Avo | ||
class FieldsExecutionContext < Avo::ExecutionContext | ||
include Avo::Concerns::HasItems | ||
|
||
def detect_fields | ||
self.items_holder = Avo::Resources::Items::Holder.new(parent: self) | ||
|
||
instance_exec(&target) if target.present? && target.respond_to?(:call) | ||
|
||
self | ||
end | ||
end | ||
end |
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.
One more thing.
Let's see if this
permit
statement emits a red warning in the console for other params which are sent by the modal.I'd like to prevent that behavior. Some users when they have a problem with that actions, they will look at that and think that that is the cause of the problem.
So they are led astray.
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 think that behavior depends on the parent's app configuration and can be configured by
action_on_unpermitted_parameters
(check whole docs here)Would be great to have a flag to turn it off like:
params.permit(on_unnpermited_params: false, ...)