Using HasCustomFields in a plugin

Dev
1

I have a model in a plugin. The model is Pfaffmananger::Server, the table is pfaffmanager_servers. For Reasons, I want to have custom fields for this model (I expect there to be ways that I want to extend the server data that are likely to be used only for a few instances and I don’t want a zillion mostly-unused fields in the model).

Here’s the migration that creates the table:

class CreatePfaffmanagerServerCustomField < ActiveRecord::Migration[6.0]
  def change
    create_table :pfaffmanager_server_custom_fields do |t|
      t.integer :server_id, null: false
      t.string :name, limit: 256, null: false
      t.text :value
      t.timestamps null: false
    end
    add_index :pfaffmanager_server_custom_fields, [:server_id, :name]
  end
end

If I use pfaffmanager_server_id instead of server_id the migration fails because it can’t find server_id, but when I use pfaffmanager_server_id for the field name, when I go to save a server after I create a custom field, I get:

  Pfaffmanager::ServerCustomField Load (0.4ms)  SELECT "pfaffmanager_server_custom_fields".* FROM "pfaffmanager_server_custom_fields" WHERE "pfaffmanager_server_custom_fields"."server_id" = 1
   (0.2ms)  ROLLBACK
PG::UndefinedColumn: ERROR:  column "pfaffmanager_server_id" of relation "pfaffmanager_server_custom_fields" does not exist
LINE 1: INSERT INTO pfaffmanager_server_custom_fields (pfaffmanager_...

So if the migration has field pfaffmanager_server_id the migration fails, and if the field name is server_id the save fails.

I’m looking at save_custom_fields in concers/has_custom_fields.rb, but can’t quite figure out if there’s some way to override that.

Is there something that I can do besides refactor the whole thing to use server rather than pfaffmanager_server?

2

Did you also update the add_index call to point to pfaffmanager_server_id?

2 Likes
3

Thanks so much, David!

Yes. That could be my problem. I guess that’s the error message I should have included. And I can make the migration work with either field (though with pfaffmanager_server_id as the field name, the index name becomes > 63 characters, so I have to use a different name for the index.

Let me try again.

server_id as the field name

class CreatePfaffmanagerServerCustomField < ActiveRecord::Migration[6.0]
  def change
    create_table :pfaffmanager_server_custom_fields do |t|
      t.integer :server_id, null: false
      t.string :name, limit: 256, null: false
      t.text :value
      t.timestamps null: false
    end
    add_index :pfaffmanager_server_custom_fields, [:server_id, :name]
  end
end

Fails on save with:

 Pfaffmanager::ServerCustomField Load (3.8ms)  SELECT "pfaffmanager_server_custom_fields".* FROM "pfaffmanager_server_custom_fields" WHERE "pfaffmanager_server_custom_fields"."server_id" = 1
   (1.4ms)  ROLLBACK
PG::UndefinedColumn: ERROR:  column "pfaffmanager_server_id" of relation "pfaffmanager_server_custom_fields" does not exist
LINE 1: INSERT INTO pfaffmanager_server_custom_fields (pfaffmanager_...
                                                       ^

So it’s looking for pfaffmanager_server_id. Now, let’s use pfaffmanager_server_id.

pfaffmanager_server_id as field name

Here’s the migration:

class CreatePfaffmanagerServerCustomField < ActiveRecord::Migration[6.0]
  def change
    create_table :pfaffmanager_server_custom_fields do |t|
      t.integer :pfaffmanager_server_id, null: false
      t.string :name, limit: 256, null: false
      t.text :value
      t.timestamps null: false
    end
    add_index :pfaffmanager_server_custom_fields,
      [:pfaffmanager_server_id, :name],
      name: 'index_pfaffmanager_server_custom_fields_on_server_id_and_name'

  end
end

Here’s what happens when I try s.custom_fields (the above was able to return nil for this but didn’t fail until I tried to save a custom field). I register_custom_field_type here):

 pry(main)> s.custom_fields
   (1.1ms)  SELECT "pfaffmanager_server_custom_fields"."name", "pfaffmanager_server_custom_fields"."value" FROM "pfaffmanager_server_custom_fields" WHERE "pfaffmanager_server_custom_fields"."server_id" = 1 ORDER BY id asc
ActiveRecord::StatementInvalid: PG::UndefinedColumn: ERROR:  column pfaffmanager_server_custom_fields.server_id does not exist
LINE 1: ...e" FROM "pfaffmanager_server_custom_fields" WHERE "pfaffmana...
                                                             ^

from /home/pfaffman/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-mini-profiler-2.3.0/lib/patches/db/pg.rb:69:in `exec_params'
Caused by PG::UndefinedColumn: ERROR:  column pfaffmanager_server_custom_fields.server_id does not exist
LINE 1: ...e" FROM "pfaffmanager_server_custom_fields" WHERE "pfaffmana...
                                                             ^

Now it’s looking for server_id.

So it seems like custom_fields wants it one way a save_custom_fields wants it the other.

It doesn’t care what the index name is, does it?

1 Like
4

I don’t think so, no

I think the second approach is probably the one to aim for (using pfaffman_server_id as the column name)

I wonder if overriding this method may help:

You can check the current value on the console like

Server.new.custom_fields_fk

If that’s server_id, then I suggest overriding that method in your Server model

class Server < ...
  def custom_fields_fk
    "pfaffman_server_id"
  end
end
1 Like
5

Or maybe it’s this bit :thinking:

https://github.com/discourse/discourse/blob/master/app/models/concerns/has_custom_fields.rb#L76-L77

1 Like
6

Thanks so much! So
Server.new.custom_fields_fk is pfaffmanager_server_id if I use pfaffmanager_serfver_id. So maybe I should use server_id and then override as you suggested above.

It looks like refresh_custom_fields_from_db is trying to use the wrong names. I don’t quite understand what _custom_fields.order... is doing.

https://github.com/discourse/discourse/blob/master/app/models/concerns/has_custom_fields.rb#L276-283

I’m going to see what happens if I use server_id and if I can then override custom_fields_fk

1 Like
7

Ha! You did it! Thanks so much. I switched it to server_id and then did this in my server.rb model:

    def custom_fields_fk
      @custom_fields_fk ||= "server_id"
    end

As long as that’s going to override that only for this model and not break user_custom_field and friends I think I should be ready to start beating myself over the head again on Best way to enforce permissions--controller or constraint?. And then I can add routes to do stuff like fill my new custom fields with . . . something.

I can’t thank you enough. You likely saved me a whole day. I owe you a :beer:!

1 Like
8

Great! :tada: If you can think of a way to make it more generic then that would certainly be PR-welcome.

1 Like
9

If something occurs to me I’ll let you know or submit a PR, but I have to think that a plugin that wants to use custom fields is pretty far out on the edge.

1 Like
10

OM_fk_G.

custom_fields_fk seemed so stupid that I assumed it was something that I meant to delete and had named it something stupid so I’d know it was safe to delete it later. It was later, so I deleted it. And then my specs failed.

There was even a comment with a link to this topic in the code.

1 Like