Comments should work with models with String Ids

[MikeAbner]

Edit: Original title: "Form for tables with string id's throw an exception"

PGError: ERROR: invalid input syntax for integer: "xksS5NjE"
LINE 1: ...admin' AND ("active_admin_comments".resource_id = 'xksS5NjE'..."

Looks like the comments tables expect integer ids on the parent table but this isn't universally true.

[gregbell]

Could you please post the entire stacktrace in a gist?

[MikeAbner]

https://gist.github.com/971365

[gregbell]

I see. Active Admin uses a polymorphic relationships in Active Record for comments which usually include an integer id. But of course, Active Record does not require ids to be integers.

[MikeAbner]

Right. I tried turning off comments and it looks like it still tries to load the comments on the edit screen. Perhaps that shouldn't happen if comments are turned off? I'm loving what you've got here, but we don't use integer id's basically anywhere in our system. Perhaps we just have to punt on this for now.

[gregbell]

Interesting. I'll take a look at this case and see if there is a simple solution. All of the screens should still function without comments enabled.

On 2011-05-13, at 2:55 PM, malyk wrote:

Right. I tried turning off comments and it looks like it still tries to load the comments on the edit screen. Perhaps that shouldn't happen if comments are turned off? I'm loving what you've got here, but we don't use integer id's basically anywhere in our system. Perhaps we just have to punt on this for now.

Reply to this email directly or view it on GitHub:
#34 (comment)

[MikeAbner]

It just occurred to me that I may not have restarted my server after turning comments off. So that may be why the error is happening when comments are trying to be loaded...because they are still turned on. But I would wager that string id's are fairly common in the real world.

[gregbell]

Did restarting the server turn off comments for all your models?

[MikeAbner]

Didn't appear too. I was still getting a stack trace trying to load the show page of a model that had a string id.

[jancel]

I was able to fix this behavior by changing the migrations that were generated (works with both string id's and integer id's). And causes little or no ill that I can tell.

I'll leave it up to the crew here to determine if this would be in scope for addition to active_admin...
Here's what I had to do at install time.

in myapp/db/migrate/...create_admin_notes.rb (remove the commented line below, and add the two lines above it in place of it)

  t.string :resource_id, :null => false
  t.string :resource_type, :null => false
  # t.references :resource, :polymorphic => true, :null => false

[pcreux]

That's cool. Don't hesitate to update the migration and create a pull request.

[jancel]

I'm working this.

[nbartlomiej]

When viewing an entry (e.g.: http://localhost/admin/employees/1) I am getting an error from postgres:

ActiveRecord::StatementInvalid in Admin/employees#show

Showing /Users/benek/.rvm/gems/ruby-1.9.2-p290@x_2_0/gems/activeadmin-0.4.0/app/views/active_admin/resource/show.html.arb where line #1 raised:

PGError: ERROR:  operator does not exist: character varying = integer
LINE 1: ...ployee' AND "active_admin_comments"."resource_id" = 1 AND "a...
                                                             ^
HINT:  No operator matches the given name and argument type(s). You might need to add explicit type casts.
: SELECT COUNT(*) FROM "active_admin_comments"  WHERE "active_admin_comments"."resource_type" = 'Employee' AND "active_admin_comments"."resource_id" = 1 AND "active_admin_comments"."namespace" = 'admin'
Extracted source (around line #1):

1: render renderer_for(:show)
Rails.root: /Users/benek/repositories/temp/x_2_0

Full gist: https://gist.github.com/1886112

Could it be that commit 00e6b24 introduces the regression? I have replaced in the file: lib/generators/active_admin/install/templates/migrations/1_create_admin_notes.rb the line

t.string :resource_id

with

t.integer :resource_id

and for now everything seems to work.

[pcreux]

@jancel Could you have a look at that?

[jancel]

Sure can.

[jancel]

jancel@e24cb3f

This should a. fix issue for new installs, and b. remain compatible with old installs

[localhots]

Could you please release a patch version with this fix?

[pcreux]

@Magnolia-Fan Only @gregbell can publish a new gem. He's out of town till Tuesday. Please use the master branch on github till then. :)