[PATCH v4 0/5] patch-list: improve usability of list action bar

Raxel Gutierrez raxel at google.com
Tue Aug 24 04:28:28 AEST 2021


This series is a revision to the previous version of the patch series 
[1]. The revision mainly addresses comments from the previous series. 
Also, the JS Cookie library and rest.js file have already landed from a 
different patch series.

Since v3:
- Patch 1: split from [v3 1/5] to only cover renaming of the patch-list template
- Patch 2: split from [v3 1/5] to only cover separating js code from patch-list
- Patch 3: split from [v3 1/5] to only cover refactoring patch forms
- Patch 4: no changes
- Patch 5: only show inline dropdown changes to users that can edit the patch

[1] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006996.html

Raxel Gutierrez (5):
  patch-list: clean up patch-list page
  patch-list: move js code to separate file
  patch-list: move and refactor patch-forms
  patch-list: style modification forms as an action bar
  patch-list: add inline dropdown for delegate and state one-off changes

 htdocs/README.rst                             |  12 +
 htdocs/css/style.css                          |  87 +++++--
 htdocs/js/bundle.js                           |  12 +-
 htdocs/js/patch-list.js                       |  60 +++++
 patchwork/forms.py                            |  56 ++++-
 .../patchwork/partials/patch-forms.html       |  45 ++++
 .../patchwork/partials/patch-list.html        | 218 ++++++------------
 patchwork/templates/patchwork/submission.html |  89 +------
 patchwork/tests/views/test_bundles.py         |  46 ++--
 patchwork/tests/views/test_patch.py           |   6 +-
 patchwork/views/__init__.py                   |  81 ++++---
 patchwork/views/bundle.py                     |   2 +-
 patchwork/views/patch.py                      |  35 +--
 13 files changed, 396 insertions(+), 353 deletions(-)
 create mode 100644 htdocs/js/patch-list.js
 create mode 100644 patchwork/templates/patchwork/partials/patch-forms.html

Range-diff against v3:
1:  859789e3 < -:  -------- static: add JS Cookie Library to get csrftoken for fetch requests
-:  -------- > 1:  efd4e7fe patch-list: clean up patch-list page
-:  -------- > 2:  08388a92 patch-list: move js code to separate file
2:  2ac10c89 ! 3:  f85cac5b patch-list: clean up patch-list page and refactor patch forms
    @@ Metadata
     Author: Raxel Gutierrez <raxel at google.com>
     
      ## Commit message ##
    -    patch-list: clean up patch-list page and refactor patch forms
    +    patch-list: move and refactor patch-forms
     
    -    Clean up the patch-list page by moving forms to a new template file
    -    patch-forms.html and move them to the top of the page, add ids to
    -    table cells, and rename selectors using hyphen delimited strings where
    -    the relevant changes were made. Also, create a partial template for
    -    errors that render with form submission. These changes improve the
    -    discoverability of the patch-list forms and make the code healthier,
    -    ready for change, and overall more readable.
    +    Move patch forms in patch-list and detail page to a new template file
    +    patch-forms.html and move them to the top of the patch-list page to
    +    improve their discoverability.
     
    -    Also, move patch-list related js code to a new patch-list.js file, to
    -    make the JavaScript easy to read and change in one place. This makes
    -    automatic code formatting easier, makes it more straightforward to
    -    measure test coverage and discover opportunities for refactoring, and
    -    simplifies a possible future migration to TypeScript if the project
    -    chooses to go in that direction.
    -
    -    No user-visible change should be noticed since this change does not
    -    make stye changes. Refactor forms.py, __init__.py, patch.py, and
    -    test_bundles.py files so that the shared bundle form in patch-forms.html
    -    works for both the patch-list and patch-detail pages. In particular, the
    -    changes normalize the behavior of the error and update messages of the
    -    patch forms and updates tests to reflect the changes. Overall, these
    -    changes make patch forms ready for change and more synchronized in their
    -    behavior. More specifically:
    +    Refactor forms.py, __init__.py, patch.py, and test_bundles.py files so
    +    that the shared bundle form in patch-forms.html works for both the
    +    patch-list and patch-detail pages. In particular, the changes normalize
    +    the behavior of the error and update messages of the patch forms and
    +    updates tests to reflect the changes. Overall, these changes make patch
    +    forms ready for change and more synchronized in their behavior. More
    +    specifically:
     
         - Previously patch forms changes were separated between the patch-detail
           and patch-list pages. Thus, desired changes to the patch forms
    @@ Commit message
     
         - Previously the patch forms in patch-list.html handled error and
           update messages through views in patch.py, whereas the patch forms in
    -      submission handled the messages with forms.py. Now, with a single
    +      submission.html handled the messages with forms.py. Now, with a single
           patch forms component in patch-forms.html, forms.py is set to handle
           the messages and handle form validation for both pages.
     
    - ## htdocs/README.rst ##
    -@@ htdocs/README.rst: js
    -   :GitHub: https://github.com/js-cookie/js-cookie/
    -   :Version: 3.0.0
    - 
    -+``patch-list.js.``
    -+
    -+  Event helpers and other application logic for patch-list.html. These
    -+  support patch list manipulation (e.g. inline dropdown changes).
    -+
    -+  Part of Patchwork.
    -+
    - ``selectize.min.js``
    - 
    -   Selectize is the hybrid of a ``textbox`` and ``<select>`` box. It's jQuery
    -
    - ## htdocs/css/style.css ##
    -@@ htdocs/css/style.css: a.colinactive:hover {
    - div.filters {
    - }
    - 
    --div.patchforms {
    -+div.patch-forms {
    -     margin-top: 1em;
    - }
    - 
    -@@ htdocs/css/style.css: table.patchmeta tr th, table.patchmeta tr td {
    - }
    - 
    - /* checks forms */
    --/* TODO(stephenfin): Merge this with 'div.patchform' rules */
    -+/* TODO(stephenfin): Merge this with 'div.patch-form' rules */
    - .checks {
    -     border: 1px solid gray;
    -     margin: 0.5em 1em;
    -@@ htdocs/css/style.css: table.bundlelist td
    - }
    - 
    - /* forms that appear for a patch */
    --div.patchform {
    -+div.patch-form {
    -     border: thin solid #080808;
    -     padding-left: 0.6em;
    -     padding-right: 0.6em;
    -@@ htdocs/css/style.css: div.patchform {
    -     margin: 0.5em 5em 0.5em 10px;
    - }
    - 
    --div.patchform h3 {
    -+div.patch-form h3 {
    -     margin-top: 0em;
    -     margin-left: -0.6em;
    -     margin-right: -0.6em;
    -@@ htdocs/css/style.css: div.patchform h3 {
    -     font-size: 100%;
    - }
    - 
    --div.patchform ul {
    -+div.patch-form ul {
    -     list-style-type: none;
    -     padding-left: 0.2em;
    -     margin-top: 0em;
    -
    - ## htdocs/js/patch-list.js (new) ##
    -@@
    -+$( document ).ready(function() {
    -+    $("#patchlist").stickyTableHeaders();
    -+
    -+    $("#check-all").change(function(e) {
    -+        if(this.checked) {
    -+            $("#patchlist > tbody").checkboxes("check");
    -+        } else {
    -+            $("#patchlist > tbody").checkboxes("uncheck");
    -+        }
    -+        e.preventDefault();
    -+    });
    -+});
    -
      ## patchwork/forms.py ##
     @@
    - # Copyright (C) 2008 Jeremy Kerr <jk at ozlabs.org>
    - #
    - # SPDX-License-Identifier: GPL-2.0-or-later
    --
      from django.contrib.auth.models import User
      from django import forms
      from django.db.models import Q
    @@ patchwork/forms.py
      from patchwork.models import Bundle
     @@ patchwork/forms.py: class EmailForm(forms.Form):
      
    - 
      class BundleForm(forms.ModelForm):
    -+    field_mapping = {'name': 'bundle_name'}
          name = forms.RegexField(
     -        regex=r'^[^/]+$', min_length=1, max_length=50, label=u'Name',
     +        regex=r'^[^/]+$', min_length=1, max_length=50, required=False,
              error_messages={'invalid': 'Bundle names can\'t contain slashes'})
      
    -+    # Changes the HTML 'name' attr of the input element from "name"
    -+    # (inherited from the model field 'name' of the Bundle object)
    -+    # to "bundle_name" as the server expects "bundle_name" as a
    -+    # parameter not "name" to process patch forms 'POST' requests.
    -+    def add_prefix(self, field_name):
    -+        field_name = self.field_mapping.get(field_name, field_name)
    -+        return super(BundleForm, self).add_prefix(field_name)
    -+
          class Meta:
    -         model = Bundle
    -         fields = ['name', 'public']
     @@ patchwork/forms.py: class CreateBundleForm(BundleForm):
      
          def clean_name(self):
    @@ patchwork/forms.py: class CreateBundleForm(BundleForm):
      
      
     
    - ## patchwork/templates/patchwork/list.html ##
    -@@
    - 
    - {% block body %}
    - 
    --{% if errors %}
    --<p>The following error{{ errors|length|pluralize:" was,s were" }} encountered
    --while updating patches:</p>
    --<ul class="errorlist">
    --{% for error in errors %}
    -- <li>{{ error }}</li>
    --{% endfor %}
    --</ul>
    --{% endif %}
    --
    -+{% include "patchwork/partials/errors.html" %}
    - {% include "patchwork/partials/patch-list.html" %}
    - 
    - {% endblock %}
    -
    - ## patchwork/templates/patchwork/partials/errors.html (new) ##
    -@@
    -+{% if errors %}
    -+<p>The following error{{ errors|length|pluralize:" was,s were" }} encountered
    -+while updating patches:</p>
    -+<ul class="errorlist">
    -+{% for error in errors %}
    -+ <li>{{ error }}</li>
    -+{% endfor %}
    -+</ul>
    -+{% endif %}
    -
      ## patchwork/templates/patchwork/partials/patch-forms.html (new) ##
     @@
     +<div class="patch-forms" id="patch-forms">
    @@ patchwork/templates/patchwork/partials/patch-forms.html (new)
     
      ## patchwork/templates/patchwork/partials/patch-list.html ##
     @@
    - {% load project %}
    - {% load static %}
      
    --{% include "patchwork/partials/filters.html" %}
    -+{% block headers %}
    -+  <script src="{% static "js/patch-list.js" %}"></script>
    -+{% endblock %}
    + {% include "patchwork/partials/filters.html" %}
      
     -{% include "patchwork/partials/pagination.html" %}
    -+{% include "patchwork/partials/filters.html" %}
    - 
    +-
      {% if order.editable %}
    - <table class="patchlist">
    + <table class="patch-list">
    +  <tr>
     @@
      </table>
      {% endif %}
      
     -{% if page.paginator.long_page and user.is_authenticated %}
     -<div class="floaty">
    -- <a title="jump to form" href="#patchforms">
    +- <a title="jump to form" href="#patch-forms">
     -  <span style="font-size: 120%">▾</span>
     - </a>
     -</div>
     -{% endif %}
     -
    --<script type="text/javascript">
    --$(document).ready(function() {
    --    $('#patchlist').stickyTableHeaders();
    --
    --    $('#check-all').change(function(e) {
    --        if(this.checked) {
    --            $('#patchlist > tbody').checkboxes('check');
    --        } else {
    --            $('#patchlist > tbody').checkboxes('uncheck');
    --        }
    --        e.preventDefault();
    --    });
    --});
    --</script>
    --
     -<form method="post">
     +<form id="patch-list-form" method="post">
      {% csrf_token %}
    --<input type="hidden" name="form" value="patchlistform"/>
    -+<input type="hidden" name="form" value="patch-list-form"/>
    + <input type="hidden" name="form" value="patch-list-form"/>
      <input type="hidden" name="project" value="{{project.id}}"/>
    +-<table id="patch-list" class="table table-hover table-extra-condensed table-striped pw-list"
     +<div class="patch-list-actions">
     +  {% include "patchwork/partials/patch-forms.html" %}
     +  {% include "patchwork/partials/pagination.html" %}
     +</div>
    - <table id="patchlist" class="table table-hover table-extra-condensed table-striped pw-list"
    ++<table id="patchlist" class="table table-hover table-extra-condensed table-striped pw-list"
             data-toggle="checkboxes" data-range="true">
       <thead>
    -@@ patchwork/templates/patchwork/partials/patch-list.html: $(document).ready(function() {
    +   <tr>
    +@@
      
       <tbody>
       {% for patch in page.object_list %}
    --  <tr id="patch_row:{{patch.id}}">
    -+  <tr id="patch_row:{{patch.id}}" data-patch-id="{{patch.id}}">
    +-  <tr id="patch-row:{{patch.id}}">
    ++  <tr id="patch-row:{{patch.id}}" data-patch-id="{{patch.id}}">
         {% if user.is_authenticated %}
    --   <td style="text-align: center;">
    -+   <td id="select-patch:{{patch.id}}" style="text-align: center;">
    +    <td id="select-patch:{{patch.id}}" style="text-align: center;">
          <input type="checkbox" name="patch_id:{{patch.id}}"/>
    -    </td>
    -    {% endif %}
    -@@ patchwork/templates/patchwork/partials/patch-list.html: $(document).ready(function() {
    -     </button>
    -    </td>
    -    {% endif %}
    --   <td>
    -+   <td id="patch-name:{{patch.id}}">
    -     <a href="{% url 'patch-detail' project_id=project.linkname msgid=patch.url_msgid %}">
    -      {{ patch.name|default:"[no subject]"|truncatechars:100 }}
    -     </a>
    -    </td>
    --   <td>
    -+   <td id="patch-series:{{patch.id}}">
    -     {% if patch.series %}
    -     <a href="?series={{patch.series.id}}">
    -      {{ patch.series|truncatechars:100 }}
    -     </a>
    -     {% endif %}
    -    </td>
    --   <td class="text-nowrap">{{ patch|patch_tags }}</td>
    --   <td class="text-nowrap">{{ patch|patch_checks }}</td>
    --   <td class="text-nowrap">{{ patch.date|date:"Y-m-d" }}</td>
    --   <td>{{ patch.submitter|personify:project }}</td>
    --   <td>{{ patch.delegate.username }}</td>
    --   <td>{{ patch.state }}</td>
    -+   <td id="patch-tags:{{patch.id}}" class="text-nowrap">{{ patch|patch_tags }}</td>
    -+   <td id="patch-checks:{{patch.id}}" class="text-nowrap">{{ patch|patch_checks }}</td>
    -+   <td id="patch-date:{{patch.id}}" class="text-nowrap">{{ patch.date|date:"Y-m-d" }}</td>
    -+   <td id="patch-submitter:{{patch.id}}">{{ patch.submitter|personify:project }}</td>
    -+   <td id="patch-delegate:{{patch.id}}">{{ patch.delegate.username }}</td>
    -+   <td id="patch-state:{{patch.id}}">{{ patch.state }}</td>
    -   </tr>
    -  {% empty %}
    -   <tr>
    -@@ patchwork/templates/patchwork/partials/patch-list.html: $(document).ready(function() {
    +@@
      
      {% if page.paginator.count %}
      {% include "patchwork/partials/pagination.html" %}
     -
    --<div class="patchforms" id="patchforms">
    +-<div class="patch-forms" id="patch-forms">
     -
     -{% if patchform %}
    -- <div class="patchform patchform-properties">
    +- <div class="patch-form patch-form-properties">
     -  <h3>Properties</h3>
     -    <table class="form">
     -     <tr>
    @@ patchwork/templates/patchwork/partials/patch-list.html: $(document).ready(functi
     -{% endif %}
     -
     -{% if user.is_authenticated %}
    -- <div class="patchform patchform-bundle">
    +- <div class="patch-form patch-form-bundle">
     -  <h3>Bundling</h3>
     -   <table class="form">
     -    <tr>
    @@ patchwork/templates/patchwork/partials/patch-list.html: $(document).ready(functi
      </form>
     
      ## patchwork/templates/patchwork/submission.html ##
    -@@ patchwork/templates/patchwork/submission.html: function toggle_div(link_id, headers_id, label_show, label_hide)
    - }
    - </script>
    - 
    -+{% include "patchwork/partials/errors.html" %}
    -+
    - <div>
    -   {% include "patchwork/partials/download-buttons.html" %}
    -   <h1>{{ submission.name }}</h1>
    -@@ patchwork/templates/patchwork/submission.html: function toggle_div(link_id, headers_id, label_show, label_hide)
    +@@
      {% endif %}
      </table>
      
    --<div class="patchforms">
    +-<div class="patch-forms">
     -{% if patchform %}
    -- <div class="patchform patchform-properties">
    +- <div class="patch-form patch-form-properties">
     -  <h3>Patch Properties</h3>
     -   <form method="post">
     -    {% csrf_token %}
    @@ patchwork/templates/patchwork/submission.html: function toggle_div(link_id, head
     -{% endif %}
     -
     -{% if createbundleform %}
    -- <div class="patchform patchform-bundle">
    +- <div class="patch-form patch-form-bundle">
     -  <h3>Bundling</h3>
     -   <table class="form">
     -    <tr>
    @@ patchwork/templates/patchwork/submission.html: function toggle_div(link_id, head
      <h2>Pull-request</h2>
     
      ## patchwork/tests/views/test_bundles.py ##
    -@@ patchwork/tests/views/test_bundles.py: class BundleUpdateTest(BundleTestBase):
    -         data = {
    -             'form': 'bundle',
    -             'action': 'update',
    --            'name': newname,
    -+            'bundle_name': newname,
    -             'public': '',
    -         }
    -         response = self.client.post(bundle_url(self.bundle), data)
    -@@ patchwork/tests/views/test_bundles.py: class BundleUpdateTest(BundleTestBase):
    -         data = {
    -             'form': 'bundle',
    -             'action': 'update',
    --            'name': self.bundle.name,
    -+            'bundle_name': self.bundle.name,
    -             'public': 'on',
    -         }
    -         response = self.client.post(bundle_url(self.bundle), data)
    -@@ patchwork/tests/views/test_bundles.py: class BundlePublicModifyTest(BundleTestBase):
    -         data = {
    -             'form': 'bundle',
    -             'action': 'update',
    --            'name': newname,
    -+            'bundle_name': newname,
    -         }
    -         self.bundle.name = oldname
    -         self.bundle.save()
     @@ patchwork/tests/views/test_bundles.py: class BundleCreateFromListTest(BundleTestBase):
    - 
          def test_create_empty_bundle(self):
              newbundlename = 'testbundle-new'
    --        params = {'form': 'patchlistform',
    -+        params = {'form': 'patch-list-form',
    -                   'bundle_name': newbundlename,
    +         params = {'form': 'patch-list-form',
    +-                  'bundle_name': newbundlename,
    ++                  'name': newbundlename,
                        'action': 'Create',
                        'project': self.project.id}
    + 
     @@ patchwork/tests/views/test_bundles.py: class BundleCreateFromListTest(BundleTestBase):
    -         newbundlename = 'testbundle-new'
              patch = self.patches[0]
      
    --        params = {'form': 'patchlistform',
    -+        params = {'form': 'patch-list-form',
    -                   'bundle_name': newbundlename,
    +         params = {'form': 'patch-list-form',
    +-                  'bundle_name': newbundlename,
    ++                  'name': newbundlename,
                        'action': 'Create',
                        'project': self.project.id,
    +                   'patch_id:%d' % patch.id: 'checked'}
     @@ patchwork/tests/views/test_bundles.py: class BundleCreateFromListTest(BundleTestBase):
    - 
              n_bundles = Bundle.objects.count()
      
    --        params = {'form': 'patchlistform',
    -+        params = {'form': 'patch-list-form',
    -                   'bundle_name': '',
    +         params = {'form': 'patch-list-form',
    +-                  'bundle_name': '',
    ++                  'name': '',
                        'action': 'Create',
                        'project': self.project.id,
    +                   'patch_id:%d' % patch.id: 'checked'}
     @@ patchwork/tests/views/test_bundles.py: class BundleCreateFromListTest(BundleTestBase):
    -         newbundlename = 'testbundle-dup'
              patch = self.patches[0]
      
    --        params = {'form': 'patchlistform',
    -+        params = {'form': 'patch-list-form',
    -                   'bundle_name': newbundlename,
    +         params = {'form': 'patch-list-form',
    +-                  'bundle_name': newbundlename,
    ++                  'name': newbundlename,
                        'action': 'Create',
                        'project': self.project.id,
    +                   'patch_id:%d' % patch.id: 'checked'}
     @@ patchwork/tests/views/test_bundles.py: class BundleCreateFromListTest(BundleTestBase):
                  params)
      
    @@ patchwork/tests/views/test_bundles.py: class BundleCreateFromListTest(BundleTest
              self.assertEqual(bundle.patches.count(), 1)
      
     @@ patchwork/tests/views/test_bundles.py: class BundleCreateFromPatchTest(BundleTestBase):
    -         newbundlename = 'testbundle-new'
              patch = self.patches[0]
      
    --        params = {'name': newbundlename,
    +         params = {'name': newbundlename,
     -                  'action': 'createbundle'}
    -+        params = {'bundle_name': newbundlename,
     +                  'action': 'Create'}
      
              response = self.client.post(
                  reverse('patch-detail',
     @@ patchwork/tests/views/test_bundles.py: class BundleCreateFromPatchTest(BundleTestBase):
    -         newbundlename = self.bundle.name
              patch = self.patches[0]
      
    --        params = {'name': newbundlename,
    +         params = {'name': newbundlename,
     -                  'action': 'createbundle'}
    -+        params = {'bundle_name': newbundlename,
     +                  'action': 'Create'}
      
              response = self.client.post(
                  reverse('patch-detail',
    -@@ patchwork/tests/views/test_bundles.py: class BundleAddFromListTest(BundleTestBase):
    - 
    -     def test_add_to_empty_bundle(self):
    -         patch = self.patches[0]
    --        params = {'form': 'patchlistform',
    -+        params = {'form': 'patch-list-form',
    -                   'action': 'Add',
    -                   'project': self.project.id,
    -                   'bundle_id': self.bundle.id,
    -@@ patchwork/tests/views/test_bundles.py: class BundleAddFromListTest(BundleTestBase):
    -     def test_add_to_non_empty_bundle(self):
    -         self.bundle.append_patch(self.patches[0])
    -         patch = self.patches[1]
    --        params = {'form': 'patchlistform',
    -+        params = {'form': 'patch-list-form',
    -                   'action': 'Add',
    -                   'project': self.project.id,
    -                   'bundle_id': self.bundle.id,
    -@@ patchwork/tests/views/test_bundles.py: class BundleAddFromListTest(BundleTestBase):
    -         count = self.bundle.patches.count()
    -         patch = self.patches[0]
    - 
    --        params = {'form': 'patchlistform',
    -+        params = {'form': 'patch-list-form',
    -                   'action': 'Add',
    -                   'project': self.project.id,
    -                   'bundle_id': self.bundle.id,
    -@@ patchwork/tests/views/test_bundles.py: class BundleAddFromListTest(BundleTestBase):
    -         count = self.bundle.patches.count()
    -         patch = self.patches[0]
    - 
    --        params = {'form': 'patchlistform',
    -+        params = {'form': 'patch-list-form',
    -                   'action': 'Add',
    -                   'project': self.project.id,
    -                   'bundle_id': self.bundle.id,
     @@ patchwork/tests/views/test_bundles.py: class BundleAddFromPatchTest(BundleTestBase):
      
          def test_add_to_empty_bundle(self):
    @@ patchwork/tests/views/test_bundles.py: class BundleAddFromPatchTest(BundleTestBa
      
              self.assertEqual(self.bundle.patches.count(), 2)
     @@ patchwork/tests/views/test_bundles.py: class BundleInitialOrderTest(BundleTestBase):
    -         newbundlename = 'testbundle-new'
      
              # need to define our querystring explicity to enforce ordering
    --        params = {'form': 'patchlistform',
    -+        params = {'form': 'patch-list-form',
    -                   'bundle_name': newbundlename,
    +         params = {'form': 'patch-list-form',
    +-                  'bundle_name': newbundlename,
    ++                  'name': newbundlename,
                        'action': 'Create',
                        'project': self.project.id,
    -
    - ## patchwork/tests/views/test_patch.py ##
    -@@ patchwork/tests/views/test_patch.py: class PatchViewTest(TestCase):
    - 
    - class PatchUpdateTest(TestCase):
    - 
    --    properties_form_id = 'patchform-properties'
    -+    properties_form_id = 'patch-form-properties'
    - 
    -     def setUp(self):
    -         self.project = create_project()
    -@@ patchwork/tests/views/test_patch.py: class PatchUpdateTest(TestCase):
    -         self.base_data = {
    -             'action': 'Update',
    -             'project': str(self.project.id),
    --            'form': 'patchlistform',
    -+            'form': 'patch-list-form',
    -             'archived': '*',
    -             'delegate': '*',
    -             'state': '*'
    +                   }
     
      ## patchwork/views/__init__.py ##
     @@
    @@ patchwork/views/__init__.py: class Order(object):
          user = request.user
      
          if action == 'create':
    -         bundle_name = data['bundle_name'].strip()
    +-        bundle_name = data['bundle_name'].strip()
     -        if '/' in bundle_name:
     -            return ['Bundle names can\'t contain slashes']
     -
    @@ patchwork/views/__init__.py: class Order(object):
     -        if Bundle.objects.filter(owner=user, name=bundle_name).count() > 0:
     -            return ['You already have a bundle called "%s"' % bundle_name]
     -
    ++        bundle_name = data['name'].strip()
              bundle = Bundle(owner=user, project=project,
                              name=bundle_name)
     -        bundle.save()
    @@ patchwork/views/__init__.py: class Order(object):
     +                                              data=request.POST)
     +        if create_bundle_form.is_valid():
     +            create_bundle_form.save()
    -+            addBundlePatches(request, patches, bundle)
    ++            add_bundle_patches(request, patches, bundle)
     +            bundle.save()
    -+            create_bundle_form = CreateBundleForm()
     +            messages.success(request, 'Bundle %s created' % bundle.name)
     +        else:
     +            formErrors = json.loads(create_bundle_form.errors.as_json())
    @@ patchwork/views/__init__.py: class Order(object):
     +        if not data['bundle_id']:
     +            return ['No bundle was selected']
              bundle = get_object_or_404(Bundle, id=data['bundle_id'])
    -+        addBundlePatches(request, patches, bundle)
    ++        add_bundle_patches(request, patches, bundle)
          elif action == 'remove':
              bundle = get_object_or_404(Bundle, id=data['removed_bundle_id'])
     -
    @@ patchwork/views/__init__.py: def set_bundle(request, project, action, data, patc
     -    bundle.save()
      
     -    return []
    -+def addBundlePatches(request, patches, bundle):
    ++def add_bundle_patches(request, patches, bundle):
     +    for patch in patches:
     +        bundlepatch_count = BundlePatch.objects.filter(bundle=bundle,
     +                                                       patch=patch).count()
    @@ patchwork/views/__init__.py: def generic_list(request, project, view, view_args=
      
          if user.is_authenticated:
              # we only pass the post data to the MultiplePatchForm if that was
    -         # the actual form submitted
    -         data_tmp = None
    --        if data and data.get('form', '') == 'patchlistform':
    -+        if data and data.get('form', '') == 'patch-list-form':
    +@@ patchwork/views/__init__.py: def generic_list(request, project, view, view_args=None, filter_settings=None,
                  data_tmp = data
      
              properties_form = MultiplePatchForm(project, data=data_tmp)
    -+        if request.user.is_authenticated:
    -+            create_bundle_form = CreateBundleForm()
    ++        create_bundle_form = CreateBundleForm()
      
    --    if request.method == 'POST' and data.get('form') == 'patchlistform':
    -+    if request.method == 'POST' and data.get('form') == 'patch-list-form':
    +     if request.method == 'POST' and data.get('form') == 'patch-list-form':
              action = data.get('action', '').lower()
      
              # special case: the user may have hit enter in the 'create bundle'
    -@@ patchwork/views/__init__.py: def generic_list(request, project, view, view_args=None, filter_settings=None,
    +         # text field, so if non-empty, assume the create action:
    +-        if data.get('bundle_name', False):
    ++        if data.get('name', False):
    +             action = 'create'
    + 
              ps = Patch.objects.filter(id__in=get_patch_ids(data))
      
              if action in bundle_actions:
3:  5d5eedcf ! 4:  fbab1825 patch-list: style modification forms as an action bar
    @@ Metadata
      ## Commit message ##
         patch-list: style modification forms as an action bar
     
    -    Added styling to the new patch list html code to make the change
    -    property and bundle action forms more usable. Before [1] and after [2]
    -    images for reference.
    +    Add styling to the new patch list html code to make the change property
    +    and bundle action forms more usable. Before [1] and after [2] images for
    +    reference.
     
    -    [1] https://imgur.com/Pzelipp
    -    [2] https://imgur.com/UtNJXuf
    +    [1] https://i.imgur.com/Pzelipp.png
    +    [2] https://i.imgur.com/UtNJXuf.png
     
      ## htdocs/css/style.css ##
    -@@
    -+:root {
    -+    --light-color: #F7F7F7;
    -+}
    -+
    - h2 {
    -     font-size: 25px;
    -     margin: 18px 0 18px 0;
    -@@ htdocs/css/style.css: a.colinactive:hover {
    +@@ htdocs/css/style.css: a.col-inactive:hover {
      div.filters {
      }
      
    @@ htdocs/css/style.css: a.colinactive:hover {
     -
      /* list order manipulation */
      
    - table.patchlist tr.draghover {
    + table.patch-list tr.draghover {
     @@ htdocs/css/style.css: input#reorder-change {
      .paginator {
          text-align: right;
    @@ htdocs/css/style.css: div.patch-form ul {
          vertical-align: top;
     
      ## patchwork/forms.py ##
    -@@ patchwork/forms.py: class BundleForm(forms.ModelForm):
    -     field_mapping = {'name': 'bundle_name'}
    +@@ patchwork/forms.py: class EmailForm(forms.Form):
    + class BundleForm(forms.ModelForm):
          name = forms.RegexField(
              regex=r'^[^/]+$', min_length=1, max_length=50, required=False,
     -        error_messages={'invalid': 'Bundle names can\'t contain slashes'})
    @@ patchwork/forms.py: class BundleForm(forms.ModelForm):
     +            attrs={'class': 'create-bundle',
     +                   'placeholder': 'Bundle name'}))
      
    -     # Changes the HTML 'name' attr of the input element from "name"
    -     # (inherited from the model field 'name' of the Bundle object)
    +     class Meta:
    +         model = Bundle
     @@ patchwork/forms.py: class PatchForm(forms.ModelForm):
          def __init__(self, instance=None, project=None, *args, **kwargs):
              super(PatchForm, self).__init__(instance=instance, *args, **kwargs)
4:  d77051d8 < -:  -------- static: add rest.js to handle requests & respective messages
5:  27290622 ! 5:  97353744 patch-list: add inline dropdown for delegate and state one-off changes
    @@ Commit message
     
         Add dropdown for the cell values of the Delegate and State columns for
         each individual patch to make one-off changes to patches. The dropdowns
    -    are only viewable when logged in and revert selections made by users
    -    that don't have permission to change a given patch's state or delegate.
    +    are only viewable if the user has patch edit permissions.
    +
         Change the generic_list method to pass the list of states and
         maintainers to the patch list view context to populate the dropdown
         options. The static patch-list.js file now uses the modularity of the
         fetch request and update/error messages handling of rest.js.
     
    +    TODO: The loading of the patch-list page is very slow now because it
    +    seems that for each call to check if a patch is editable by a user, the
    +    db is accessed. Changes in the backend need to be made so this is done
    +    with only done with only one call to the db.
    +
    + ## htdocs/README.rst ##
    +@@ htdocs/README.rst: js
    + 
    +   Part of Patchwork.
    + 
    ++``patch-list.js.``
    ++  Event helpers and other application logic for patch-list.html. These
    ++  support patch list manipulation (e.g. inline dropdown changes).
    ++
    ++  Part of Patchwork.
    ++
    + ``selectize.min.js``
    +   Selectize is the hybrid of a ``textbox`` and ``<select>`` box. It's jQuery
    +   based and it has autocomplete and native-feeling keyboard navigation; useful
    +
      ## htdocs/js/patch-list.js ##
     @@
     +import { updateProperty } from "./rest.js";
     +
      $( document ).ready(function() {
    +-    $("#patch-list").stickyTableHeaders();
     +    let inlinePropertyDropdowns = $("td > select[class^='change-property-']");
     +    $(inlinePropertyDropdowns).each(function() {
     +        // Store previous dropdown selection
    @@ htdocs/js/patch-list.js
     +        const property = event.target.getAttribute("value");
     +        const { url, data } = getPatchProperties(event.target, property);
     +        const updateMessage = {
    -+            'none': "No patches updated",
    -+            'some': "1 patch updated",
    ++            'error': "No patches updated",
    ++            'success': "1 patch updated",
     +        };
    -+        updateProperty(url, data, updateMessage).then(is_success => {
    -+            if (!is_success) {
    ++        updateProperty(url, data, updateMessage).then(isSuccess => {
    ++            if (!isSuccess) {
     +                // Revert to previous selection
     +                $(event.target).val($(event.target).data("prevProperty"));
     +            } else {
    @@ htdocs/js/patch-list.js
     +        });
     +    });
     +
    -     $("#patchlist").stickyTableHeaders();
    ++    $("#patchlist").stickyTableHeaders();
      
          $("#check-all").change(function(e) {
    +         if(this.checked) {
     @@ htdocs/js/patch-list.js: $( document ).ready(function() {
              }
              e.preventDefault();
          });
    +-});
    + \ No newline at end of file
     +
     +    /**
    -+     * Returns the data to make property changes to a patch through fetch request.
    ++     * Returns the data to make property changes to a patch through PATCH request.
     +     * @param {Element} propertySelect Property select element modified.
     +     * @param {string} property Patch property modified (e.g. "state", "delegate")
     +     * @return {{property: string, value: string}}
    @@ htdocs/js/patch-list.js: $( document ).ready(function() {
     +        const data = {};
     +        data[property] = propertyValue;
     +        return {
    -+            "url": "/api/patches/" + patchId + "/",
    ++            "url": `/api/patches/${patchId}/`,
     +            "data": data,
     +        };
     +    }
    - });
    -
    - ## htdocs/js/rest.js ##
    -@@ htdocs/js/rest.js: function handleErrorMessages(errorMessage) {
    -     container.prepend(errorHeader);
    - }
    - 
    --export { updateProperty, handleUpdateMessages, handleUpdateMessages};
    - \ No newline at end of file
    -+export { updateProperty };
    - \ No newline at end of file
    ++});
     
      ## patchwork/templates/patchwork/partials/patch-list.html ##
     @@
    @@ patchwork/templates/patchwork/partials/patch-list.html
     -   <td id="patch-delegate:{{patch.id}}">{{ patch.delegate.username }}</td>
     -   <td id="patch-state:{{patch.id}}">{{ patch.state }}</td>
     +   <td id="patch-delegate:{{patch.id}}">
    -+    <td id="patch-delegate:{{patch.id}}">
    -+      {% if user.is_authenticated %}
    -+        <select class="change-property-delegate" value="delegate">
    -+          {% if not patch.delegate.username %}
    -+            <option value="*" selected>No delegate</option>
    ++    {% if patch.is_editable %}
    ++      <select class="change-property-delegate" value="delegate">
    ++        {% if not patch.delegate.username %}
    ++          <option value="*" selected>No delegate</option>
    ++        {% else %}
    ++          <option value="*" selected>{{ patch.delegate.username }}</option>
    ++        {% endif %}
    ++        {% for maintainer in maintainers %}
    ++          <option value="{{ maintainer.id }}">{{ maintainer.name }}</option>
    ++        {% endfor %}
    ++      </select>
    ++    {% else %}
    ++      {{ patch.delegate.username }}
    ++    {% endif %}
    ++   </td>
    ++   <td id="patch-state:{{patch.id}}">
    ++    {% if patch.is_editable %}
    ++      <select class="change-property-state" value="state">
    ++        {% for state in states %}
    ++          {% if state.name == patch.state.name %}
    ++            <option value="{{ patch.state.ordering }}" selected>{{ patch.state }}</option>
     +          {% else %}
    -+            <option value="*">No delegate</option>
    ++            <option value="{{ state.ordering  }}">{{ state.name }}</option>
     +          {% endif %}
    -+          {% for maintainer in maintainers %}
    -+            {% if maintainer.name == patch.delegate.username %}
    -+              <option value="{{ patch.delegate.username.id }}" selected>{{ patch.delegate.username }}</option>
    -+            {% else %}
    -+              <option value="{{ maintainer.id }}">{{ maintainer.name }}</option>
    -+            {% endif %}
    -+          {% endfor %}
    -+        </select>
    -+      {% else %}
    -+        {{ patch.delegate.username }}
    -+      {% endif %}
    -+     </td>
    -+     <td id="patch-state:{{patch.id}}">
    -+      {% if user.is_authenticated %}
    -+        <select class="change-property-state" value="state">
    -+          {% for state in states %}
    -+            {% if state.name == patch.state.name %}
    -+              <option value="{{ patch.state.ordering }}" selected>{{ patch.state }}</option>
    -+            {% else %}
    -+              <option value="{{ state.ordering  }}">{{ state.name }}</option>
    -+            {% endif %}
    -+          {% endfor %}
    -+        </select>
    -+      {% else %}
    -+        {{ patch.state }}
    -+      {% endif %}
    -+     </td>
    ++        {% endfor %}
    ++      </select>
    ++    {% else %}
    ++      {{ patch.state }}
    ++    {% endif %}
    ++   </td>
        </tr>
       {% empty %}
        <tr>
    @@ patchwork/views/__init__.py: def generic_list(request, project, view, view_args=
          }
      
          # pagination
    -
    - ## templates/base.html ##
    -@@
    -   {% endfor %}
    -   </div>
    - {% endif %}
    --  <div class="container-fluid">
    -+  <div id="main-content" class="container-fluid">
    - {% block body %}
    - {% endblock %}
    -   </div>
    +@@ patchwork/views/__init__.py: def generic_list(request, project, view, view_args=None, filter_settings=None,
    +         Prefetch('check_set', queryset=Check.objects.only(
    +             'context', 'user_id', 'patch_id', 'state', 'date')))
    + 
    ++    for patch in patches:
    ++        patch.is_editable = patch.is_editable(user)
    ++
    +     paginator = Paginator(request, patches)
    + 
    +     context.update({
-- 
2.33.0.rc2.250.ged5fa647cd-goog



More information about the Patchwork mailing list