Opened 9 years ago

Closed 9 years ago

#5514 closed defect (fixed)

"Add to Collection" throws server error when no collection selected

Reported by: ayleph Owned by:
Priority: major Milestone: 0.10.0
Component: programming Keywords: crash, error, collection
Cc: Parent Tickets:

Description

The Add to Collection page provides a pre-populated dropdown list of collections to which media can be added. By default, the dropdown box says "-- Select --." If you click Add without selecting a collection, a server error is generated with the following traceback.

2017-06-28 21:30:46,445 ERROR   [waitress] Exception when serving /u/andrew/m/1/collect/
Traceback (most recent call last):
  File "/path/to/mediagoblin/lib/python2.7/site-packages/waitress-1.0a1-py2.7.egg/waitress/channel.py", line 338, in service
    task.service()
  File "/path/to/mediagoblin/lib/python2.7/site-packages/waitress-1.0a1-py2.7.egg/waitress/task.py", line 169, in service
    self.execute()
  File "/path/to/mediagoblin/lib/python2.7/site-packages/waitress-1.0a1-py2.7.egg/waitress/task.py", line 399, in execute
    app_iter = self.channel.server.application(env, start_response)
  File "/path/to/mediagoblin/mediagoblin/app.py", line 342, in __call__
    return self.call_backend(environ, start_response)
  File "/path/to/mediagoblin/lib/python2.7/site-packages/Werkzeug-0.11.10-py2.7.egg/werkzeug/wsgi.py", line 599, in __call__
    return self.app(environ, start_response)
  File "/path/to/mediagoblin/mediagoblin/app.py", line 276, in call_backend
    return self._finish_call_backend(request, environ, start_response)
  File "/path/to/mediagoblin/mediagoblin/app.py", line 318, in _finish_call_backend
    response = controller(request)
  File "/path/to/mediagoblin/mediagoblin/decorators.py", line 47, in wrapper
    return controller(request, *args, **kwargs)
  File "/path/to/mediagoblin/mediagoblin/decorators.py", line 280, in wrapper
    return controller(request, media=media, *args, **kwargs)
  File "/path/to/mediagoblin/mediagoblin/decorators.py", line 47, in wrapper
    return controller(request, *args, **kwargs)
  File "/path/to/mediagoblin/mediagoblin/decorators.py", line 74, in new_controller_func
    return controller(request, *args, **kwargs)
  File "/path/to/mediagoblin/mediagoblin/user_pages/views.py", line 300, in media_collect
    item = CollectionItem.query.filter_by(collection=collection.id)
AttributeError: 'NoneType' object has no attribute 'id'

Attachments (1)

0001-Fix-5514-Add-to-Collection-causes-server-error.patch​ (1.7 KB ) - added by ayleph 9 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 by ayleph, 9 years ago

It looks like this regression was added in ​0f3bf8d. In the updated code, the collection is used to query the database for an existing item before checking whether collection is none.

     # Make sure the user actually selected a collection
+    item = CollectionItem.query.filter_by(collection=collection.id)
+    item = item.join(CollectionItem.object_helper).filter_by(
+        model_type=media.__tablename__,
+        obj_pk=media.id
+    ).first()
+
     if not collection:
         messages.add_message(
             request, messages.ERROR,
             _('You have to select or add a collection'))
         return redirect(request, "mediagoblin.user_pages.media_collect",
-                    user=media.get_uploader.username,
+                    user=media.get_actor.username,
                     media_id=media.id)
 
-
     # Check whether media already exists in collection
-    elif CollectionItem.query.filter_by(
-        media_entry=media.id,
-        collection=collection.id).first():
+    elif item is not None:
         messages.add_message(request, messages.ERROR,
                              _('"%s" already in collection "%s"')
                              % (media.title, collection.title))

comment:2 by ayleph, 9 years ago

Status: new → review

I've attached a patch which I think resolves this issue. Ready for review.

comment:3 by ayleph, 9 years ago

Milestone: 1.0 → 0.10.0

comment:4 by ayleph, 9 years ago

Resolution: → fixed
Status: review → closed

I've been using this patch for weeks without issue. Pushed to master in ​​763eae8.

Note: See TracTickets for help on using tickets.