Mediagoblin gives an error when trying to add an item to an existing collection

[sazius]

Go to an existing item, select "Add to a collection", pick an existing collection from the drop down menu, press "Add" => sad goblin appears.

Running with lazyserver to get more informative error message I get (two CRITICA rows added according to pythonsnake's suggestion for added debugging info):

Starting server in PID 32700.
serving on 0.0.0.0:6543 view at http://127.0.0.1:6543
2013-03-10 14:13:40,638 CRITICA [mediagoblin.user_pages.views] <class 'mediagoblin.db.models.Collection'>
2013-03-10 14:13:40,638 CRITICA [mediagoblin.user_pages.views] <type 'int'>
Error - <class 'sqlalchemy.exc.InterfaceError'>: (InterfaceError) Error binding parameter 0 - probably unsupported type. u'SELECT core__collections.id AS core__collections_id, core__collections.title AS core__collections_title, core__collections.slug AS core__collections_slug, core__collections.created AS core__collections_created, core__collections.description AS core__collections_description, core__collections.creator AS core__collections_creator, core__collections.items AS core__collections_items \nFROM core__collections \nWHERE core__collections.id = ? AND core__collections.creator = ?\n LIMIT ? OFFSET ?' (<mediagoblin.db.models.Collection object at 0x2b81910>, 1, 1, 0)
URL: http://saz.im:6543/u/sazius/m/58/collect/
File '/var/www/mediagoblin/lib/python2.6/site-packages/Paste-1.7.5.1-py2.6.egg/paste/exceptions/errormiddleware.py', line 144 in __call__
  app_iter = self.application(environ, sr_checker)
File '/var/www/mediagoblin/lib/python2.6/site-packages/Paste-1.7.5.1-py2.6.egg/paste/urlmap.py', line 203 in __call__
  return app(environ, start_response)
File '/var/www/mediagoblin/lib/python2.6/site-packages/Beaker-1.6.4-py2.6.egg/beaker/middleware.py', line 155 in __call__
  return self.wrap_app(environ, session_start_response)
File '/var/www/mediagoblin/mediagoblin/app.py', line 236 in __call__
  return self.call_backend(environ, start_response)
File '/var/www/mediagoblin/mediagoblin/app.py', line 216 in call_backend
  response = controller(request)
File '/var/www/mediagoblin/mediagoblin/decorators.py', line 224 in wrapper
  return controller(request, media=media, *args, **kwargs)
File '/var/www/mediagoblin/mediagoblin/decorators.py', line 48 in new_controller_func
  return controller(request, *args, **kwargs)
File '/var/www/mediagoblin/mediagoblin/user_pages/views.py', line 232 in media_collect
  creator=request.user.id).first()
File '/var/www/mediagoblin/lib/python2.6/site-packages/SQLAlchemy-0.8.0-py2.6-linux-x86_64.egg/sqlalchemy/orm/query.py', line 2181 in first
  ret = list(self[0:1])
File '/var/www/mediagoblin/lib/python2.6/site-packages/SQLAlchemy-0.8.0-py2.6-linux-x86_64.egg/sqlalchemy/orm/query.py', line 2048 in __getitem__
  return list(res)
File '/var/www/mediagoblin/lib/python2.6/site-packages/SQLAlchemy-0.8.0-py2.6-linux-x86_64.egg/sqlalchemy/orm/query.py', line 2252 in __iter__
  return self._execute_and_instances(context)
File '/var/www/mediagoblin/lib/python2.6/site-packages/SQLAlchemy-0.8.0-py2.6-linux-x86_64.egg/sqlalchemy/orm/query.py', line 2267 in _execute_and_instances
  result = conn.execute(querycontext.statement, self._params)
File '/var/www/mediagoblin/lib/python2.6/site-packages/SQLAlchemy-0.8.0-py2.6-linux-x86_64.egg/sqlalchemy/engine/base.py', line 664 in execute
  params)
File '/var/www/mediagoblin/lib/python2.6/site-packages/SQLAlchemy-0.8.0-py2.6-linux-x86_64.egg/sqlalchemy/engine/base.py', line 764 in _execute_clauseelement
  compiled_sql, distilled_params
File '/var/www/mediagoblin/lib/python2.6/site-packages/SQLAlchemy-0.8.0-py2.6-linux-x86_64.egg/sqlalchemy/engine/base.py', line 878 in _execute_context
  context)
File '/var/www/mediagoblin/lib/python2.6/site-packages/SQLAlchemy-0.8.0-py2.6-linux-x86_64.egg/sqlalchemy/engine/base.py', line 871 in _execute_context
  context)
File '/var/www/mediagoblin/lib/python2.6/site-packages/SQLAlchemy-0.8.0-py2.6-linux-x86_64.egg/sqlalchemy/engine/default.py', line 320 in do_execute
  cursor.execute(statement, parameters)
InterfaceError: (InterfaceError) Error binding parameter 0 - probably unsupported type. u'SELECT core__collections.id AS core__collections_id, core__collections.title AS core__collections_title, core__collections.slug AS core__collections_slug, core__collections.created AS core__collections_created, core__collections.description AS core__collections_description, core__collections.creator AS core__collections_creator, core__collections.items AS core__collections_items \nFROM core__collections \nWHERE core__collections.id = ? AND core__collections.creator = ?\n LIMIT ? OFFSET ?' (<mediagoblin.db.models.Collection object at 0x2b81910>, 1, 1, 0)


CGI Variables
-------------
  CONTENT_LENGTH: '676'
  CONTENT_TYPE: 'multipart/form-data; boundary=---------------------------145476651317241088371037342262'
  CSRF_TOKEN: u'14123432470749457721'
  HTTP_ACCEPT: 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8'
  HTTP_ACCEPT_ENCODING: 'gzip, deflate'
  HTTP_ACCEPT_LANGUAGE: 'en,en-gb;q=0.8,sv-fi;q=0.6,sv;q=0.4,fi;q=0.2'
  HTTP_CONNECTION: 'keep-alive'
  HTTP_COOKIE: 'mediagoblin_csrftoken=14123432470749457721; mediagoblin_csrftoken=14123432470749457721; mediagoblin=c096c3693b3d407a8586a3c412afb3b9'
  HTTP_DNT: '1'
  HTTP_HOST: 'saz.im:6543'
  HTTP_REFERER: 'http://saz.im:6543/u/sazius/m/58/collect/'
  HTTP_USER_AGENT: 'Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20100101 Firefox/10.0.12 Iceweasel/10.0.12'
  PATH_INFO: '/u/sazius/m/58/collect/'
  REMOTE_ADDR: '82.181.107.174'
  REQUEST_METHOD: 'POST'
  SERVER_NAME: '0.0.0.0'
  SERVER_PORT: '6543'
  SERVER_PROTOCOL: 'HTTP/1.1'


WSGI Variables
--------------
  application: {(None, ''): <beaker.middleware.SessionMiddleware object at 0x27efdd0>, (None, '/mgoblin_static'): <StaticURLParser '/var/www/mediagoblin/mediagoblin/static'>, (None, '/mgoblin_media'): <StaticURLParser '/var/www/mediagoblin/user_dev/media/public'>, (None, '/theme_static'): <StaticURLParser '/var/www/mediagoblin/user_dev/theme_static'>}
  beaker.get_session: <bound method SessionMiddleware._get_session of <beaker.middleware.SessionMiddleware object at 0x27efdd0>>
  beaker.session: {'_accessed_time': 1362921220.6312571, 'user_id': u'1', '_creation_time': 1362921214.262624}
  paste.httpserver.thread_pool: <paste.httpserver.ThreadPool object at 0xc43d50>
  paste.throw_errors: True
  werkzeug.request: <Request 'http://saz.im:6543/u/sazius/m/58/collect/' [POST]>
  wsgi process: 'Multithreaded'
------------------------------------------------------------

After discussions in irc with pythonsnake this fix was tried and seemed to fix at least this problem:

diff --git a/mediagoblin/user_pages/views.py b/mediagoblin/user_pages/views.py
index dc56208..0036ded 100644
--- a/mediagoblin/user_pages/views.py
+++ b/mediagoblin/user_pages/views.py
@@ -227,7 +227,7 @@ def media_collect(request, media):
     # Otherwise, use the collection selected from the drop-down
     else:
         collection = Collection.query.filter_by(
-            id=form.collection.data,
+            id=form.collection.data.id,
             creator=request.user.id).first()
 
     # Make sure the user actually selected a collection

[Elrond]

The problem is:

Collection.query.filter_by(id=X, ...)

1. X = form.collection.data This works nicely for the completely empty form (X = None). It does not work for a selected collection, because X will be the collection, not its id.
2. X = request.form.get('collection') (old code). This one works mostly, except for the completely empty form on postgres, because in this case X = u"__None" and postgres does not like comparing an integer column with a string.

Suggested fix:

        collection = form.collection.data
        if collection and collection.creator != request.user.id:
            collection = None

If approved, I'll push that fix.

[Elrond]

Not to mention: We really need unit tests for this stuff.

[Elrond]

Should be fixed in e9330b9552858d46783971e8624ccc5ac80cfe46.