4450: ReviewRequest.last_review_activity_timestamp should be exposed in the API

samsun387 
What version are you running?


2.5.6.1


What steps will reproduce the problem?


    

  1. Create a review request
    2. 

  2. Create an issue in the review request
    3. 

  3. Check the last_updated_from field. Note that it's updated
    4. 

  4. Wait for at least one minute or longer, then fix the issue or drop the issue.
    5. 

  5. Note that the last_updated_from is not updated
    6. 



What is the expected output? What do you see instead


In Step 5, last_updated_from should be updated when an issue is closed or dropped. Otherwise it seems contradicting as the last_updated_from is updated when the issue is created


What operating system are you using? What browser?


Linux, Chrome


Please provide any additional information below.


I'm doing some integration with Reviewboard. Basically I have a script which uses python API to fetch any review request with last-updated-from within 24 hours. If the review is approved, then trigger something.


With this ticket i'm reporting, It's causing some review requests slipping through the cracks. Let's say everyone approves the review request 5 days ago, with an open issue. In this case, the last-updated-from would be 5 days ago, but it's not approved due to the open issue. Then someone drops the issue today. Now the request.approved becomes True, but last-updated-from is still 5 days ago.
chipx86
#1 chipx86 
This is intentional. It's not the creation of the issue that updates last_updated (which ?last-updated-from= queries). It's the publishing of the review. We update the timestamp when something fundamental happens on the review request, like a new draft of the review request is published or a new review/reply is published. This timestamp impacts the order in which review requests are shown in the dashboard (assuming you're sorting by Last Updated, which is the default). A review request jumping to the top of the dashboard is an indicator that there's something new there perhaps worth looking at, and changing issue statuses doesn't fall into that category (as it's primarily useful for the owner of the review request, not other reviewers).


Toggling the issue status of a comment does impact another field on the model, last_review_activity_timestamp. This value, unfortunately, is not exposed in the API, and is worth adding (along with a ?last-review-activity-from=). I'd take a patch to add that, and it'd probably be only about 3 or 4 lines of code in reviewboard/webapi/resources/review_request.py plus a couple of unit tests.
  • -New
    +Confirmed
  • -Type:Defect
    +Component:API
    +Component:Reviews
    +EasyFix
    +Type:Enhancement
  • -Fixed issue / Drop issue doesn't update the last_updated_from in Review Request
    +ReviewRequest.last_review_activity_timestamp should be exposed in the API
#2 samsun387 
Thanks for the quick response. Yes I also tried to use the last_review_timestamp(the column name in db in reviewrequest table) but found out it's not available through API. It would be great if you can expose that.


With last_review_timestamp being exposed through API, is there a way to call the API so that it returns the review requests with either last_updated_from or last_review_timestamp within 24 hours, whichever is later? Otherwise I will make two api calls to get two lists of review request and combine them.
chipx86
#3 chipx86 
We don't really have a mechanism for queries like that today. We'd have to consider how that would work, but I don't think I can make any promises there. I mean, we could perhaps add a ?last-updated-or-review-activity-from=, but that's sort of a slippery slope.
#4 samsun387 
Hi,


Just following up on this. Since it's marked as an easy fix, is there a targetted release for it at the moment?
chipx86
#5 chipx86 
Needs more thought before we can say it's easy and hand it to contributors.
  • -EasyFix