Opened 8 years ago

Last modified 20 months ago

#418 accepted defect

Leave slug empty until we are sure media processing was successful

Reported by: joar Owned by:
Priority: major Milestone:
Component: programming Keywords: post-sql bitesized test introspection
Cc: Emily O'Leary Parent Tickets:

Description

We should leave the slug field of a media entry empty until we are done processing and only set it if processing was successful.

Subtickets

Attachments (1)

trac_418_slug_generation.patch (3.2 KB) - added by Emily O'Leary 6 years ago.
Patch to fix #418

Download all attachments as: .zip

Change History (16)

comment:1 Changed 8 years ago by Christopher Allan Webber

Component: component1programming
Owner: somebody deleted
Status: newassigned

comment:2 Changed 7 years ago by Christopher Allan Webber

Keywords: sprint bitesized added

Hm. This shouldn't be hard to do afaict...

Currently we do:

                # Generate a slug from the title
                entry.generate_slug()

In submit/views.py but it would be easy to move this to processing/init.py, after processing ends.

This is a bitesized task, good for a new contributor who's willing to learn how processing works. Good sprint task, too!

comment:3 Changed 7 years ago by Emily O'Leary

Owner: set to Emily O'Leary
Status: newaccepted

I took a crack at this. I dived right into the processing code (side note: in my dev instance I could upload videos for some reason. I tried webm, flv, mp4, and 3gp.

I couldn't find a good place in init.py to stick the slug generation that seemed to be effective and so I stuck it in task.py. Thoughts?

Here is my feature branch and commit: https://gitorious.org/~lotusecho/mediagoblin/lotusechos-mediagoblin/commit/97c9efaf47d5c5bfee33bf250567cec96a231488

comment:4 Changed 7 years ago by Emily O'Leary

Cc: Emily O'Leary added
Owner: changed from Emily O'Leary to joar
Status: acceptedassigned

I'll assign this to joar for review since he reported it.

comment:5 Changed 7 years ago by Christopher Allan Webber

Keywords: review added

Marking as review

comment:6 Changed 7 years ago by Christopher Allan Webber

Milestone: 0.4.0

comment:7 Changed 7 years ago by Elrond

Keywords: sprint review removed
Milestone: 0.4.0
Owner: changed from joar to Emily O'Leary

Hi LotusEcho,

could remove the generate_slug() in plugins/api/views.py also?

comment:8 Changed 7 years ago by Elrond

Oh, and please do not use TABs for indenting. Not with python.

comment:9 Changed 7 years ago by Emily O'Leary

I'll have to change my settings to convert my TABs, thanks for the tip. :)

I'll look at that bit you mentioned.

comment:10 Changed 7 years ago by Christopher Allan Webber

Status: assignedin_progress

Changed 6 years ago by Emily O'Leary

Patch to fix #418

comment:11 Changed 6 years ago by Emily O'Leary

Owner: Emily O'Leary deleted
Status: in_progressreview

I added a patch and it's also available here (https://gitorious.org/~lotusecho/mediagoblin/lotusechos-mediagoblin/commit/8b25f7139bdd8180947fe36dba0cebf80b94c74f). Although my re-basing didn't quite go as planned so it added more commits to it than it should have so it was a bit unwieldy there.

comment:12 Changed 6 years ago by rodney757

Resolution: fixed
Status: reviewclosed

Merged

Thanks!

comment:13 Changed 6 years ago by rodney757

Resolution: fixed
Status: closedaccepted

Okay, so I didn't test this well enough and am getting a really strange error when trying to upload an image with the same title as another image, so I reverted the commits and am reopening this until we can figure out what is wrong.

For some reason the query in check_media_slug_used is giving me an integrity error when doing query.first():

 IntegrityError) columns uploader, slug are not unique u'UPDATE core__media_entries SET slug=?, state=? WHERE core__media_entries.id = ?' (u'img-0019', u'processed', 145)

I'm not sure why it trying to UPDATE on a query.

This is with sqlite.

Last edited 6 years ago by rodney757 (previous) (diff)

comment:14 Changed 5 years ago by ShawnRisk

Keywords: test introspection added

comment:15 Changed 4 years ago by Loic Dachary

After removing entry.generate_slug() from mediagoblin/api/views.py the tests ( ./runtests.sh --cov-report=term-missing --cov=mediagoblin mediagoblin/tests ) pass with success, meaning it is not tested. Before modifying the code it seems necessary to add test coverage verifying the slug exists when it should and duplicates raise the appropriate errors.

Note: See TracTickets for help on using tickets.