Clean up and optimize logic for copying field state on publish.

Review Request #13066 — Created May 25, 2023 and submitted

Information

Review Board
release-6.x

Reviewers

The code for copying state on publish is pretty old, and had some very
inefficient database operations. Particularly for screenshots and file
attachments.

While care was done to avoid any database operations on review
requests/drafts without screenshots or file attachments, the actual
copying logic itself was messy:

  1. Much of the code between screenshots and file attachments were
    duplicated.

  2. Full-model fetches were performed when we only needed small pieces of
    information.

  3. Copying of items was done one-by-one, instead of batch-copying.

  4. We looped over objects twice for captions, which wasn't necessary.

  5. We redefined update_list every time we performed a publish.

This change redoes much of this logic. There's now a single function
that can handle copying state for both screenshots and file attachments.
This takes in parameters for the counts and relations, figures out what
minimum information we need to fetch, and performs the copy as
efficiently as possible.

Some related code has been cleaned up to avoid re-fetching attributes
line after line after line.

While this won't offer a massive speed improvement for most deployments,
it will at least reduce the work the database needs to perform whenever
we publish going forward.

Unit tests pass.

Summary ID
Clean up and optimize logic for copying field state on publish.
The code for copying state on publish is pretty old, and had some very inefficient database operations. Particularly for screenshots and file attachments. While care was done to avoid any database operations on review requests/drafts without screenshots or file attachments, the actual copying logic itself was messy: 1. Much of the code between screenshots and file attachments were duplicated. 2. Full-model fetches were performed when we only needed small pieces of information. 3. Copying of items was done one-by-one, instead of batch-copying. 4. We looped over objects twice for captions, which wasn't necessary. 4. We redefined `update_list` every time we performed a publish. This change redoes much of this logic. There's now a single function that can handle copying state for both screenshots and file attachments. This takes in parameters for the counts and relations, figures out what minimum information we need to fetch, and performs the copy as efficiently as possible. Some related code has been cleaned up to avoid re-fetching attributes line after line after line. While this won't offer a massive speed improvement for most deployments, it will at least reduce the work the database needs to perform whenever we publish going forward.
f4219a7ecb3ead094b2834e366ba05ccd18ae6a5
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (31e100d)
Loading...