Opened 11 years ago

Closed 10 years ago

Last modified 9 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:


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

It does seem that the item gets added though.

Change History (10)

comment:1 by Simon Fondrie-Teitler, 11 years ago

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

in reply to:  1 comment:2 by Simon Fondrie-Teitler, 11 years ago

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 by Simon Fondrie-Teitler, 11 years ago

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

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

comment:4 by Elrond, 11 years ago

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 by Elrond, 11 years ago

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 by Christopher Allan Webber, 11 years ago

I'm fine with 2.

comment:7 by rodney757, 11 years ago

Keywords: bitesized added

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

comment:8 by Josie, 10 years ago

Status: acceptedreview

comment:9 by NattilyPidgin, 10 years ago

okay, I'll get right on reviewing this~

comment:10 by NattilyPidgin, 10 years ago

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.