Opened 7 years ago

Closed 7 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 7 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 by ayleph, 7 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, 7 years ago

Status: newreview

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

comment:3 by ayleph, 7 years ago

Milestone: 1.00.10.0

comment:4 by ayleph, 7 years ago

Resolution: fixed
Status: reviewclosed

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

Note: See TracTickets for help on using tickets.