| Message ID | 20210728152419.3588812-1-raxel@google.com |
|---|---|
| Headers | show |
| Series | patch-list: improve usability of list action bar | expand |
Hi Raxel,
A few bugs:
- if you submit a change with the dropdowns that errors out, and then
subsequently submit one that succeeds, the error messages are not
cleared.
- You've somehow ended up with two td.patch-delegate:
<td id="patch-delegate:20">
<td id="patch-delegate:20">
</td>
<td id="patch-state:20">
New
</td>
</tr>
This is throwing off column alignment: delegates now line up under the
State heading.
> For the first patch, the recently released v3.0.0 of the JS cookie
> library replaces the previous version.
- the js.cookie.min.js file is still corrupted and I redownloaded it
from a CDN. I'm not sure where the bug lies here and it's probably
not something you need to fix, but a link to the source in the commit
message would be nice.
> For the second patch, following jrnieder’s comments [2], the commit
> message is updated to better explain the benefits of the change and its
> details. Also, an unclear comment in forms.py for BundleForm is cleaned
> up to better explain what the changes do. The id for <td> table cells in
> patch-list.html are correctly differentiated by patch id. I noticed a
> bug where property changes in the patch detail page weren’t working
> because the respective “update” action was not accounted for.
>
This (patch 2) is where I'm up to in reviews atm. I'm hoping to do more soon.
> For the third patch, the “jump to form” arrow is removed [3].
>
> For the fourth patch, the `updateProperty` function now returns whether
> the update request was successful or not with `response.ok` so that
> callers can use that information and respond accordingly. The next patch
> adds a use case for the return value.
>
> For the fifth patch, the inline dropdowns are changed so that they are
> only visible to logged in users. Also, upon any unsuccessful update
> requests, the dropdown selection reverts to its previous selection.
> Since update requests to a patch’s state and delegate fields fail for
> unauthorized users, this behavior covers the case where unauthorized
> users don't see the current state of the db after they change the
> dropdown selection. This behavior is a useful example of patch four’s
> change of returning whether an update request is successful.
- they're visible to logged in users but still suggest that if I am a
regular user that I have the ability to change anyone's patch
state/delegate. I am still a bit dubious overall about inline
dropdowns in general, although I am cognisant that it's much more
discoverable this way. Either way, IMO dropdowns need to be
restricted to editable patches.
Kind regards,
Daniel
> [1] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006968.html
> [2] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006988.html
> [3] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006991.html
>
> Raxel Gutierrez (5):
> static: add JS Cookie Library to get csrftoken for fetch requests
> patch-list: clean up patch-list page and refactor patch forms
> patch-list: style modification forms as an action bar
> static: add rest.js to handle requests & respective messages
> patch-list: add inline dropdown for delegate and state one-off changes
>
> htdocs/README.rst | 23 +++
> htdocs/css/style.css | 85 +++++++--
> htdocs/js/js.cookie.min.js | 2 +
> htdocs/js/patch-list.js | 60 ++++++
> htdocs/js/rest.js | 72 ++++++++
> patchwork/forms.py | 66 +++++--
> patchwork/templates/patchwork/list.html | 11 +-
> .../templates/patchwork/partials/errors.html | 9 +
> .../patchwork/partials/patch-forms.html | 45 +++++
> .../patchwork/partials/patch-list.html | 171 ++++++------------
> patchwork/templates/patchwork/submission.html | 91 +---------
> patchwork/tests/views/test_bundles.py | 44 ++---
> patchwork/tests/views/test_patch.py | 4 +-
> patchwork/views/__init__.py | 76 ++++----
> patchwork/views/patch.py | 35 +---
> templates/base.html | 3 +-
> 16 files changed, 472 insertions(+), 325 deletions(-)
> create mode 100644 htdocs/js/js.cookie.min.js
> create mode 100644 htdocs/js/patch-list.js
> create mode 100644 htdocs/js/rest.js
> create mode 100644 patchwork/templates/patchwork/partials/errors.html
> create mode 100644 patchwork/templates/patchwork/partials/patch-forms.html
>
> Range-diff:
> 1: 2a34394 ! 1: 859789e static: add JS Cookie Library to get csrftoken for fetch requests
> @@ htdocs/README.rst: js
> + This is used to get the ``csrftoken`` cookie for AJAX requests in JavaScript.
> +
> + :GitHub: https://github.com/js-cookie/js-cookie/
> -+ :Version: 2.2.1
> ++ :Version: 3.0.0
> +
> ``selectize.min.js``
>
> @@ htdocs/README.rst: js
>
> ## htdocs/js/js.cookie.min.js (new) ##
> @@
> -+/*! js-cookie v2.2.1 | MIT */
> -+
> -+!function(a){var b;if("function"==typeof define&&define.amd&&(define(a),b=!0),"object"==typeof exports&&(module.exports=a(),b=!0),!b){var c=window.Cookies,d=window.Cookies=a();d.noConflict=function(){return window.Cookies=c,d}}}(function(){function a(){for(var a=0,b={};a<arguments.length;a++){var c=arguments[a];for(var d in c)b[d]=c[d]}return b}function b(a){return a.replace(/(%[0-9A-Z]{2})+/g,decodeURIComponent)}function c(d){function e(){}function f(b,c,f){if("undefined"!=typeof document){f=a({path:"/"},e.defaults,f),"number"==typeof f.expires&&(f.expires=new Date(1*new Date+864e5*f.expires)),f.expires=f.expires?f.expires.toUTCString():"";try{var g=JSON.stringify(c);/^[\{\[]/.test(g)&&(c=g)}catch(j){}c=d.write?d.write(c,b):encodeURIComponent(c+"").replace(/%(23|24|26|2B|3A|3C|3E|3D|2F|3F|40|5B|5D|5E|60|7B|7D|7C)/g,decodeURIComponent),b=encodeURIComponent(b+"").replace(/%(23|24|26|2B|5E|60|7C)/g,decodeURIComponent).replace(/[\(\)]/g,escape);var h="";for(var i in f)f[i]&&(h+="; "+i,!0!==f[i]&&(h+="="+f[i].split(";")[0]));return document.cookie=b+"="+c+h}}function g(a,c){if("undefined"!=typeof document){for(var e={},f=document.cookie?document.cookie.split("; "):[],g=0;g<f.length;g++){var h=f[g].split("="),i=h.slice(1).join("=");c||'"'!==i.charAt(0)||(i=i.slice(1,-1));try{var j=b(h[0]);if(i=(d.read||d)(i,j)||b(i),c)try{i=JSON.parse(i)}catch(k){}if(e[j]=i,a===j)break}catch(k){}}return a?e[a]:e}}return e.set=f,e.get=function(a){return g(a,!1)},e.getJSON=function(a){return g(a,!0)},e.remove=function(b,c){f(b,"",a(c,{expires:-1}))},e.defaults={},e.withConverter=c,e}return c(function(){})});
> - \ No newline at end of file
> ++/*! js-cookie v3.0.0 | MIT */
> ++!function(e,t){"object"==typeof exports&&"undefined"!=typeof module?module.exports=t():"function"==typeof define&&define.amd?define(t):(e=e||self,function(){var n=e.Cookies,r=e.Cookies=t();r.noConflict=function(){return e.Cookies=n,r}}())}(this,(function(){"use strict";function e(e){for(var t=1;t<arguments.length;t++){var n=arguments[t];for(var r in n)e[r]=n[r]}return e}var t={read:function(e){return e.replace(/(%[\dA-F]{2})+/gi,decodeURIComponent)},write:function(e){return encodeURIComponent(e).replace(/%(2[346BF]|3[AC-F]|40|5[BDE]|60|7[BCD])/g,decodeURIComponent)}};return function n(r,o){function i(t,n,i){if("undefined"!=typeof document){"number"==typeof(i=e({},o,i)).expires&&(i.expires=new Date(Date.now()+864e5*i.expires)),i.expires&&(i.expires=i.expires.toUTCString()),t=encodeURIComponent(t).replace(/%(2[346B]|5E|60|7C)/g,decodeURIComponent).replace(/[()]/g,escape),n=r.write(n,t);var c="";for(var u in i)i[u]&&(c+="; "+u,!0!==i[u]&&(c+="="+i[u].split(";")[0]));return document.cookie=t+"="+n+c}}return Object.create({set:i,get:function(e){if("undefined"!=typeof document&&(!arguments.length||e)){for(var n=document.cookie?document.cookie.split("; "):[],o={},i=0;i<n.length;i++){var c=n[i].split("="),u=c.slice(1).join("=");'"'===u[0]&&(u=u.slice(1,-1));try{var f=t.read(c[0]);if(o[f]=r.read(u,f),e===f)break}catch(e){}}return e?o[e]:o}},remove:function(t,n){i(t,"",e({},n,{expires:-1}))},withAttributes:function(t){return n(this.converter,e({},this.attributes,t))},withConverter:function(t){return n(e({},this.converter,t),this.attributes)}},{attributes:{value:Object.freeze(o)},converter:{value:Object.freeze(r)}})}(t,{path:"/"})}));
>
> ## templates/base.html ##
> @@
> 2: b30ba35 ! 2: 2ac10c8 patch-list: clean up patch-list page and refactor patch forms
> @@ Commit message
> patch-list: clean up patch-list page and refactor patch forms
>
> Clean up the patch-list page by moving forms to a new template file
> - patch-forms.html and moving them to the top of the page, adding ids to
> - table cells, and renaming selectors using hyphen delimited strings where
> - the relevant changes were made. Also, created a partial template for
> + 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 makes the code healthier,
> + discoverability of the patch-list forms and make the code healthier,
> ready for change, and overall more readable.
>
> Also, move patch-list related js code to a new patch-list.js file, to
> @@ Commit message
> simplifies a possible future migration to TypeScript if the project
> chooses to go in that direction.
>
> - 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. Error messages and success/warning
> - messages are now normalized throughout these files for testing and
> - functionality. Also, important to these change is that CreateBundleForm
> - in forms.py handles the validation of the bundle form input so that now
> - the patch-list bundle form also makes use of the Django forms API.
> + 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:
> +
> + - Previously patch forms changes were separated between the patch-detail
> + and patch-list pages. Thus, desired changes to the patch forms
> + required changes to patch-list.html, submission.html, and forms.py.
> + So, the most important benefit to this change is that forms.py and
> + patch-forms.html become the two places to adjust the forms to handle
> + form validation and functionality as well as UI changes.
> +
> + - 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
> + 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: 2.2.1
> + :Version: 3.0.0
>
> +``patch-list.js.``
> +
> -+ Utility functions for patch list manipulation (inline dropdown changes,
> -+ etc.)
> ++ Event helpers and other application logic for patch-list.html. These
> ++ support patch list manipulation (e.g. inline dropdown changes).
> +
> + Part of Patchwork.
> +
> @@ htdocs/js/patch-list.js (new)
> + e.preventDefault();
> + });
> +});
> - \ No newline at end of file
>
> ## patchwork/forms.py ##
> @@
> @@ patchwork/forms.py: class EmailForm(forms.Form):
>
>
> class BundleForm(forms.ModelForm):
> -+ # Map 'name' field to 'bundle_name' attr
> + 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'})
>
> -+ # Maps form fields 'name' attr rendered in HTML element
> ++ # 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)
> @@ patchwork/templates/patchwork/partials/patch-list.html
> {% if order.editable %}
> <table class="patchlist">
> @@
> -
> - {% 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>
> + </table>
> {% endif %}
>
> +-{% if page.paginator.long_page and user.is_authenticated %}
> +-<div class="floaty">
> +- <a title="jump to form" href="#patchforms">
> +- <span style="font-size: 120%">▾</span>
> +- </a>
> +-</div>
> +-{% endif %}
> +-
> -<script type="text/javascript">
> -$(document).ready(function() {
> - $('#patchlist').stickyTableHeaders();
> @@ patchwork/templates/patchwork/partials/patch-list.html: $(document).ready(functi
> + <tr id="patch_row:{{patch.id}}" data-patch-id="{{patch.id}}">
> {% if user.is_authenticated %}
> - <td style="text-align: center;">
> -+ <td id="select-patch" 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(functi
> </td>
> {% endif %}
> - <td>
> -+ <td id="patch-name">
> ++ <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">
> ++ <td id="patch-series:{{patch.id}}">
> {% if patch.series %}
> <a href="?series={{patch.series.id}}">
> {{ patch.series|truncatechars:100 }}
> @@ patchwork/templates/patchwork/partials/patch-list.html: $(document).ready(functi
> - <td>{{ patch.submitter|personify:project }}</td>
> - <td>{{ patch.delegate.username }}</td>
> - <td>{{ patch.state }}</td>
> -+ <td id="patch-tags" class="text-nowrap">{{ patch|patch_tags }}</td>
> -+ <td id="patch-checks" class="text-nowrap">{{ patch|patch_checks }}</td>
> -+ <td id="patch-date" class="text-nowrap">{{ patch.date|date:"Y-m-d" }}</td>
> -+ <td id="patch-submitter">{{ patch.submitter|personify:project }}</td>
> -+ <td id="patch-delegate">{{ patch.delegate.username }}</td>
> -+ <td id="patch-state">{{ 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/tests/views/test_patch.py: class PatchUpdateTest(TestCase):
>
> ## patchwork/views/__init__.py ##
> @@
> - # Copyright (C) 2008 Jeremy Kerr <jk@ozlabs.org>
> #
> # SPDX-License-Identifier: GPL-2.0-or-later
> -+import json
>
> ++import json
> ++
> from django.contrib import messages
> from django.shortcuts import get_object_or_404
> -@@ patchwork/views/__init__.py: from django.db.models import Prefetch
> + from django.db.models import Prefetch
>
> from patchwork.filters import Filters
> - from patchwork.forms import MultiplePatchForm
> +from patchwork.forms import CreateBundleForm
> + from patchwork.forms import MultiplePatchForm
> from patchwork.models import Bundle
> from patchwork.models import BundlePatch
> - from patchwork.models import Patch
> -@@ patchwork/views/__init__.py: from patchwork.models import Project
> - from patchwork.models import Check
> - from patchwork.paginator import Paginator
> -
> --
> - bundle_actions = ['create', 'add', 'remove']
> -
> -
> @@ patchwork/views/__init__.py: class Order(object):
>
>
> @@ patchwork/views/__init__.py: class Order(object):
> + messages.success(request, 'Bundle %s created' % bundle.name)
> + else:
> + formErrors = json.loads(create_bundle_form.errors.as_json())
> -+ errors = [formErrors['name'][0]['message']]
> ++ errors = [e['message'] for e in formErrors['name']]
> + return errors
> elif action == 'add':
> + if not data['bundle_id']:
> @@ patchwork/views/patch.py: def patch_detail(request, project_id, msgid):
> elif not editable:
> return HttpResponseForbidden()
>
> +- elif action is None:
> ++ elif action == 'update':
> + form = PatchForm(data=request.POST, instance=patch)
> + if form.is_valid():
> + form.save()
> @@ patchwork/views/patch.py: def patch_detail(request, project_id, msgid):
> context['project'] = patch.project
> context['related_same_project'] = related_same_project
> 3: 3571476 ! 3: 5d5eedc patch-list: style modification forms as an action bar
> @@ patchwork/forms.py: class BundleForm(forms.ModelForm):
> + attrs={'class': 'create-bundle',
> + 'placeholder': 'Bundle name'}))
>
> - # Maps form fields 'name' attr rendered in HTML element
> - def add_prefix(self, field_name):
> + # Changes the HTML 'name' attr of the input element from "name"
> + # (inherited from the model field 'name' of the Bundle object)
> @@ 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: c3f2a17 ! 4: d77051d static: add rest.js to handle requests & respective messages
> @@ htdocs/js/rest.js (new)
> + body: JSON.stringify(data),
> + });
> +
> -+ await fetch(request)
> ++ return await fetch(request)
> + .then(response => {
> + if (!response.ok) {
> + response.text().then(text => {
> @@ htdocs/js/rest.js (new)
> + // Update message for successful changes
> + handleUpdateMessages(updateMessage.some);
> + }
> ++ return response.ok
> + });
> +}
> +
> 5: 62e3434 ! 5: e4e0674 patch-list: add inline dropdown for delegate and state one-off changes
> @@ Commit message
> patch-list: add inline dropdown for delegate and state one-off changes
>
> Add dropdown for the cell values of the Delegate and State columns for
> - each individual patch to make one-off changes to patches. 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.
> + 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.
> + 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.
>
> ## htdocs/js/patch-list.js ##
> @@
> +import { updateProperty } from "./rest.js";
> +
> $( document ).ready(function() {
> ++ let inlinePropertyDropdowns = $("td > select[class^='change-property-']");
> ++ $(inlinePropertyDropdowns).each(function() {
> ++ // Store previous dropdown selection
> ++ $(this).data("prevProperty", $(this).val());
> ++ });
> ++
> + // Change listener for dropdowns that change an individual patch's delegate and state properties
> -+ $("select[class^='change-property-']").change((event) => {
> ++ $(inlinePropertyDropdowns).change((event) => {
> + const property = event.target.getAttribute("value");
> + const { url, data } = getPatchProperties(event.target, property);
> + const updateMessage = {
> + 'none': "No patches updated",
> + 'some': "1 patch updated",
> + };
> -+ updateProperty(url, data, updateMessage);
> ++ updateProperty(url, data, updateMessage).then(is_success => {
> ++ if (!is_success) {
> ++ // Revert to previous selection
> ++ $(event.target).val($(event.target).data("prevProperty"));
> ++ } else {
> ++ // Update to new previous selection
> ++ $(event.target).data("prevProperty", $(event.target).val());
> ++ }
> ++ });
> + });
> +
> $("#patchlist").stickyTableHeaders();
> @@ htdocs/js/patch-list.js: $( document ).ready(function() {
> + };
> + }
> });
> - \ No newline at end of file
>
> ## htdocs/js/rest.js ##
> @@ htdocs/js/rest.js: function handleErrorMessages(errorMessage) {
> @@ patchwork/templates/patchwork/partials/patch-list.html
>
> {% include "patchwork/partials/filters.html" %}
> @@
> - <td id="patch-checks" class="text-nowrap">{{ patch|patch_checks }}</td>
> - <td id="patch-date" class="text-nowrap">{{ patch.date|date:"Y-m-d" }}</td>
> - <td id="patch-submitter">{{ patch.submitter|personify:project }}</td>
> -- <td id="patch-delegate">{{ patch.delegate.username }}</td>
> -- <td id="patch-state">{{ patch.state }}</td>
> -+ <td id="patch-delegate">
> -+ <select class="change-property-delegate" value="delegate">
> -+ {% if not patch.delegate.username %}
> -+ <option value="*" selected>No delegate</option>
> -+ {% else %}
> -+ <option value="*">No delegate</option>
> -+ {% endif %}
> -+ {% for maintainer in maintainers %}
> -+ {% if maintainer.name == patch.delegate.username %}
> -+ <option value="{{ patch.delegate.username.id }}" selected>{{ patch.delegate.username }}</option>
> + <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>
> ++ <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>
> + {% else %}
> -+ <option value="{{ maintainer.id }}">{{ maintainer.name }}</option>
> ++ <option value="*">No delegate</option>
> + {% endif %}
> -+ {% endfor %}
> -+ </select>
> -+ </td>
> -+ <td id="patch-state">
> -+ <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>
> -+ </td>
> ++ {% 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>
> </tr>
> {% empty %}
> <tr>
> --
> 2.32.0.554.ge1b32706d8-goog
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
ccing list, sorry for the noise again Daniel! :( -------- Hi Daniel, > A few bugs: > > - if you submit a change with the dropdowns that errors out, and then > subsequently submit one that succeeds, the error messages are not > cleared. Thanks for mentioning this behavior! The behavior for error and update messages after fetch requests in JS is not polished because of what I wrote in my v1 cover letter [1 - paragraph 2]: "For example, the current handling of messages stacks the messages up as a list of messages. Alternative behaviors like adding timeouts to requests to remove each update message, simply replacing the existing messages & errors, or adding to the counter of (e.g. "x+1 patches updated") can be made." Given your response, for this case in particular it makes sense to clear out error messages after any successful action. I will make those changes in the upcoming v4. I would appreciate feedback on the general behavior for update/errors messages though. As mentioned before, should messages stack up, timeout, or be replaced by each other? Or should they be accumulated as a counter (e.g. "x" => "x+1 patches updated")? Personally, I think that any of the first behaviors get the job done. However, the latter (x+1) and timeout of messages add more complexity which is something to take into account. [1] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006953.html > - You've somehow ended up with two td.patch-delegate: > > <td id="patch-delegate:20"> > <td id="patch-delegate:20"> > > > > </td> > <td id="patch-state:20"> > > New > > </td> > </tr> > > This is throwing off column alignment: delegates now line up under the > State heading. Sorry about that! I realized with your message that I somehow lost the correct changes because I had the correct templating with only one td.patch-delegate in my `git stash`. > > For the first patch, the recently released v3.0.0 of the JS cookie > > library replaces the previous version. > > - the js.cookie.min.js file is still corrupted and I redownloaded it > from a CDN. I'm not sure where the bug lies here and it's probably > not something you need to fix, but a link to the source in the commit > message would be nice. I replied on list [2] for the link on v2, but I will also add the link to download the file to the commit message. [2] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006992.html > > For the fifth patch, the inline dropdowns are changed so that they are > > only visible to logged in users. Also, upon any unsuccessful update > > requests, the dropdown selection reverts to its previous selection. > > Since update requests to a patch’s state and delegate fields fail for > > unauthorized users, this behavior covers the case where unauthorized > > users don't see the current state of the db after they change the > > dropdown selection. This behavior is a useful example of patch four’s > > change of returning whether an update request is successful. > > - they're visible to logged in users but still suggest that if I am a > regular user that I have the ability to change anyone's patch > state/delegate. I am still a bit dubious overall about inline > dropdowns in general, although I am cognisant that it's much more > discoverable this way. Either way, IMO dropdowns need to be > restricted to editable patches. I did make the implementation where only users with permissions can see the dropdown, however the loading of the page is significantly slower. So, I feel that the safety measures added for logged in users would be best. The message could be more explicit and say the exact permissions required, which would prevent less mistakes in the future as users are more informed. Nonetheless, I'm interested in knowing how you feel about this considering the slower page loads. My earlier reply (that I forgot to cc) in the v2 patch [3]. [3] https://lists.ozlabs.org/pipermail/patchwork/2021-August/007020.html
On Wed, 2021-07-28 at 15:24 +0000, Raxel Gutierrez wrote: > This series is a revision to the previous version of the patch series. > The series mainly addresses the review comments from the v2 series [1]. > > For the first patch, the recently released v3.0.0 of the JS cookie > library replaces the previous version. > > For the second patch, following jrnieder’s comments [2], the commit > message is updated to better explain the benefits of the change and its > details. Also, an unclear comment in forms.py for BundleForm is cleaned > up to better explain what the changes do. The id for <td> table cells in > patch-list.html are correctly differentiated by patch id. I noticed a > bug where property changes in the patch detail page weren’t working > because the respective “update” action was not accounted for. > > For the third patch, the “jump to form” arrow is removed [3]. > > For the fourth patch, the `updateProperty` function now returns whether > the update request was successful or not with `response.ok` so that > callers can use that information and respond accordingly. The next patch > adds a use case for the return value. > > For the fifth patch, the inline dropdowns are changed so that they are > only visible to logged in users. Also, upon any unsuccessful update > requests, the dropdown selection reverts to its previous selection. > Since update requests to a patch’s state and delegate fields fail for > unauthorized users, this behavior covers the case where unauthorized > users don't see the current state of the db after they change the > dropdown selection. This behavior is a useful example of patch four’s > change of returning whether an update request is successful. > > [1] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006968.html > [2] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006988.html > [3] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006991.html fwiw, I've read the reviews from Daniel on this series and am waiting on v4 before I weigh in. Stephen
This series is a revision to the previous version of the patch series. The series mainly addresses the review comments from the v2 series [1]. For the first patch, the recently released v3.0.0 of the JS cookie library replaces the previous version. For the second patch, following jrnieder’s comments [2], the commit message is updated to better explain the benefits of the change and its details. Also, an unclear comment in forms.py for BundleForm is cleaned up to better explain what the changes do. The id for <td> table cells in patch-list.html are correctly differentiated by patch id. I noticed a bug where property changes in the patch detail page weren’t working because the respective “update” action was not accounted for. For the third patch, the “jump to form” arrow is removed [3]. For the fourth patch, the `updateProperty` function now returns whether the update request was successful or not with `response.ok` so that callers can use that information and respond accordingly. The next patch adds a use case for the return value. For the fifth patch, the inline dropdowns are changed so that they are only visible to logged in users. Also, upon any unsuccessful update requests, the dropdown selection reverts to its previous selection. Since update requests to a patch’s state and delegate fields fail for unauthorized users, this behavior covers the case where unauthorized users don't see the current state of the db after they change the dropdown selection. This behavior is a useful example of patch four’s change of returning whether an update request is successful. [1] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006968.html [2] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006988.html [3] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006991.html Raxel Gutierrez (5): static: add JS Cookie Library to get csrftoken for fetch requests patch-list: clean up patch-list page and refactor patch forms patch-list: style modification forms as an action bar static: add rest.js to handle requests & respective messages patch-list: add inline dropdown for delegate and state one-off changes htdocs/README.rst | 23 +++ htdocs/css/style.css | 85 +++++++-- htdocs/js/js.cookie.min.js | 2 + htdocs/js/patch-list.js | 60 ++++++ htdocs/js/rest.js | 72 ++++++++ patchwork/forms.py | 66 +++++-- patchwork/templates/patchwork/list.html | 11 +- .../templates/patchwork/partials/errors.html | 9 + .../patchwork/partials/patch-forms.html | 45 +++++ .../patchwork/partials/patch-list.html | 171 ++++++------------ patchwork/templates/patchwork/submission.html | 91 +--------- patchwork/tests/views/test_bundles.py | 44 ++--- patchwork/tests/views/test_patch.py | 4 +- patchwork/views/__init__.py | 76 ++++---- patchwork/views/patch.py | 35 +--- templates/base.html | 3 +- 16 files changed, 472 insertions(+), 325 deletions(-) create mode 100644 htdocs/js/js.cookie.min.js create mode 100644 htdocs/js/patch-list.js create mode 100644 htdocs/js/rest.js create mode 100644 patchwork/templates/patchwork/partials/errors.html create mode 100644 patchwork/templates/patchwork/partials/patch-forms.html Range-diff: 1: 2a34394 ! 1: 859789e static: add JS Cookie Library to get csrftoken for fetch requests @@ htdocs/README.rst: js + This is used to get the ``csrftoken`` cookie for AJAX requests in JavaScript. + + :GitHub: https://github.com/js-cookie/js-cookie/ -+ :Version: 2.2.1 ++ :Version: 3.0.0 + ``selectize.min.js`` @@ htdocs/README.rst: js ## htdocs/js/js.cookie.min.js (new) ## @@ -+/*! js-cookie v2.2.1 | MIT */ -+ -+!function(a){var b;if("function"==typeof define&&define.amd&&(define(a),b=!0),"object"==typeof exports&&(module.exports=a(),b=!0),!b){var c=window.Cookies,d=window.Cookies=a();d.noConflict=function(){return window.Cookies=c,d}}}(function(){function a(){for(var a=0,b={};a<arguments.length;a++){var c=arguments[a];for(var d in c)b[d]=c[d]}return b}function b(a){return a.replace(/(%[0-9A-Z]{2})+/g,decodeURIComponent)}function c(d){function e(){}function f(b,c,f){if("undefined"!=typeof document){f=a({path:"/"},e.defaults,f),"number"==typeof f.expires&&(f.expires=new Date(1*new Date+864e5*f.expires)),f.expires=f.expires?f.expires.toUTCString():"";try{var g=JSON.stringify(c);/^[\{\[]/.test(g)&&(c=g)}catch(j){}c=d.write?d.write(c,b):encodeURIComponent(c+"").replace(/%(23|24|26|2B|3A|3C|3E|3D|2F|3F|40|5B|5D|5E|60|7B|7D|7C)/g,decodeURIComponent),b=encodeURIComponent(b+"").replace(/%(23|24|26|2B|5E|60|7C)/g,decodeURIComponent).replace(/[\(\)]/g,escape);var h="";for(var i in f)f[i]&&(h+="; "+i,!0!==f[i]&&(h+="="+f[i].split(";")[0]));return document.cookie=b+"="+c+h}}function g(a,c){if("undefined"!=typeof document){for(var e={},f=document.cookie?document.cookie.split("; "):[],g=0;g<f.length;g++){var h=f[g].split("="),i=h.slice(1).join("=");c||'"'!==i.charAt(0)||(i=i.slice(1,-1));try{var j=b(h[0]);if(i=(d.read||d)(i,j)||b(i),c)try{i=JSON.parse(i)}catch(k){}if(e[j]=i,a===j)break}catch(k){}}return a?e[a]:e}}return e.set=f,e.get=function(a){return g(a,!1)},e.getJSON=function(a){return g(a,!0)},e.remove=function(b,c){f(b,"",a(c,{expires:-1}))},e.defaults={},e.withConverter=c,e}return c(function(){})}); - \ No newline at end of file ++/*! js-cookie v3.0.0 | MIT */ ++!function(e,t){"object"==typeof exports&&"undefined"!=typeof module?module.exports=t():"function"==typeof define&&define.amd?define(t):(e=e||self,function(){var n=e.Cookies,r=e.Cookies=t();r.noConflict=function(){return e.Cookies=n,r}}())}(this,(function(){"use strict";function e(e){for(var t=1;t<arguments.length;t++){var n=arguments[t];for(var r in n)e[r]=n[r]}return e}var t={read:function(e){return e.replace(/(%[\dA-F]{2})+/gi,decodeURIComponent)},write:function(e){return encodeURIComponent(e).replace(/%(2[346BF]|3[AC-F]|40|5[BDE]|60|7[BCD])/g,decodeURIComponent)}};return function n(r,o){function i(t,n,i){if("undefined"!=typeof document){"number"==typeof(i=e({},o,i)).expires&&(i.expires=new Date(Date.now()+864e5*i.expires)),i.expires&&(i.expires=i.expires.toUTCString()),t=encodeURIComponent(t).replace(/%(2[346B]|5E|60|7C)/g,decodeURIComponent).replace(/[()]/g,escape),n=r.write(n,t);var c="";for(var u in i)i[u]&&(c+="; "+u,!0!==i[u]&&(c+="="+i[u].split(";")[0]));return document.cookie=t+"="+n+c}}return Object.create({set:i,get:function(e){if("undefined"!=typeof document&&(!arguments.length||e)){for(var n=document.cookie?document.cookie.split("; "):[],o={},i=0;i<n.length;i++){var c=n[i].split("="),u=c.slice(1).join("=");'"'===u[0]&&(u=u.slice(1,-1));try{var f=t.read(c[0]);if(o[f]=r.read(u,f),e===f)break}catch(e){}}return e?o[e]:o}},remove:function(t,n){i(t,"",e({},n,{expires:-1}))},withAttributes:function(t){return n(this.converter,e({},this.attributes,t))},withConverter:function(t){return n(e({},this.converter,t),this.attributes)}},{attributes:{value:Object.freeze(o)},converter:{value:Object.freeze(r)}})}(t,{path:"/"})})); ## templates/base.html ## @@ 2: b30ba35 ! 2: 2ac10c8 patch-list: clean up patch-list page and refactor patch forms @@ Commit message patch-list: clean up patch-list page and refactor patch forms Clean up the patch-list page by moving forms to a new template file - patch-forms.html and moving them to the top of the page, adding ids to - table cells, and renaming selectors using hyphen delimited strings where - the relevant changes were made. Also, created a partial template for + 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 makes the code healthier, + discoverability of the patch-list forms and make the code healthier, ready for change, and overall more readable. Also, move patch-list related js code to a new patch-list.js file, to @@ Commit message simplifies a possible future migration to TypeScript if the project chooses to go in that direction. - 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. Error messages and success/warning - messages are now normalized throughout these files for testing and - functionality. Also, important to these change is that CreateBundleForm - in forms.py handles the validation of the bundle form input so that now - the patch-list bundle form also makes use of the Django forms API. + 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: + + - Previously patch forms changes were separated between the patch-detail + and patch-list pages. Thus, desired changes to the patch forms + required changes to patch-list.html, submission.html, and forms.py. + So, the most important benefit to this change is that forms.py and + patch-forms.html become the two places to adjust the forms to handle + form validation and functionality as well as UI changes. + + - 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 + 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: 2.2.1 + :Version: 3.0.0 +``patch-list.js.`` + -+ Utility functions for patch list manipulation (inline dropdown changes, -+ etc.) ++ Event helpers and other application logic for patch-list.html. These ++ support patch list manipulation (e.g. inline dropdown changes). + + Part of Patchwork. + @@ htdocs/js/patch-list.js (new) + e.preventDefault(); + }); +}); - \ No newline at end of file ## patchwork/forms.py ## @@ @@ patchwork/forms.py: class EmailForm(forms.Form): class BundleForm(forms.ModelForm): -+ # Map 'name' field to 'bundle_name' attr + 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'}) -+ # Maps form fields 'name' attr rendered in HTML element ++ # 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) @@ patchwork/templates/patchwork/partials/patch-list.html {% if order.editable %} <table class="patchlist"> @@ - - {% 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> + </table> {% endif %} +-{% if page.paginator.long_page and user.is_authenticated %} +-<div class="floaty"> +- <a title="jump to form" href="#patchforms"> +- <span style="font-size: 120%">▾</span> +- </a> +-</div> +-{% endif %} +- -<script type="text/javascript"> -$(document).ready(function() { - $('#patchlist').stickyTableHeaders(); @@ patchwork/templates/patchwork/partials/patch-list.html: $(document).ready(functi + <tr id="patch_row:{{patch.id}}" data-patch-id="{{patch.id}}"> {% if user.is_authenticated %} - <td style="text-align: center;"> -+ <td id="select-patch" 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(functi </td> {% endif %} - <td> -+ <td id="patch-name"> ++ <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"> ++ <td id="patch-series:{{patch.id}}"> {% if patch.series %} <a href="?series={{patch.series.id}}"> {{ patch.series|truncatechars:100 }} @@ patchwork/templates/patchwork/partials/patch-list.html: $(document).ready(functi - <td>{{ patch.submitter|personify:project }}</td> - <td>{{ patch.delegate.username }}</td> - <td>{{ patch.state }}</td> -+ <td id="patch-tags" class="text-nowrap">{{ patch|patch_tags }}</td> -+ <td id="patch-checks" class="text-nowrap">{{ patch|patch_checks }}</td> -+ <td id="patch-date" class="text-nowrap">{{ patch.date|date:"Y-m-d" }}</td> -+ <td id="patch-submitter">{{ patch.submitter|personify:project }}</td> -+ <td id="patch-delegate">{{ patch.delegate.username }}</td> -+ <td id="patch-state">{{ 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/tests/views/test_patch.py: class PatchUpdateTest(TestCase): ## patchwork/views/__init__.py ## @@ - # Copyright (C) 2008 Jeremy Kerr <jk@ozlabs.org> # # SPDX-License-Identifier: GPL-2.0-or-later -+import json ++import json ++ from django.contrib import messages from django.shortcuts import get_object_or_404 -@@ patchwork/views/__init__.py: from django.db.models import Prefetch + from django.db.models import Prefetch from patchwork.filters import Filters - from patchwork.forms import MultiplePatchForm +from patchwork.forms import CreateBundleForm + from patchwork.forms import MultiplePatchForm from patchwork.models import Bundle from patchwork.models import BundlePatch - from patchwork.models import Patch -@@ patchwork/views/__init__.py: from patchwork.models import Project - from patchwork.models import Check - from patchwork.paginator import Paginator - -- - bundle_actions = ['create', 'add', 'remove'] - - @@ patchwork/views/__init__.py: class Order(object): @@ patchwork/views/__init__.py: class Order(object): + messages.success(request, 'Bundle %s created' % bundle.name) + else: + formErrors = json.loads(create_bundle_form.errors.as_json()) -+ errors = [formErrors['name'][0]['message']] ++ errors = [e['message'] for e in formErrors['name']] + return errors elif action == 'add': + if not data['bundle_id']: @@ patchwork/views/patch.py: def patch_detail(request, project_id, msgid): elif not editable: return HttpResponseForbidden() +- elif action is None: ++ elif action == 'update': + form = PatchForm(data=request.POST, instance=patch) + if form.is_valid(): + form.save() @@ patchwork/views/patch.py: def patch_detail(request, project_id, msgid): context['project'] = patch.project context['related_same_project'] = related_same_project 3: 3571476 ! 3: 5d5eedc patch-list: style modification forms as an action bar @@ patchwork/forms.py: class BundleForm(forms.ModelForm): + attrs={'class': 'create-bundle', + 'placeholder': 'Bundle name'})) - # Maps form fields 'name' attr rendered in HTML element - def add_prefix(self, field_name): + # Changes the HTML 'name' attr of the input element from "name" + # (inherited from the model field 'name' of the Bundle object) @@ 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: c3f2a17 ! 4: d77051d static: add rest.js to handle requests & respective messages @@ htdocs/js/rest.js (new) + body: JSON.stringify(data), + }); + -+ await fetch(request) ++ return await fetch(request) + .then(response => { + if (!response.ok) { + response.text().then(text => { @@ htdocs/js/rest.js (new) + // Update message for successful changes + handleUpdateMessages(updateMessage.some); + } ++ return response.ok + }); +} + 5: 62e3434 ! 5: e4e0674 patch-list: add inline dropdown for delegate and state one-off changes @@ Commit message patch-list: add inline dropdown for delegate and state one-off changes Add dropdown for the cell values of the Delegate and State columns for - each individual patch to make one-off changes to patches. 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. + 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. + 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. ## htdocs/js/patch-list.js ## @@ +import { updateProperty } from "./rest.js"; + $( document ).ready(function() { ++ let inlinePropertyDropdowns = $("td > select[class^='change-property-']"); ++ $(inlinePropertyDropdowns).each(function() { ++ // Store previous dropdown selection ++ $(this).data("prevProperty", $(this).val()); ++ }); ++ + // Change listener for dropdowns that change an individual patch's delegate and state properties -+ $("select[class^='change-property-']").change((event) => { ++ $(inlinePropertyDropdowns).change((event) => { + const property = event.target.getAttribute("value"); + const { url, data } = getPatchProperties(event.target, property); + const updateMessage = { + 'none': "No patches updated", + 'some': "1 patch updated", + }; -+ updateProperty(url, data, updateMessage); ++ updateProperty(url, data, updateMessage).then(is_success => { ++ if (!is_success) { ++ // Revert to previous selection ++ $(event.target).val($(event.target).data("prevProperty")); ++ } else { ++ // Update to new previous selection ++ $(event.target).data("prevProperty", $(event.target).val()); ++ } ++ }); + }); + $("#patchlist").stickyTableHeaders(); @@ htdocs/js/patch-list.js: $( document ).ready(function() { + }; + } }); - \ No newline at end of file ## htdocs/js/rest.js ## @@ htdocs/js/rest.js: function handleErrorMessages(errorMessage) { @@ patchwork/templates/patchwork/partials/patch-list.html {% include "patchwork/partials/filters.html" %} @@ - <td id="patch-checks" class="text-nowrap">{{ patch|patch_checks }}</td> - <td id="patch-date" class="text-nowrap">{{ patch.date|date:"Y-m-d" }}</td> - <td id="patch-submitter">{{ patch.submitter|personify:project }}</td> -- <td id="patch-delegate">{{ patch.delegate.username }}</td> -- <td id="patch-state">{{ patch.state }}</td> -+ <td id="patch-delegate"> -+ <select class="change-property-delegate" value="delegate"> -+ {% if not patch.delegate.username %} -+ <option value="*" selected>No delegate</option> -+ {% else %} -+ <option value="*">No delegate</option> -+ {% endif %} -+ {% for maintainer in maintainers %} -+ {% if maintainer.name == patch.delegate.username %} -+ <option value="{{ patch.delegate.username.id }}" selected>{{ patch.delegate.username }}</option> + <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> ++ <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> + {% else %} -+ <option value="{{ maintainer.id }}">{{ maintainer.name }}</option> ++ <option value="*">No delegate</option> + {% endif %} -+ {% endfor %} -+ </select> -+ </td> -+ <td id="patch-state"> -+ <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> -+ </td> ++ {% 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> </tr> {% empty %} <tr>