Code review comment for lp:~aaronp/rnr-server/delete-review

Revision history for this message
Aaron Peachey (aaronp) wrote :

Hi Michael,
Thanks alot for the speedy review. You can probably tell I'm just feeling my way around django for now so I appreciate the comprehensive feedback.

I'm happy to make the suggested changes from you and achuni and resubmit shortly.

Just a couple of things:
1. Re the risk of a deleted review being unhidden. Good point! Should I just modify the moderation stuff to prevent a review being queued for moderation (or deleted from the queue if it's there already) if it has a deleted date or add a new boolean field ' deleted' (would still need to modify the moderation chide). I'm thinking the latter is cleaner, especially if we want to treat hidden reviews differently to deleted reviews later e.g. Allowing a user to resubmit for that app.

2. Re get_object_or_404 - the reason I differentiated between the reasons is to return more useful feedback to the client, as i've written some code in usc that uses the body of the response to give the user a clue as to why it failed. Would I still be able to do this using thie get_object_or_404 method? I.e. Saying the review does not exist is technically only correct if it can't be found using its id - what do you think?

Cheers
Aaron

« Back to merge proposal