need API for edit-review

[Michael Vogt]

The spec has says that it should be possible to re-edit your review (maybe limited for a certain time only). We need a API for this so that I can send a updated version. Maybe something like "/1.0/reviews/$review-id/+update that then ensures that the updates come from the person who wrote the original review?

Notes for QA:
  The steps to check this fix would be:
 - Submit a new review.
 - Check that you can immediately edit the review, and modify summary, rating and review text.
 - Wait a while, until the time for editing your review expires (2 hours by default, but we can set it to a lower value for testing)
 - Check that you can no longer modify your review in any way.

[Anthony Lenton (elachuni)]

This has been bumped for now.

[Michael Nelson (michael.nelson)]

Although adding /reviews/(?P<review_id>\d+)/+update/ would work (and is similar to the /delete/ that was added as part of bug 778301), we'll end up with an API that has lots of - perhaps unnecessary - verbs (delete, update etc.). We did start addressing this, removing unecessary verbs once before with bug 690551.

Basically, it would be possible for use the http methods so that:
1) GET /reviews/(?P<review_id>\d+)/ to get a review
2) PUT /reviews/(?P<review_id>\d+)/ to update a review
3) DELETE /reviews/(?P<review_id>\d+)/ to delete the review (even though we don't remove it from the DB, the resource will 404 after this action, so it's appropriate, I think?).

Just one URL, with different actions. Piston already supports this with its various methods corresponding to the http methods (GET-> read, POST -> create, PUT-> update, DELETE-> delete). See the full example at https://bitbucket.org/jespern/django-piston/wiki/Home

The only issue is that we'd want (1) above to be http, while the others to redirect if necessary to https, but that doesn't seem to difficult (http://stackoverflow.com/questions/5834239/need-to-redirect-apache-for-just-certain-http-methods )

[Aaron Peachey (aaronp)]

In terms of the db and backend implementation my immediate thought is to create a new table for deleted (due to modification) reviews so they can stay on record. The table would act pretty much like an audit history table in a normal commercial app db and the 'modified' review would just replace it in the main Reviews table.

This way, if there is any questionable content in any of the earlier reviews and an issue arises later, we still retain a record of everything the user has submitted, just in case. By moving it to a different table we would not be unnecessarily bloating the main reviews table.

A couple of other points:
a. Do we need to limit the number of times a user can modify their review?
b. If a review has been flagged and is awaiting moderation, how do we handle user trying to modify it?
c. Should there be a time limit on how long after submitting the review a user is able to modify it before it is 'locked' for modification?

[Aaron Peachey (aaronp)]

Just another thought on my questions (a) and (c) from comment # 3.

We need to be mindful of that fact that if a user is prevented from modifying their review for any reason (e.g. over time limit, over modify count limit) then with the current implementation of delete review, they could simply work around the restriction by deleting and resubmitting their review modified.

This means we would either need to:
(1) impose similar limitations on delete to those which we want to impose on modify, or
(2) allow modification without (much) limitation

[Michael Nelson (michael.nelson)]

On Sat, May 7, 2011 at 11:57 AM, Aaron Peachey <email address hidden> wrote:

> In terms of the db and backend implementation my immediate thought is to
> create a new table for deleted (due to modification) reviews so they can
> stay on record. The table would act pretty much like an audit history
> table in a normal commercial app db and the 'modified' review would just
> replace it in the main Reviews table.
>

Hi Aaron (sorry for not getting back earlier - UDS and all that). Do we need
to keep an audit history if we only allow editing of a review for the first
few hours?

>
> This way, if there is any questionable content in any of the earlier
> reviews and an issue arises later, we still retain a record of
> everything the user has submitted, just in case. By moving it to a
> different table we would not be unnecessarily bloating the main reviews
> table.
>
> A couple of other points:
> a. Do we need to limit the number of times a user can modify their review?
>

I don't think so - I think enabling them to edit as many times as they want
for the first X minutes/hours after creation is ok? If they need to re-write
it after that, they can delete it and start again as you mention.

> b. If a review has been flagged and is awaiting moderation, how do we
> handle user trying to modify it?
>

Good point - I think if it has been manually flagged by someone, we should
not allow it to be edited until it has been moderated. What do you think?

> c. Should there be a time limit on how long after submitting the review a
> user is able to modify it before it is 'locked' for modification?
>

Yes, I think that would make most sense.

[Michael Nelson (michael.nelson)]

On Sun, May 15, 2011 at 12:50 PM, Aaron Peachey <email address hidden> wrote:

> Just another thought on my questions (a) and (c) from comment # 3.
>
> We need to be mindful of that fact that if a user is prevented from
> modifying their review for any reason (e.g. over time limit, over modify
> count limit) then with the current implementation of delete review, they
> could simply work around the restriction by deleting and resubmitting
> their review modified.
>

Is that a problem though? If they've got lots of "likes" for their review,
then they'll only delete it if they think the modification is worthwhile?

>
> This means we would either need to:
> (1) impose similar limitations on delete to those which we want to impose
> on modify, or
>

Do you mean to not allow deletion after a certain time? I don't see why
(yet) :)

> (2) allow modification without (much) limitation
>

What is the problem with always allowing deleting of own review (you lose
all your "likes", hrm, or conversely, your "dislikes"). If we really wanted
to, we could add logic such as: if a person submits a review for an app for
which they've already got a non-useful deleted review, flag it as moderated
automatically? Just a thought.

[Aaron Peachey (aaronp)]

I was approaching this a little differently, thinking that we would allow the modify indefinitely but I'm on board with the approach you have outlined, as it makes more sense.

I like your idea of auto-moderation if a user tries to delete and re-submit a non-useful review, but wonder whether that might be overkill. I'd say if a review is considered non-useful then they resubmit another one that is not particularly useful (but not offensive or a violation) then we are unlikely to make the call to moderate it because it will ultimately be up to the community as to whether it's useful or not.

Perhaps as a separate exercise we should consider doing a regular bulk deletion of reviews that are 100% un-useful (maybe with > 2 votes) and have been around for a defined period of time.

For flagged reviews awaiting moderation, I agree, we should not allow editing until moderation occurs. I suspect this will be a pretty rare case if we only allow an hour or two to modify the review.

Re: how long to allow moderation, how do we make this call? It's obviously something we can easily code now and change later so not too urgent. I'm thinking 2 hours would be the absolute maximum.

[Michael Nelson (michael.nelson)]

On Tue, May 17, 2011 at 11:59 AM, Aaron Peachey <email address hidden> wrote:

> I like your idea of auto-moderation if a user tries to delete and re-
> submit a non-useful review, but wonder whether that might be overkill.
>

Yes, it was just a "if we really wanted to" - I don't think we should worry
about that now.

>
> Perhaps as a separate exercise we should consider doing a regular bulk
> deletion of reviews that are 100% un-useful (maybe with > 2 votes) and
> have been around for a defined period of time.
>

Yep, that'd be worth thinking about when we get to that point.

>
> Re: how long to allow moderation, how do we make this call? It's
> obviously something we can easily code now and change later so not too
> urgent. I'm thinking 2 hours would be the absolute maximum.
>

Normally we make that kind of stuff a standard setting (see
django_project/schema.py and django_project/config/main.cfg). If you think
it's worth a run-time-changeable setting, there's also the RNRSettings
model... see what you think.

Thanks!

[Dave Morley (davmor2)]

The USC display isn't updated locally unless you close and reopen USC this could get fairly confusing very quickly. Currently checking with mvo about how he wants to handle this before writting a bug will attach bug number here once one is done.

[Aaron Peachey (aaronp)]

I think I know what this issue is, not that the update doesn't occur, but moreso that the old copy stays in the screen even though the new one is added.
I wrote the modify and delete code a long time ago and since then we received pagination support in USC which meant we needed to tweak the existing submit/flag/vote code to prevent duplication after a successful submit (see bug LP: #794060) with the new way the callback works after pagination was added.

I don't think we have made the same tweak on the modify/delete code since then so it's probably using the old callback method and 'duplicating' the reviews by not removing the original before inserting the updated version.
There are some helper functions in the UIReview class that should make this very simple to fix.

[Dave Morley (davmor2)]

bug 807010 is for the edit