Introduce new file attachment and screenshot counters for review requests.

Review Request #9242 — Created Oct. 3, 2017 and submitted

Information

Review Board
release-3.0.x
fc2d939...

Reviewers

When viewing or publishing review requests, we perform a series of
lookups on the file attachments and screenshots (active and inactive) on
the review request and on the draft. This can be up to 8 queries that
we don't really want in most cases (screenshots are never used anymore,
but remain supported for old review requests), but we had no good way of
choosing whether to perform queries for them.

ReviewRequest and ReviewRequestDraft now have new
RelationCounterFields that track how many active and inactive file
attachments and screenshots there are. This allows code to simply check
the populated counters instead of performing a series of queries,
improving performance across the product and all but guaranteeing that
we won't look up screenshots anymore.

Unit tests pass.

Checked existing drafts and review requests and saw that the counters
were populated to the correct values.

Created a new draft and saw that it had the correct counters from the
review request.

Uploaded new file attachments and saw that the draft's counters were
updated correctly.

Deleted a file attachment and saw that the draft's counters were updated
correctly.

Published the draft. Saw that the review request's counters were updated
correctly.

Description From Last Updated

E501 line too long (81 > 79 characters)

reviewbotreviewbot

There's not a lot of benefit to returning a generator here because sorted() has to create the list. In fact, …

daviddavid

E303 too many blank lines (2)

reviewbotreviewbot

E127 continuation line over-indented for visual indent

reviewbotreviewbot

E127 continuation line over-indented for visual indent

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

david
  1. 
      
  2. There's not a lot of benefit to returning a generator here because sorted() has to create the list. In fact, if someone uses this who wants a list we'd be creating it, iterating it, and creating a second. Better to just return sorted like before.

    1. It was more to keep the consistency. I'd either like to use lists for both or generators for both.

    2. Your call but it seems silly to prioritize consistency here, where it doesn't really matter to callers.

  3. 
      
chipx86
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (4602f42)
Loading...