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

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

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.

Commits

Files

    Loading...