Add support for editing polymorphic fields #984

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.

Sign up for GitHub

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

Jump to bottom

Closed

pedantic-git wants to merge 3 commits into thoughtbot:master from fishpercolator:edit_polymorphic

Contributor

pedantic-git commented Sep 28, 2017

This PR allows users to edit Field::Polymorphic fields (previously they were just told this was not possible).

The <select> is a grouped select, populated using the contents of zero-or-more model classes specified in the :classes option.

I appreciate I haven't really added much in the way of testing for this, but I think it's comparable with the amount of testing the other complex built-in types already have (e.g. Field::HasMany).

The one place in the core that's changed here and might be controversial, is that #resource_params now auto-converts anything that looks like a GlobalID. This is necessary for a polymorphic field without JavaScript as GlobalIDs are the only IDs that work across multiple models. Let me know if you need this implementing a different way.


          Add support for editing polymorphic fields

6c8f1b4

houndci-bot reviewed

View reviewed changes

spec/lib/fields/polymorphic_spec.rb Outdated

+                describe '#selected_global_id' do
+                  it "returns the global ID of the data" do
+                    item = double('SomeModel', to_global_id: "gid://myapp/SomeModel/1")

houndci-bot Sep 28, 2017

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

spec/lib/fields/polymorphic_spec.rb Outdated

@@ @@ -37,4 +38,16 @@ def display_resource(*) @@
                     end
                   end
                 end
+                describe '#selected_global_id' do

houndci-bot Sep 28, 2017

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

spec/lib/fields/polymorphic_spec.rb Outdated

@@ @@ -37,4 +38,16 @@ def display_resource(*) @@
                     end
                   end
                 end

houndci-bot Sep 28, 2017

Trailing whitespace detected.

lib/administrate/field/polymorphic.rb Outdated

+                    def candidate_resources(klass)
+                      order ? klass.order(order) : klass.all
+                    end

houndci-bot Sep 28, 2017

Trailing whitespace detected.

lib/administrate/field/polymorphic.rb Outdated

+                    def order
+                      @_order ||= options.delete(:order)
+                    end

houndci-bot Sep 28, 2017

Trailing whitespace detected.

lib/administrate/field/polymorphic.rb Outdated

+                    def self.permitted_attribute(attr)
+                      attr
+                    end

houndci-bot Sep 28, 2017

Trailing whitespace detected.

lib/administrate/field/polymorphic.rb Outdated

+                        end ]
+                      end
+                    end

houndci-bot Sep 28, 2017

Trailing whitespace detected.

lib/administrate/field/polymorphic.rb Outdated

+                      classes.map do |klass|
+                        [ klass.to_s, candidate_resources(klass).map do |resource|
+                          [display_candidate_resource(resource), resource.to_global_id]
+                        end ]

houndci-bot Sep 28, 2017

Space inside square brackets detected.

lib/administrate/field/polymorphic.rb Outdated

+                  class Polymorphic < BelongsTo
+                    def associated_resource_grouped_options
+                      classes.map do |klass|
+                        [ klass.to_s, candidate_resources(klass).map do |resource|

houndci-bot Sep 28, 2017

Space inside square brackets detected.

app/controllers/administrate/application_controller.rb Outdated

@@ @@ -120,7 +120,7 @@ def resource_includes @@
                   def resource_params
                     params.require(resource_class.model_name.param_key).
-                      permit(dashboard.permitted_attributes)
+                      permit(dashboard.permitted_attributes).transform_values {|v|  v.respond_to?(:starts_with?) && v.starts_with?("gid://") ? GlobalID::Locator.locate(v) : v }

houndci-bot Sep 28, 2017

Space between { and | missing.
Line is too long. [162/80]

houndci-bot reviewed

View reviewed changes

app/controllers/administrate/application_controller.rb Outdated

-                      permit(dashboard.permitted_attributes)
+                      permit(dashboard.permitted_attributes).
+                      transform_values do |v|
+                        (v.respond_to?(:starts_with?) && v.starts_with?("gid://")) ?

houndci-bot Sep 28, 2017

Avoid multi-line ternary operators, use if or unless instead.
Omit parentheses for ternary conditions.

pedantic-git force-pushed the edit_polymorphic branch from d772304 to 0956238 Compare

September 28, 2017 10:32

pedantic-git added 2 commits

September 28, 2017 11:32


          Address @houndci-bot concerns


          Don't use same method name as method in parent class with different s…

554942e

…ignature

ewilliam commented Oct 18, 2017

any plans to implement this? or any polymorphic field solution?

pablobm mentioned this pull request

Add support for editing polymorphic fields - Revisited #1020

Merged

Collaborator

pablobm commented Nov 1, 2017

@pedantic-git - Thank you for your PR. I've been having a look at this and I like the general direction. You are right that the implementation of #resource_params is a pickle, so I gave it some thought. I think I have with a good solution now, and I have implemented it on a separate PR #1020, on top of your work.

The main change is in how to deal with this global id. Instead of relying on the global id only, I split the field in two:

my_field[value]: the global id
my_field[type]: literally "polymorphic"

Then #resource_params can look at the param values and treat them appropriately. If it's a simple value such a string, it does nothing special. However, if it comes in this value/type form, we treat it as polymorphic, reading the global id.

Something to note is that the globalid gem is not a default in old versions of Rails. Therefore I had to add it as a dependency. I could have done without it, but I liked it when I saw it on your code.

Additionally, I added some tests. I may add a couple more, but this does pretty well for now.

Closing this as it's superseded by the new PR.

pablobm closed this

pablobm added a commit that referenced this pull request


          Add support for editing polymorphic fields - Revisited (#1020)

8329b21

This adds support for editing polymorphic fields. Props go to @pedantic-git, on whose work this is based. This PR supersedes #984.

An important aspect of this PR is how it handles the polymorphic value. At heart, polymorphic values are split in two: an id and a type. Using [the `globalid` gem](https://github.com/rails/globalid) we can obtain a string that represents both. However, we still need to tell that this string is indeed a globalid and not a literal value when we get to `Administrate::ApplicationController#resource_params`. I handle this by splitting the field in two:

  * `my_field[value]`: the global id, which includes both type and instance id.
  * `my_field[type]`: literally `"Administrate::Field::Polymorphic"`

Then `#resource_params` can look at the param values and treat them appropriately. If it's a simple value such a string, it does nothing special. However, if it comes in this value/type form, we treat it as polymorphic, reading the global id.

svqualitydev pushed a commit to svqualitydev/admin-cms that referenced this pull request


          Add support for editing polymorphic fields - Revisited (#1020)

f900d6c

This adds support for editing polymorphic fields. Props go to @pedantic-git, on whose work this is based. This PR supersedes thoughtbot/administrate#984.

An important aspect of this PR is how it handles the polymorphic value. At heart, polymorphic values are split in two: an id and a type. Using [the `globalid` gem](https://github.com/rails/globalid) we can obtain a string that represents both. However, we still need to tell that this string is indeed a globalid and not a literal value when we get to `Administrate::ApplicationController#resource_params`. I handle this by splitting the field in two:

  * `my_field[value]`: the global id, which includes both type and instance id.
  * `my_field[type]`: literally `"Administrate::Field::Polymorphic"`

Then `#resource_params` can look at the param values and treat them appropriately. If it's a simple value such a string, it does nothing special. However, if it comes in this value/type form, we treat it as polymorphic, reading the global id.

KingTiger001 added a commit to KingTiger001/admin-Rails-project that referenced this pull request


          Add support for editing polymorphic fields - Revisited (#1020)

This adds support for editing polymorphic fields. Props go to @pedantic-git, on whose work this is based. This PR supersedes thoughtbot/administrate#984.

An important aspect of this PR is how it handles the polymorphic value. At heart, polymorphic values are split in two: an id and a type. Using [the `globalid` gem](https://github.com/rails/globalid) we can obtain a string that represents both. However, we still need to tell that this string is indeed a globalid and not a literal value when we get to `Administrate::ApplicationController#resource_params`. I handle this by splitting the field in two:

  * `my_field[value]`: the global id, which includes both type and instance id.
  * `my_field[type]`: literally `"Administrate::Field::Polymorphic"`

Then `#resource_params` can look at the param values and treat them appropriately. If it's a simple value such a string, it does nothing special. However, if it comes in this value/type form, we treat it as polymorphic, reading the global id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet