[RFC PATCH 4/4] patch-detail: add functionality to add/remove related patches

[Raxel Gutierrez]

Made a mistake with making update_patch_relations modular [1]. Tests
are failing. The following changes should be made to this patch to
fix:

diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
index 92f9038a..e1f2846d 100644
--- a/patchwork/views/patch.py
+++ b/patchwork/views/patch.py
@@ -106,7 +106,7 @@ def patch_detail(request, project_id, msgid):
                 patch, action, request.POST
             )

-            if data['related']:  # check if any ids matched
+            if data['related']['patches']:  # check if any ids matched
                 if action == 'add-related':
                     update_errors = update_patch_relations(
                         request.user.profile, patch, data
@@ -115,11 +115,13 @@ def patch_detail(request, project_id, msgid):
                     if not update_errors:
                         changed_relations += 1
                 elif action == 'remove-related':
-                    for rp in data['related']:
+                    for rp in data['related']['patches']:
                         # for removal, to-be removed patch(es)'
                         # relations are emptied
                         update_errors = update_patch_relations(
-                            request.user.profile, rp, {'related': []}
+                           request.user.profile, rp,
+                           {'related': {'patches': []}}

[1] https://lists.ozlabs.org/pipermail/patchwork/2021-August/007171.html

On Mon, Aug 23, 2021 at 10:58 AM Raxel Gutierrez <raxel at google.com> wrote:
>
> Currently, patch relations can be modified using the REST API, but the
> Patchwork UI provides no tools that allow users to create and edit patch
> relations. This patch adds that as part of the patch relations table.
> The changes involve the following details:
>
> Add handling of user input to the add/remove related patches as such:
> - Parse user input of ids and msgids by commas and whitespace
>   - Match parsed ids to patches in the db
>     - Record invalid ids to report back to the user
> - Update patch relations based on selected option to add/remove
> - Add update/error messages to web page as feedback
>
> In the future, an automated system to relate patches together would be
> ideal, meaning that this functionality should be considered a fallback
> option when the automated system has faults. At the very least, this
> manual option serves as the basic function to create and modify patch
> relations.
>
> Signed-off-by: Raxel Gutierrez <raxel at google.com>
> ---
>  patchwork/views/__init__.py | 33 +++++++++++++++++++++++++++++++
>  patchwork/views/patch.py    | 39 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+)
>
> diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
> index 3efe90cd..35c9c23c 100644
> --- a/patchwork/views/__init__.py
> +++ b/patchwork/views/__init__.py
> @@ -6,6 +6,7 @@
>  from django.contrib import messages
>  from django.shortcuts import get_object_or_404
>  from django.db.models import Prefetch
> +from django.core.exceptions import ObjectDoesNotExist
>
>  from patchwork.filters import Filters
>  from patchwork.forms import MultiplePatchForm
> @@ -323,3 +324,35 @@ def process_multiplepatch_form(request, form, action, patches, context):
>          messages.warning(request, 'No patches updated')
>
>      return errors
> +
> +
> +def get_patch_relations_data(patch, action, data):
> +    related_input = data.get('related_input', '').strip()
> +    related_ids = [id.strip('<> ') for id in related_input.split(",")]
> +    # patches that match the parsed user input ids and ids that did not match
> +    related_patches, invalid_ids = get_patches_id_msgid(related_ids)
> +
> +    if action == 'remove-related':
> +        related_patches = [
> +            p for p in patch.related.patches.all()
> +            if p in related_patches
> +        ]
> +    return ({'related': related_patches}, invalid_ids)
> +
> +
> +def get_patches_id_msgid(ids):
> +    patches = []
> +    invalid_ids = []
> +    for str_id in ids:
> +        try:
> +            id = int(str_id)
> +            try:
> +                patches.append(Patch.objects.get(id=id))
> +            except ObjectDoesNotExist:
> +                invalid_ids.append(id)
> +        except ValueError:
> +            try:
> +                patches.append(Patch.objects.get(msgid='<' + str_id + '>'))
> +            except ObjectDoesNotExist:
> +                invalid_ids.append(str_id)
> +    return (patches, invalid_ids)
> diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
> index 8e685add..92f9038a 100644
> --- a/patchwork/views/patch.py
> +++ b/patchwork/views/patch.py
> @@ -19,8 +19,10 @@ from patchwork.models import Cover
>  from patchwork.models import Patch
>  from patchwork.models import Project
>  from patchwork.views import generic_list
> +from patchwork.views import get_patch_relations_data
>  from patchwork.views.utils import patch_to_mbox
>  from patchwork.views.utils import series_patch_to_mbox
> +from patchwork.api.patch import update_patch_relations
>
>
>  def patch_list(request, project_id):
> @@ -64,6 +66,7 @@ def patch_detail(request, project_id, msgid):
>
>      form = None
>      createbundleform = None
> +    errors = []
>
>      if editable:
>          form = PatchForm(instance=patch)
> @@ -97,6 +100,41 @@ def patch_detail(request, project_id, msgid):
>                                 'Failed to add patch "%s" to bundle "%s": '
>                                 'patch is already in bundle' % (
>                                     patch.name, bundle.name))
> +        elif action in ('add-related', 'remove-related'):
> +            changed_relations = 0  # used for update message count
> +            data, invalid_ids = get_patch_relations_data(
> +                patch, action, request.POST
> +            )
> +
> +            if data['related']:  # check if any ids matched
> +                if action == 'add-related':
> +                    update_errors = update_patch_relations(
> +                        request.user.profile, patch, data
> +                    )
> +                    errors.extend(update_errors)
> +                    if not update_errors:
> +                        changed_relations += 1
> +                elif action == 'remove-related':
> +                    for rp in data['related']:
> +                        # for removal, to-be removed patch(es)'
> +                        # relations are emptied
> +                        update_errors = update_patch_relations(
> +                            request.user.profile, rp, {'related': []}
> +                        )
> +                        errors.extend(update_errors)
> +                        if not update_errors:
> +                            changed_relations += 1
> +
> +            errors.extend(
> +                ['%s is not a valid patch/msg id' % pid for pid in invalid_ids]
> +            )
> +            if changed_relations >= 1:
> +                messages.success(
> +                    request,
> +                    '%d patch relation(s) updated' % changed_relations
> +                )
> +            else:
> +                messages.warning(request, 'No patch relations updated')
>
>          # all other actions require edit privs
>          elif not editable:
> @@ -150,6 +188,7 @@ def patch_detail(request, project_id, msgid):
>      context['related_same_project'] = related_same_project
>      context['related_ids'] = related_ids
>      context['related_different_project'] = related_different_project
> +    context['errors'] = errors
>
>      return render(request, 'patchwork/submission.html', context)
>
> --
> 2.33.0.rc2.250.ged5fa647cd-goog
>