administrate icon indicating copy to clipboard operation
administrate copied to clipboard

POST to create through controller with different FK constraint than 'id' fails

Open lake-effect opened this issue 3 years ago • 8 comments

Result versus expectation

Following the docs we created a dashboard for the below model, which is the "N" in a 1:N relationship between an OwningModel (1) and an OwnedModel (N). The catch is that the OwnedModel is keyed (FK) on a unique ID that is not owning_model_id, it's constrained on special_owner_id.

So, as below, we have

    owning_model: Field::BelongsTo.with_options(
      class_name: "SomeModule::OwningModel",
      foreign_key: 'special_owner_id'
    ),

When we submit a new "create" through the below dashboard, we get an FK error:

ActiveRecord::InvalidForeignKey (PG::ForeignKeyViolation: ERROR:  insert or update on table "owned_model" violates foreign key constraint "fk_rails_e9b0191624"

2022-03-28T18:05:01.013857+00:00 app[web.1]: DETAIL:  Key (special_owner_id)=(20) is not present in table "owning_models".

2022-03-28T18:05:01.013858+00:00 app[web.1]: : INSERT INTO "owned_models" ("special_owner_id", "attr_1", "created_at", "updated_at") VALUES ('20', 2500, '2022-03-28 18:05:00.837807', '2022-03-28 18:05:00.837807') RETURNING "id"):

The '20' in that log corresponds to owning_model.id, which is actually 20, but the default controller method for create in the administrate controller is POSTing as above with (special_owner_id)=(20). We expected it to POST with (special_owner_id)=(SOME_SPECIAL_ID), and succeed.

Workaround

One workaround is to modify the OwnedModel's dashboard as follows:

    owning_model: Field::BelongsTo.with_options(
      class_name: "SomeModule::OwningModel",
      foreign_key: 'special_owner_id',
      primary_key: 'special_owner_id',
    ),

However, this is semantically incorrect (that isn't the primary key), and will probably confuse developers later.

Dashboard model

require "administrate/base_dashboard"

class OwnedModelDashboard < Administrate::BaseDashboard
  NAMESPACE = 'internal'
  # ATTRIBUTE_TYPES
  # a hash that describes the type of each of the model's fields.
  #
  # Each different type represents an Administrate::Field object,
  # which determines how the attribute is displayed
  # on pages throughout the dashboard.
  ATTRIBUTE_TYPES = {
    owning_model: Field::BelongsTo.with_options(
      class_name: "SomeModule::OwningModel",
      foreign_key: 'special_owner_id'
    ),
    id: Field::Number,
    attr_1: Field::Boolean,
    attr_2: Field::Number,
    created_at: Field::DateTime,
    updated_at: Field::DateTime,
  }.freeze

  # COLLECTION_ATTRIBUTES
  # an array of attributes that will be displayed on the model's index page.
  COLLECTION_ATTRIBUTES = [
    :owning_model,
    :attr_1,
    :attr_2,
  ].freeze

  # SHOW_PAGE_ATTRIBUTES
  # an array of attributes that will be displayed on the model's show page.
  SHOW_PAGE_ATTRIBUTES = [
    :owning_model,
    :id,
    :attr_1,
    :attr_2,
    :created_at,
    :updated_at,
  ].freeze

  # FORM_ATTRIBUTES
  # an array of attributes that will be displayed
  # on the model's form (`new` and `edit`) pages.
  FORM_ATTRIBUTES = [
    :owning_model,
    :attr_1,
    :attr_2,
  ].freeze

Environment

  • Rails 5
  • administrate 0.17

lake-effect avatar Mar 28 '22 18:03 lake-effect

Hello @lake-effect. Thank you for the thorough description. This is indeed an issue we were not aware of, and it should be fixed. To add insult to injury, our example app has an instance of this, and we still missed it.

I have put together a fix at this branch: https://github.com/thoughtbot/administrate/compare/main...pablobm:association-primary-key, would you be able to give it a whirl? Make sure not to use any of the options, as they are deprecated and in this specific case :primary_key won't be used.

pablobm avatar Apr 08 '22 07:04 pablobm

I'll take a look, thanks!

lake-effect avatar Apr 08 '22 17:04 lake-effect

@lake-effect - Would you be able to paste here your has_many and belongs_to declarations in your models? I'm checking that I'm not missing anything with my fix.

pablobm avatar Apr 09 '22 07:04 pablobm

@pablobm Here's the test I carried out:

Environment

Gemfile

# ...
gem 'administrate', git: 'https://github.com/pablobm/administrate.git', branch: 'association-primary-key'
# ...

Dashboard model (variation 1)

require "administrate/base_dashboard"

class OwnedModelDashboard < Administrate::BaseDashboard
  NAMESPACE = 'internal'
  # ATTRIBUTE_TYPES
  # a hash that describes the type of each of the model's fields.
  #
  # Each different type represents an Administrate::Field object,
  # which determines how the attribute is displayed
  # on pages throughout the dashboard.
  ATTRIBUTE_TYPES = {
    owning_model: Field::BelongsTo,
    id: Field::Number,
    attr_1: Field::Boolean,
    attr_2: Field::Number,
    created_at: Field::DateTime,
    updated_at: Field::DateTime,
  }.freeze

  # ...same as above

Dashboard model (variation 2)

require "administrate/base_dashboard"

class OwnedModelDashboard < Administrate::BaseDashboard
  NAMESPACE = 'internal'
  # ATTRIBUTE_TYPES
  # a hash that describes the type of each of the model's fields.
  #
  # Each different type represents an Administrate::Field object,
  # which determines how the attribute is displayed
  # on pages throughout the dashboard.
  ATTRIBUTE_TYPES = {
    owning_model: Field::BelongsTo.with_options(
      class_name: "SomeModule::OwningModel",
      foreign_key: 'special_owner_id'
    ),
    id: Field::Number,
    attr_1: Field::Boolean,
    attr_2: Field::Number,
    created_at: Field::DateTime,
    updated_at: Field::DateTime,
  }.freeze

  # ...same as above

Test steps

  1. bundle install, confirm administrate installed from correct source
  2. Visit http://localhost:3000/internal/owned_models/new
  3. Attempt to create new OwnedModel using dropdown containing correct list of OwningModels

Results

With both variations on the dashboard model above, the same FK error was thrown. Querying the OwningModel in rails console confirmed that the association ID POSTed as part of the INSERT was a PK for OwningModel, not the 'special_owner_id' that would have been valid.

lake-effect avatar Apr 11 '22 19:04 lake-effect

Sorry, I just saw this:

@lake-effect - Would you be able to paste here your has_many and belongs_to declarations in your models? I'm checking that I'm not missing anything with my fix.

Owning model

owning_model.rb

class SomeModule::OwningModel < ApplicationRecord

# ...

has_many :owned_models, primary_key: "special_owner_id", foreign_key: "special_owner_id", class_name: "OwnedModel"

# ...

Owned model

class OwnedModel < ApplicationRecord
  belongs_to :owning_model,
             primary_key: "special_owner_id", # primary key of associated object
             foreign_key: "special_owner_id", # foreign key used for the association
             class_name: "SomeModule::OwningModel"

# ...

lake-effect avatar Apr 11 '22 19:04 lake-effect

Thank you for the additional info :-)

One thing I'm noticing is that your associations are defined with class_name, primary_key and foreign_key, the same settings that work for you on Administrate. Indeed, the Administrate fields were named to be consistent with the ActiveRecord ones. Therefore I don't understand what you mean what you say the following:

However, this is semantically incorrect (that isn't the primary key), and will probably confuse developers later.

Or am I missing something? :slightly_frowning_face:

In any case, the settings shouldn't be needed on the dashboard, as Administrate should be able to detect what ActiveRecord is using, so that's still a bug. So let's see what the issue might be...

Just to make sure. When you bundle-installed the version from my branch, did you restart the server and reload the /new page? If you didn't the page would still have had old values for the primary key, triggering the wrong behaviour.

Silly question, but I thought I'd check while I'm looking into it :slightly_smiling_face:

pablobm avatar Apr 13 '22 21:04 pablobm

I restarted and reloaded, definitely.

However, this is semantically incorrect (that isn't the primary key), and will probably confuse developers later.

The problem here is that the "Workaround" I mentioned involves specifying a "primary" key that isn't the PK of the associated model in Administrate. I will double check to make sure my model configuration isn't muddying the waters to be sure myself 🙂

lake-effect avatar May 05 '22 17:05 lake-effect

Do you mean that the original belongs_to association in the ActiveRecord model is also using your workaround? If that's the case, then it sounds to me like the problem is with Rails and not with Administrate. Does that make sense?

pablobm avatar Jun 02 '22 14:06 pablobm

I think this will be fixed by https://github.com/thoughtbot/administrate/pull/2292

pablobm avatar Jan 12 '23 15:01 pablobm

Thanks so much! I'm sorry, I haven't been able to give time to this at work.

lake-effect avatar Apr 18 '23 17:04 lake-effect