diff mbox series

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

Message ID 20210823145847.3944896-5-raxel@google.com
State RFC
Headers show
Series patch-detail: add patch relations table | expand

Commit Message

Raxel Gutierrez Aug. 23, 2021, 2:58 p.m. UTC 
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@google.com>
---
 patchwork/views/__init__.py | 33 +++++++++++++++++++++++++++++++
 patchwork/views/patch.py    | 39 +++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

Comments

Raxel Gutierrez Aug. 23, 2021, 10:50 p.m. UTC | #1 
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@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@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
>
diff mbox series

Patch

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)