Feature/cdodge/soft delete assets - #170
Conversation
There was a problem hiding this comment.
If trash_store.find(asset_location) raises a NotFoundError, trash_store.find(content.thumbnail_location) will never be executed. You may want to break this into two separate try/catch blocks, to be sure that both lines are executed.
|
@dmitchell fixed incorrect default parameter as well as added unit test for emptying the trashcan |
|
@singingwolfboy cleaned up that test logic to make it cleaner (avoid all those try: except: paths). |
|
@singingwolfboy @dmitchell can you do another pass on the review. I fixed up things that I perceived to be strong requests and left softer requests unanswered. I'd like to get this merge don Monday for Tues release. |
|
Seems like I need to rebase |
|
+1 once it passes tests |
…Update class factory to allow callers to specify if they want the default set of assets or the trashcan
…we can clean up caller logic in the tests
|
Looks good to me. 👍 |
Feature/cdodge/soft delete assets
…ance This improves the performance of the save to github for the cms (had some minor requests for clarification, but that's all)
…-current-position cdodge/speed-up-get-current-position: enough said
delete account trans error and trans
PROD-1236: Do not expose user id with certificate URL.
…re/auth-backends-from-json use AUTHENTICATION_BACKENDS from APPSEMBLER_FEATURES
3 participants
Feature has been submitted to test, so I figured I'd get the pull request underway.