Download All submissions (based on #1181) #1456
Conversation
This improves the bulk download unit test by having it check that only selected files are downloaded and not unselected files. It extends the unit test to include a test for downloading all present files.
|
Reviewing this now! If you're interested in starting on another one, #383 looks like an issue that might be suited to your skills and interests. |
There was a problem hiding this comment.
Okay, so we started discussing some problems with this one (other than the line comments I made so far) that needed to be resolved. Those have turned out to be a little more complicated than I thought, and describing them was turning into a long-form essay. What I think is a better solution is for me to fix these things, and then make a PR against your branch that you can look over. You said you will be here tomorrow, and are working on #383 now, so by me taking over the final stretch of this one it will let you focus more on that issue, and we can make sure we get both done by tomorrow night 😸. Anyway, great working diving into the project so quickly, and thanks for the high-quality PR (both you and @pwplus)!
| @@ -627,20 +641,22 @@ def bulk_delete(sid, items_selected): | |||
|
|
|||
| def bulk_download(sid, items_selected): | |||
| source = get_source(sid) | |||
| filenames = [store.path(sid, item.filename) for item in items_selected] | |||
| return bulk_dl(source.journalist_filename, items_selected) | |||
|
|
|||
There was a problem hiding this comment.
Nit: can you add another newline here, just to keep the double-newline between functions style consistent?
| if docs == []: | ||
| flash("No unread documents in collections!", "error") | ||
| return redirect(url_for('index')) | ||
| return bulk_dl('all_unread_' + g.user.username, docs) |
There was a problem hiding this comment.
The one thing about this naming scheme is that for multi-journalist user SD instances, if one journalist downloads a message, it's marked read for all journalists. So I'd just leave the first positional argument as all_unread.
| <button type="submit" name="action" value="download"><i class="fa fa-download"></i> Download selected</button> | ||
| <button type="submit" name="action" value="confirm_delete" class="danger" id="delete_selected"><i class="fa fa-times"></i> Delete selected</button> | ||
| </div> | ||
| <div class="document-actions"> |
There was a problem hiding this comment.
Can you redo the indent for this block?
|
To be clear, I'll take care of the small in-line issues too. |
|
@fowlslegs I'm more than happy to make style-related changes :) |
|
My bad, I missed your first comment before making the changes. Feel free to revert if it's easier. |
|
Closing this in favor of #1467 |
This PR takes #1181, rebases develop onto it, and adds further test.
This allows for a Journalist to click "Download all unread" from the submissions index view and download all unread submissions in one zip file.
Related to #1354