When viewing a Collection item, only cycle through the media in the Collection

[rodney757]

Currently if you click on a media entry from within a collection, it takes you to the media's page /u/rodney757/m/photo/. From there when you click older or newer, it cycles through all of the user's media. I think that it should only cycle through the media in the collection.

cwebb says:

maybe like when viewing a page like:
/u/cwebber/m/new-comic/?in=collection:comics
which would mean you're browsing in the "comics" collection
not sure though
could also do session hacks to preserve state
I think flickr does something similar.

[rodney757]

​https://github.com/rodney757/mediagoblin/tree/collections

[Brett Smith]

This branch is very nice. There's nothing extraneous, the patch uses patterns that are familiar from other parts of the code base, and it's clear to see how the pieces fit together. New bits are documented as appropriate, too. Thanks!

I do think a couple of pieces that might be more long-term maintainable with a different approach. The biggest thing is that there's a new route that goes to views.py, and then URL parsing happens in there:

    if '/in/collection/' in request.path:
        collection_args = request.path.split('/in/collection/')[1]
        [… parse specific bits of the URL…]

This doesn't get the full value of the abstraction of the routing table. If the new route called dedicated code (perhaps handing off to the regular view code after extra setup), or we could at least introspect specific values out of the request object rather than parsing the string, that would keep things robust.

The url_to_prev and url_to_next functions share a lot of code. I think it should be possible to abstract them to a single function, something like url_to_rel, that takes a couple of sort operators as arguments, and then this pair could just call that general function appropriately. (I realize this all also applies to the corresponding media entry methods.) Similar abstraction could happen in the prev_next.html template—just use the conditional to capture the object you want to work on, then call methods on that object.

I also wonder if the condition to check the item's "processed" state could simplified by moving it to the query directly, rather than handling it in Python.

And then the last thing is just a small typo in mediagoblin/user_pages/lib.py:

    collection_title - The title of the Collection the collectionItem belongs
        too.

s/too/to/ at the end there.

Still, all this is just style/maintainability stuff—the basic approach is sound. Thanks again!