[PATCH v2 0/6] Remove 'Submission' model

[Stephen Finucane]

On Thu, 2020-04-09 at 18:12 +0100, Stephen Finucane wrote:
> Time to clean up the Submission-Patch-CoverLetter mess and unblock
> features like Veronika's reworked tagging infrastructure series.
> 
> As discussed in the commit message, this series ultimately takes us from
> this model hierarchy:
> 
>   Submission
>     Patch
>     CoverLetter
>     Comment
> 
> To this hierarchy:
> 
>   Patch
>     PatchComment
>   Cover
>     CoverComment
> 
> It does this by splitting out the cover letter-related entries from the
> Submission-CoverLetter and Comment models into two entirely new models,
> Cover and CoverComment, and then renaming Comment to PatchComment and
> combining Patch and Submission. This is all explored in more detail in
> the commit messages.
> 
> There were a couple of other approaches explored over the two years or
> so of on-and-off investigation I've had into this issue, which I've
> listed below:
> 
> - Add entirely new models
> 
>   Add new models that will replace Patch, CoverLetter and Comment, as
>   well as Submission. These could be called Patch, Cover, PatchComment
>   and CoverComment, (with some intermediate names to allow the old and
>   new models to coexist while we migrate stuff across), so:
> 
>       Submission+Patch -> Patch
>       Submission+CoverLetter -> Cover
>       Comment -> PatchComment or CoverComment, depending on parent
> 
>   Rejected because of the sheer amount of data that has to be moved
>   around for this to work. Effectively, the entire database would need
>   to be rewritten.
> 
> - Drop CoverLetter in favour of Series
> 
>   A cover letter is essentially additional metadata for a series and
>   could be folded into this.
> 
>   Rejected because of the impact on the API and UI since we'd no longer
>   have cover letter IDs to use in the URL. Doesn't give us enough to
>   justify the effort.
> 
> - Use generic relations for 'Comment'
> 
>   The contenttypes framework provided by Django allows you to map a
>   model to another model. This means we could have a single Comment
>   model that could point to either the Cover or Patch model.
> 
>   Rejected because I didn't think the complexity was worth it. I may
>   revisit this though before we merge since we'll presumably want
>   similar functionality for the reworked tags feature in the future.
> 
> - Merge Submission into Patch, rather than the other way around
> 
>   This was actually my first approach and initially seemed easier due to
>   the amount of references to the Patch model.
> 
>   Rejected because this is not currently possible using stock Django due
>   to bug #23521, which states that there is no way to change a model's
>   'bases' attribute at the moment. I had to copy code from an unmerged
>   PR, #11222, to get this to work which didn't seem ideal. In addition,
>   it turned out updating the references wasn't difficult once I realized
>   that another bug, #25530, only affected Django 1.11 and could be
>   worked around.

I've rebased and applied this series since it was a blocker for the
other work I want to get closed out for 3.0. Reviews are still welcome,
particularly on the SQL'y parts of the migrations where my lack of DB-
fu would be most evident.

Stephen

> Stephen Finucane (6):
>   trivial: Rename 'CoverLetter' references to 'Cover'
>   Remove unnecessary references to Submission model
>   Revert "Be sensible computing project patch counts"
>   models: Split 'CoverLetter' from 'Submission'
>   models: Merge 'Patch' and 'Submission'
>   models: Trivial post-merge cleanup
> 
>  docs/api/schemas/latest/patchwork.yaml        |  14 +-
>  docs/api/schemas/patchwork.j2                 |  14 +-
>  docs/api/schemas/v1.0/patchwork.yaml          |  14 +-
>  docs/api/schemas/v1.1/patchwork.yaml          |  14 +-
>  docs/api/schemas/v1.2/patchwork.yaml          |  14 +-
>  patchwork/admin.py                            |  28 +-
>  patchwork/api/comment.py                      |  56 +++-
>  patchwork/api/cover.py                        |  34 +-
>  patchwork/api/embedded.py                     |   4 +-
>  patchwork/api/event.py                        |   4 +-
>  patchwork/api/filters.py                      |  11 +-
>  patchwork/api/series.py                       |   4 +-
>  patchwork/management/commands/dumparchive.py  |   2 +-
>  patchwork/management/commands/parsearchive.py |  11 +-
>  patchwork/migrations/0042_add_cover_model.py  | 247 +++++++++++++++
>  .../migrations/0043_merge_patch_submission.py | 292 ++++++++++++++++++
>  patchwork/models.py                           | 170 ++++++----
>  patchwork/parser.py                           |  89 ++++--
>  patchwork/signals.py                          |   4 +-
>  patchwork/tests/api/test_comment.py           |  11 +-
>  patchwork/tests/api/test_cover.py             |   2 +-
>  patchwork/tests/test_detail.py                |  33 +-
>  patchwork/tests/test_mboxviews.py             |  40 ++-
>  patchwork/tests/test_parser.py                |   4 +-
>  patchwork/tests/test_series.py                |   2 +-
>  patchwork/tests/test_tags.py                  |   6 +-
>  patchwork/tests/utils.py                      |  37 ++-
>  patchwork/urls.py                             |   8 +-
>  patchwork/views/__init__.py                   |   2 +-
>  patchwork/views/comment.py                    |  43 ++-
>  patchwork/views/cover.py                      |  22 +-
>  patchwork/views/patch.py                      |  13 +-
>  patchwork/views/project.py                    |  29 +-
>  patchwork/views/utils.py                      |  21 +-
>  34 files changed, 1013 insertions(+), 286 deletions(-)
>  create mode 100644 patchwork/migrations/0042_add_cover_model.py
>  create mode 100644 patchwork/migrations/0043_merge_patch_submission.py
>