Adding Collections to addmedia and batchaddmedia, and tags to batchaddmedia.

[Daniel Krol]

The addmedia command allows you to specify tags to add to your submitted item, but batchaddmedia does not. Further, neither of these commands allow you to specify a collection to add your items to. I'd like these features to be added (and in fact I'm working on a patch to do just that).

For reference, here is a mailing list conversation that I started on the topic. (I'm moving the discussion here to reduce noise on the list): ​http://lists.mediagoblin.org/pipermail/devel/2015-November/001305.html

Current status is that I have the feature working, and I even added some tests for these two commands (which afaik were missing). However there are some error cases that I am trying to avoid introducing. I'll put details in the comments, since it's still getting hashed out. You can probably follow my progress here: ​https://github.com/orblivion/mediagoblin/compare/batchaddmedia-collection-tag-on-master?expand=1

[Daniel Krol]

Some notes on the outstanding issues I mentioned on the list, and a couple others:

1) Regarding celery allowing silent transcoding errors: As I mentioned to you on the list, my initial plan was to have it give a warning, or "are you sure y/n", if you try to do the batch command with both celery and collections. You then decided that there should be something like a "skip warnings" option in case the command is run as part of a script. I decided that rather than add a bunch of weird options and functionality it's simpler to just add a simple warning to --celery in the help text. It's in my WIP on github. Thoughts?

2) The Collection view seems to show unprocessed media entries. This is probably because until my patch, it was impossible to add to a collection other than in the web UI, and the web UI didn't let you work on items until they were fully processed. So, you never had a need to filter these unprocessed items out. I think I will have to fix this along with this patch (​http://i.imgur.com/D0daEJW.gifv) lest I introduce a bug.

3) That said, these unprocessed items raised my eyebrow a bit when I saw them in my big vacation collection. These unprocessed items were in the middle of my collection, where I ran into some video transcoding problems as I was doing my import. It lead me to notice the following weirdness, though I can't say for sure whether it's an unrelated bug: It seems that in the collection view, when I'm on page x where x > 1, the first item in the collection shows up as the last item on the page. In fact, it shows up _instead of_ the item that ought to be there. I freaked out because I thought that I was missing that last-item-in-the-page, but it turned out that the item wasn't actually missing, nor did it fail to get added to the collection. It's just a bug that manifests in the collection view for me right now. For the record, my mediagoblin instance is on v0.8.0 + an old version of my changes, though I accidentally partially migrated my db to what was on master about a week ago (it had some problems migrating), which may put this in a weird state.

4) Ordering seems to be unenforced in collections anyway(?). I tried adding a collection item by hand (this time in a legit master, not my weird deployed instance) and the collection_item.position was None. From a quick look, I didn't see position used in any sort of view filter. My conclusion was that this is a "TODO" for you folks; is that correct? So perhaps I shouldn't worry *so much* about this just now? I do want to make sure that once it is implemented, it's considered for batchaddmedia. I suppose you'll grep around for Collection when that happens. Though, maybe while I try to fix the "showing unprocessed items" bug, I can also at least add an order-by-timestamp or something, as a temporary fix to the problem I describe in 3)? Otherwise I'll leave it alone.

5) On the list I also mentioned a "continue where you left off" hint for when you're doing batchaddmedia and you get an error when in the middle of a csv file. While it's related to my change and relatively simple to do, I believe is out of the scope of this bug because it's not strictly a response to a new problem I'm introducing. I'd rather put it in a subsequent patch. Let me know if you disagree.

[Daniel Krol]

Or, maybe there's an assumption that you won't have hidden, unprocessed things attached to collections? Maybe we should just leave it as such after all, so the user can see it and manually remove it from the collection?

Ideally of course I just wouldn't add it unless it's processed. The problem is that (especially if the user chooses --celery) I can't prevent it in a straightforward manner.

[Daniel Krol]

And as far as ordering within a collection view, I was wrong. Though it doesn't seem to set a position value when you add an item to the collection (again, even in the existing web UI), it does attempt to use the position field for ordering. Here's the relevant code:

    def get_collection_items(self, ascending=False):
        #TODO, is this still needed with self.collection_items being available?
        order_col = CollectionItem.position
        if not ascending:
            order_col = desc(order_col)
        return CollectionItem.query.filter_by(
            collection=self.id).order_by(order_col)

And here's what I tried changing it to, to have it default to id in lieu of position:

    def get_collection_items(self, ascending=False):
        #TODO, is this still needed with self.collection_items being available?
        order_cols = CollectionItem.position, CollectionItem.id
        if not ascending:
            order_cols = map(desc, order_cols)
        return CollectionItem.query.filter_by(
            collection=self.id).order_by(*order_cols)

This makes the weird bug with the first item showing up out of place go away. The weird thing (to me) is that it's descending by default, just like (for instance) the tag view. I would have expected that collections show up in ascending order. Are collections supposed to be descending? Interestingly it happens to show up in (more or less) ascending order before I make this change. I guess it defaults to order_by(ascending id kindof).

Anyway, maybe I'll make it a personal TODO to make another PR with changes to this view, regarding unprocessed items, and item ordering.

For now I'll run with the assumption that all of these issues can be pushed off to other patches, and I'll go ahead and squarsh my changes and change this ticket to review status.

[Daniel Krol]

I just force pushed a couple fixups. Here's the current hash, in case I decide to change it again: ​https://github.com/orblivion/mediagoblin/commit/2056d5d5f23645e15fbc097ad59fc224748dda40

[Daniel Krol]

Sorry, force pushed some more. Currently: ​https://github.com/orblivion/mediagoblin/commit/e28a8677fff6022eca728053db898ee4298b1b51

[Daniel Krol]

I mentioned that I'd put off some of the work to other issues. So here it is.

Continuing where you left off in case of error: #5361

Fixing sorting/filtering of media entries within collections: #5365

[Christopher Allan Webber]

This branch is batchaddmedia-collection-tag-on-master, just for clarity.