Opened 9 years ago

Closed 8 years ago

Last modified 7 years ago

#630 closed defect (fixed)

Media goblin gives error page after adding a media item to a collection

Reported by: Simon Fondrie-Teitler Owned by:
Priority: major Milestone:
Component: programming Keywords: bitesized
Cc: Parent Tickets:

Description

After trying to add a media item to a collection, I get the following error:

http://paste.debian.net/233242/

It does seem that the item gets added though.

Subtickets

Change History (10)

comment:1 Changed 9 years ago by Simon Fondrie-Teitler

It only seems to happen if a collection does not already have an item in it .

comment:2 in reply to:  1 Changed 9 years ago by Simon Fondrie-Teitler

Replying to simonft:

It only seems to happen if a collection does not already have an item in it .

This is not actually the case. It seems to only happen to one of the images, but when added to any collection. I suspect it has something to do with the image being added prior to the version of mediagoblin being upgraded.

comment:3 Changed 9 years ago by Simon Fondrie-Teitler

It also happens when removing the media from a collection, except with the error:

File '/usr/share/mediagoblin/mediagoblin/user_pages/views.py', line 362 in collection_item_confirm_remove
  entry.collected = entry.collected - 1
TypeError: unsupported operand type(s) for -: 'NoneType' and 'int'

comment:4 Changed 9 years ago by Elrond

Could you see if the following sql gives some entries for you?

  select id,slug,collected from core__media_entries where collected is null;

If yes, I know what is happening, but I don't know how it could happen in the first place.

Personally, I want the MediaEntry.collected column go away. I hate cachy columns in the main db.

comment:5 Changed 9 years ago by Elrond

Status: newaccepted

As I suspected, the collected column for one entry was NULL on simonft's db.

This maps to None in python and m.collected = m.collected + 1 doesn't work for None.

I have no clue, why this could have happened, really. And we could not pinpoint any speciality.

Options I can see:

  1. Write a migration that searches for collected==None and replaces with 0. Then add a nullable=False constraint.
  2. Drop the column. AFAIK we don't use it anyway. And assuming the number of collections an item is in is low, a len(media.in_collections) will do also.

I vote for (2), obviously. That's "Elrond's database razor".

comment:6 Changed 9 years ago by Christopher Allan Webber

I'm fine with 2.

comment:7 Changed 8 years ago by rodney757

Keywords: bitesized added

It seems that MediaEntry.collected isn't even used for anything. This should be fairly easy to remove.

comment:8 Changed 8 years ago by Josie

Status: acceptedreview

comment:9 Changed 8 years ago by NattilyPidgin

okay, I'll get right on reviewing this~

comment:10 Changed 8 years ago by NattilyPidgin

Resolution: fixed
Status: reviewclosed

ok reviewed it and it was all great so I pushed it to master

Note: See TracTickets for help on using tickets.