Comments should work with models with String Ids #34

Closed
malyk opened this Issue May 13, 2011 · 17 comments

Projects

None yet

6 participants

@malyk
malyk commented May 13, 2011

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
Contributor

Could you please post the entire stacktrace in a gist?

@gregbell
Contributor

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.

@malyk
malyk commented May 13, 2011

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
Contributor

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)

@malyk
malyk commented May 13, 2011

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
Contributor

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

@malyk
malyk commented May 16, 2011

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
Contributor
jancel commented Feb 19, 2012

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
Contributor
pcreux commented Feb 19, 2012

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

@jancel
Contributor
jancel commented Feb 19, 2012

I'm working this.

@pcreux pcreux closed this in 7f740d8 Feb 20, 2012
@nbartlomiej
Contributor

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 pcreux reopened this Feb 22, 2012
@pcreux
Contributor
pcreux commented Feb 22, 2012

@jancel Could you have a look at that?

@jancel
Contributor
jancel commented Feb 22, 2012

Sure can.

@jancel
Contributor
jancel commented Feb 22, 2012

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
Contributor
pcreux commented Mar 2, 2012

@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. :)

@pcreux pcreux closed this Mar 21, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment