[PATCH v2 0/5] patch-list: improve usability of list action bar
Raxel Gutierrez
raxel at google.com
Fri Jul 23 03:12:46 AEST 2021
This series is a revision to the previous version of the patch series.
Following comments, I changed the commit messages to use the
conventional imperative mood and added more details to better express
the reasons behind the changes.
For the first patch, I removed the version number to make the use of the
JS cookie library cleaner. There is still debate around where and how
the file should be stored [1].
For the second patch, I did a lot more cleanup of the patch-list page.
The error messages from Django form validation is now it’s own partial
template so that it can be easily reused in various templates. The most
important and major changes are that the patch forms also become a
partial template so that it can be used for a list of patches in the
patch-list page and for individual patches in the patch-details page.
The patch forms in each respective page had their own quirks, so some
refactoring had to be done on related files to preserve the
functionality in both use cases. The effort was worth it as now the
patch forms tool is more ready for change.
For the third patch, the commit message features before and after images
for better reference of the style changes to an action bar UI. As noted
by jrnieder’s comments, I fixed the potential risk of unadapted callers
by using keyword-only arguments as better practice. Also, the
responsibility of setting the CSS class selector for the patch form’s
fields is moved from the form to the fields’ constructors which adds
more flexibility for naming within each field.
For the fourth patch, the utils.js file is renamed to rest.js to
initiate the convention of different groupings of utility files and
functions.
For the fifth patch, the commit message changes to better explain the
changes and accounts for the change to the rest.js file it uses.
[1] https://lists.ozlabs.org/pipermail/patchwork/2021-July/006966.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 | 3 +
htdocs/js/patch-list.js | 46 ++++++
htdocs/js/rest.js | 71 ++++++++
patchwork/forms.py | 64 +++++--
patchwork/templates/patchwork/list.html | 11 +-
.../templates/patchwork/partials/errors.html | 9 +
.../patchwork/partials/patch-forms.html | 45 +++++
.../patchwork/partials/patch-list.html | 156 ++++++------------
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 | 33 +---
templates/base.html | 3 +-
16 files changed, 446 insertions(+), 318 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
Interdiff:
diff --git a/htdocs/README.rst b/htdocs/README.rst
index 2bae34c..12174de 100644
--- a/htdocs/README.rst
+++ b/htdocs/README.rst
@@ -126,7 +126,7 @@ js
Library used to handle cookies.
- This is used to get the ``csrftoken`` cookie for AJAX POST requests.
+ 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
@@ -138,6 +138,13 @@ js
Part of Patchwork.
+``rest.js.``
+
+ Utility module for REST API requests to be used by other Patchwork js files
+ (fetch requests, handling update & error messages).
+
+ Part of Patchwork.
+
``selectize.min.js``
Selectize is the hybrid of a ``textbox`` and ``<select>`` box. It's jQuery
@@ -147,10 +154,3 @@ js
:Website: https://selectize.github.io/selectize.js/
:GitHub: https://github.com/selectize/selectize.js
:Version: 0.11.2
-
-``utils.js.``
-
- General utility module for functions used throughout other static Patchwork
- js files (fetch requests, handling update & error messages).
-
- Part of Patchwork.
diff --git a/htdocs/css/style.css b/htdocs/css/style.css
index 2a45ec7..9982f92 100644
--- a/htdocs/css/style.css
+++ b/htdocs/css/style.css
@@ -204,7 +204,7 @@ 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;
@@ -354,19 +354,19 @@ table.bundlelist td
}
/* forms that appear for a patch */
-.patchforms {
+.patch-forms {
display: inline-flex;
flex-wrap: wrap;
margin: 16px 0px;
}
-div.patchform {
+div.patch-form {
display: flex;
flex-wrap: wrap;
align-items: center;
}
-.change-property, select[class^="change-property-"], .archive-patch, .add-bundle {
+select[class^=change-property-], .archive-patch-select, .add-bundle {
padding: 4px;
margin-right: 8px;
box-sizing: border-box;
@@ -374,12 +374,26 @@ div.patchform {
background-color: var(--light-color);
}
-.patchform-submit {
+#patch-form-archive {
+ display: flex;
+ align-items: center;
+ margin-right: 4px;
+}
+
+#patch-form-archive > label {
+ margin: 0px;
+}
+
+#patch-form-archive > select, #patch-form-archive > input {
+ margin: 0px 4px 0px 4px;
+}
+
+.patch-form-submit {
font-weight: bold;
padding: 4px;
}
-#patchform-bundle, #add-to-bundle, #remove-bundle {
+#patch-form-bundle, #add-to-bundle, #remove-bundle {
margin-left: 16px;
}
@@ -390,6 +404,29 @@ div.patchform {
border-radius: 4px;
}
+div.patch-form h3 {
+ margin-top: 0em;
+ margin-left: -0.6em;
+ margin-right: -0.6em;
+ padding: 0.3em 0.3em 0.3em 0.6em;
+ background-color: #222;
+ color: #999;
+ font-size: 100%;
+}
+
+div.patch-form ul {
+ list-style-type: none;
+ padding-left: 0.2em;
+ margin-top: 0em;
+}
+
+.create-bundle {
+ padding: 4px;
+ margin-right: 8px;
+ box-sizing: border-box;
+ border-radius: 4px;
+}
+
span.help_text {
font-size: 80%;
}
diff --git a/htdocs/js/js.cookie-2.2.1.min.js b/htdocs/js/js.cookie.min.js
similarity index 60%
rename from htdocs/js/js.cookie-2.2.1.min.js
rename to htdocs/js/js.cookie.min.js
index 72f312a..f5f4c36 100644
--- a/htdocs/js/js.cookie-2.2.1.min.js
+++ b/htdocs/js/js.cookie.min.js
@@ -1,3 +1,3 @@
/*! 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,
+!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
diff --git a/htdocs/js/patch-list.js b/htdocs/js/patch-list.js
index b88bad5..75d9b38 100644
--- a/htdocs/js/patch-list.js
+++ b/htdocs/js/patch-list.js
@@ -1,4 +1,4 @@
-import { updateProperty } from "./utils.js";
+import { updateProperty } from "./rest.js";
$( document ).ready(function() {
// Change listener for dropdowns that change an individual patch's delegate and state properties
diff --git a/htdocs/js/utils.js b/htdocs/js/rest.js
similarity index 100%
rename from htdocs/js/utils.js
rename to htdocs/js/rest.js
diff --git a/patchwork/forms.py b/patchwork/forms.py
index b684026..9727932 100644
--- a/patchwork/forms.py
+++ b/patchwork/forms.py
@@ -2,10 +2,10 @@
# 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
+from django.core.exceptions import ValidationError
from django.db.utils import ProgrammingError
from patchwork.models import Bundle
@@ -50,9 +50,19 @@ 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',
- error_messages={'invalid': 'Bundle names can\'t contain slashes'})
+ regex=r'^[^/]+$', min_length=1, max_length=50, required=False,
+ error_messages={'invalid': 'Bundle names can\'t contain slashes'},
+ widget=forms.TextInput(
+ attrs={'class': 'create-bundle',
+ 'placeholder': 'Bundle name'}))
+
+ # Maps form fields 'name' attr rendered in HTML element
+ 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
@@ -70,11 +80,16 @@ class CreateBundleForm(BundleForm):
def clean_name(self):
name = self.cleaned_data['name']
+ if not name:
+ raise ValidationError('No bundle name was specified',
+ code="invalid")
+
count = Bundle.objects.filter(owner=self.instance.owner,
name=name).count()
if count > 0:
- raise forms.ValidationError('A bundle called %s already exists'
- % name)
+ raise ValidationError('A bundle called %(name)s already exists',
+ code="invalid",
+ params={'name': name})
return name
@@ -114,19 +129,28 @@ class PatchForm(forms.ModelForm):
def __init__(self, instance=None, project=None, *args, **kwargs):
super(PatchForm, self).__init__(instance=instance, *args, **kwargs)
self.fields['delegate'] = forms.ModelChoiceField(
- queryset=_get_delegate_qs(project, instance), required=False)
+ queryset=_get_delegate_qs(project, instance),
+ widget=forms.Select(attrs={'class': 'change-property-delegate'}),
+ required=False)
class Meta:
model = Patch
fields = ['state', 'archived', 'delegate']
+ widgets = {
+ 'state': forms.Select(
+ attrs={'class': 'change-property-state'}),
+ 'archived': forms.CheckboxInput(
+ attrs={'class': 'archive-patch-check'}),
+ }
class OptionalModelChoiceField(forms.ModelChoiceField):
no_change_choice = ('*', 'No change')
to_field_name = None
- def __init__(self, placeholder, *args, **kwargs):
+ def __init__(self, *args, placeholder, className, **kwargs):
self.no_change_choice = ('*', placeholder)
+ self.widget = forms.Select(attrs={'class': className})
super(OptionalModelChoiceField, self).__init__(
initial=self.no_change_choice[0], *args, **kwargs)
@@ -155,6 +179,10 @@ class OptionalModelChoiceField(forms.ModelChoiceField):
class OptionalBooleanField(forms.TypedChoiceField):
+ def __init__(self, className, *args, **kwargs):
+ self.widget = forms.Select(attrs={'class': className})
+ super(OptionalBooleanField, self).__init__(*args, **kwargs)
+
def is_no_change(self, value):
return value == self.empty_value
@@ -162,6 +190,7 @@ class OptionalBooleanField(forms.TypedChoiceField):
class MultiplePatchForm(forms.Form):
action = 'update'
archived = OptionalBooleanField(
+ className="archive-patch-select",
choices=[('*', 'No change'), ('True', 'Archive'),
('False', 'Unarchive')],
coerce=lambda x: x == 'True',
@@ -173,18 +202,14 @@ class MultiplePatchForm(forms.Form):
self.fields['delegate'] = OptionalModelChoiceField(
queryset=_get_delegate_qs(project=project),
placeholder="Delegate to",
+ className="change-property-delegate",
label="Delegate to",
required=False)
self.fields['state'] = OptionalModelChoiceField(
queryset=State.objects.all(),
placeholder="Change state",
+ className="change-property-state",
label="Change state")
- self.fields['state'].widget.attrs.update(
- {'class': 'change-property'})
- self.fields['delegate'].widget.attrs.update(
- {'class': 'change-property'})
- self.fields['archived'].widget.attrs.update(
- {'class': 'archive-patch'})
def save(self, instance, commit=True):
opts = instance.__class__._meta
diff --git a/patchwork/templates/patchwork/list.html b/patchwork/templates/patchwork/list.html
index 5d3d82a..d4e81bc 100644
--- a/patchwork/templates/patchwork/list.html
+++ b/patchwork/templates/patchwork/list.html
@@ -8,16 +8,7 @@
{% 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 %}
diff --git a/patchwork/templates/patchwork/partials/errors.html b/patchwork/templates/patchwork/partials/errors.html
new file mode 100644
index 0000000..9e15009
--- /dev/null
+++ b/patchwork/templates/patchwork/partials/errors.html
@@ -0,0 +1,9 @@
+{% 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 %}
diff --git a/patchwork/templates/patchwork/partials/patch-forms.html b/patchwork/templates/patchwork/partials/patch-forms.html
new file mode 100644
index 0000000..5a824aa
--- /dev/null
+++ b/patchwork/templates/patchwork/partials/patch-forms.html
@@ -0,0 +1,45 @@
+<div class="patch-forms" id="patch-forms">
+{% if patchform %}
+ <div id="patch-form-properties" class="patch-form">
+ <div id="patch-form-state">
+ {{ patchform.state.errors }}
+ {{ patchform.state }}
+ </div>
+ <div id="patch-form-delegate">
+ {{ patchform.delegate.errors }}
+ {{ patchform.delegate }}
+ </div>
+ <div id="patch-form-archive">
+ {{ patchform.archived.errors }}
+ {{ patchform.archived.label_tag }} {{ patchform.archived }}
+ </div>
+ <button class="patch-form-submit btn btn-primary" name="action" value="update">Update</button>
+ </div>
+{% endif %}
+{% if user.is_authenticated %}
+ <div id="patch-form-bundle" class="patch-form">
+ <div id="create-bundle">
+ {{ createbundleform.name.errors }}
+ {{ createbundleform.name }}
+ <input class="patch-form-submit btn btn-primary" name="action" value="Create" type="submit"/>
+ </div>
+ {% if bundles %}
+ <div id="add-to-bundle">
+ <select class="add-bundle" name="bundle_id">
+ <option value="" selected>Add to bundle</option>
+ {% for bundle in bundles %}
+ <option value="{{bundle.id}}">{{bundle.name}}</option>
+ {% endfor %}
+ </select>
+ <input class="patch-form-submit btn btn-primary" name="action" value="Add" type="submit"/>
+ </div>
+ {% endif %}
+ {% if bundle %}
+ <div id="remove-bundle">
+ <input type="hidden" name="removed_bundle_id" value="{{bundle.id}}"/>
+ <button class="patch-form-submit btn btn-primary" name="action" value="Remove">Remove from bundle</button>
+ </div>
+ {% endif %}
+ </div>
+{% endif %}
+</div>
diff --git a/patchwork/templates/patchwork/partials/patch-list.html b/patchwork/templates/patchwork/partials/patch-list.html
index 48b47db..8967cdd 100644
--- a/patchwork/templates/patchwork/partials/patch-list.html
+++ b/patchwork/templates/patchwork/partials/patch-list.html
@@ -31,7 +31,7 @@
{% 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>
@@ -39,53 +39,10 @@
<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="project" value="{{project.id}}"/>
<div class="patch-list-actions">
- <div class="patchforms" id="patchforms">
- {% if patchform %}
- <div id="patchform-properties" class="patchform">
- <div id="patchform-state">
- {{ patchform.state.errors }}
- {{ patchform.state }}
- </div>
- <div id="patchform-delegate">
- {{ patchform.delegate.errors }}
- {{ patchform.delegate }}
- </div>
- <div id="patchform-archive">
- {{ patchform.archived.errors }}
- {{ patchform.archived.label_tag }} {{ patchform.archived }}
- </div>
- <button class="patchform-submit btn btn-primary" name="action" value="{{patchform.action}}">Update</button>
- </div>
- {% endif %}
- {% if user.is_authenticated %}
- <div id="patchform-bundle" class="patchform">
- <div id="create-bundle">
- <input class="create-bundle" type="text" name="bundle_name" placeholder="Bundle name"/>
- <input class="patchform-submit btn btn-primary" name="action" value="Create" type="submit"/>
- </div>
- {% if bundles %}
- <div id="add-to-bundle">
- <select class="add-bundle" name="bundle_id">
- <option value="" selected>Add to bundle</option>
- {% for bundle in bundles %}
- <option value="{{bundle.id}}">{{bundle.name}}</option>
- {% endfor %}
- </select>
- <input class="patchform-submit btn btn-primary" name="action" value="Add" type="submit"/>
- </div>
- {% endif %}
- {% if bundle %}
- <div id="remove-bundle">
- <input type="hidden" name="removed_bundle_id" value="{{bundle.id}}"/>
- <button class="patchform-submit btn btn-primary" name="action" value="Remove">Remove from bundle</button>
- </div>
- {% endif %}
- </div>
- {% endif %}
- </div>
+ {% 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"
diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
index 978559b..17255ee 100644
--- a/patchwork/templates/patchwork/submission.html
+++ b/patchwork/templates/patchwork/submission.html
@@ -27,6 +27,8 @@ 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>
@@ -149,91 +151,10 @@ function toggle_div(link_id, headers_id, label_show, label_hide)
{% endif %}
</table>
-<div class="patchforms">
-{% if patchform %}
- <div class="patchform patchform-properties">
- <h3>Patch Properties</h3>
- <form method="post">
- {% csrf_token %}
- <table class="form">
- <tr>
- <th>Change state:</th>
- <td>
- {{ patchform.state }}
- {{ patchform.state.errors }}
- </td>
- </tr>
- <tr>
- <th>Delegate to:</th>
- <td>
- {{ patchform.delegate }}
- {{ patchform.delegate.errors }}
- </td>
- </tr>
- <tr>
- <th>Archived:</th>
- <td>
- {{ patchform.archived }}
- {{ patchform.archived.errors }}
- </td>
- </tr>
- <tr>
- <td></td>
- <td>
- <input type="submit" value="Update">
- </td>
- </tr>
- </table>
- </form>
- </div>
-{% endif %}
-
-{% if createbundleform %}
- <div class="patchform patchform-bundle">
- <h3>Bundling</h3>
- <table class="form">
- <tr>
- <td>Create bundle:</td>
- <td>
- {% if createbundleform.non_field_errors %}
- <dd class="errors">{{createbundleform.non_field_errors}}</dd>
- {% endif %}
- <form method="post">
- {% csrf_token %}
- <input type="hidden" name="action" value="createbundle"/>
- {% if createbundleform.name.errors %}
- <dd class="errors">{{createbundleform.name.errors}}</dd>
- {% endif %}
- {{ createbundleform.name }}
- <input value="Create" type="submit"/>
- </form>
- </td>
- </tr>
-{% if bundles %}
- <tr>
- <td>Add to bundle:</td>
- <td>
- <form method="post">
- {% csrf_token %}
- <input type="hidden" name="action" value="addtobundle"/>
- <select name="bundle_id"/>
- {% for bundle in bundles %}
- <option value="{{bundle.id}}">{{bundle.name}}</option>
- {% endfor %}
- </select>
- <input value="Add" type="submit"/>
- </form>
- </td>
- </tr>
-{% endif %}
- </table>
-
- </div>
-{% endif %}
-
- <div style="clear: both;">
- </div>
-</div>
+<form id="patch-list-form" method="POST">
+ {% csrf_token %}
+ {% include "patchwork/partials/patch-forms.html" %}
+</form>
{% if submission.pull_url %}
<h2>Pull-request</h2>
diff --git a/patchwork/tests/views/test_bundles.py b/patchwork/tests/views/test_bundles.py
index 6a74409..2233c21 100644
--- a/patchwork/tests/views/test_bundles.py
+++ b/patchwork/tests/views/test_bundles.py
@@ -146,7 +146,7 @@ class BundleUpdateTest(BundleTestBase):
data = {
'form': 'bundle',
'action': 'update',
- 'name': newname,
+ 'bundle_name': newname,
'public': '',
}
response = self.client.post(bundle_url(self.bundle), data)
@@ -159,7 +159,7 @@ 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)
@@ -243,7 +243,7 @@ class BundlePublicModifyTest(BundleTestBase):
data = {
'form': 'bundle',
'action': 'update',
- 'name': newname,
+ 'bundle_name': newname,
}
self.bundle.name = oldname
self.bundle.save()
@@ -353,7 +353,7 @@ class BundleCreateFromListTest(BundleTestBase):
def test_create_empty_bundle(self):
newbundlename = 'testbundle-new'
- params = {'form': 'patchlistform',
+ params = {'form': 'patch-list-form',
'bundle_name': newbundlename,
'action': 'Create',
'project': self.project.id}
@@ -369,7 +369,7 @@ class BundleCreateFromListTest(BundleTestBase):
newbundlename = 'testbundle-new'
patch = self.patches[0]
- params = {'form': 'patchlistform',
+ params = {'form': 'patch-list-form',
'bundle_name': newbundlename,
'action': 'Create',
'project': self.project.id,
@@ -393,7 +393,7 @@ class BundleCreateFromListTest(BundleTestBase):
n_bundles = Bundle.objects.count()
- params = {'form': 'patchlistform',
+ params = {'form': 'patch-list-form',
'bundle_name': '',
'action': 'Create',
'project': self.project.id,
@@ -414,7 +414,7 @@ class BundleCreateFromListTest(BundleTestBase):
newbundlename = 'testbundle-dup'
patch = self.patches[0]
- params = {'form': 'patchlistform',
+ params = {'form': 'patch-list-form',
'bundle_name': newbundlename,
'action': 'Create',
'project': self.project.id,
@@ -440,7 +440,9 @@ class BundleCreateFromListTest(BundleTestBase):
params)
self.assertNotContains(response, 'Bundle %s created' % newbundlename)
- self.assertContains(response, 'You already have a bundle called')
+ self.assertContains(
+ response,
+ 'A bundle called %s already exists' % newbundlename)
self.assertEqual(Bundle.objects.count(), n_bundles)
self.assertEqual(bundle.patches.count(), 1)
@@ -451,8 +453,8 @@ class BundleCreateFromPatchTest(BundleTestBase):
newbundlename = 'testbundle-new'
patch = self.patches[0]
- params = {'name': newbundlename,
- 'action': 'createbundle'}
+ params = {'bundle_name': newbundlename,
+ 'action': 'Create'}
response = self.client.post(
reverse('patch-detail',
@@ -470,8 +472,8 @@ class BundleCreateFromPatchTest(BundleTestBase):
newbundlename = self.bundle.name
patch = self.patches[0]
- params = {'name': newbundlename,
- 'action': 'createbundle'}
+ params = {'bundle_name': newbundlename,
+ 'action': 'Create'}
response = self.client.post(
reverse('patch-detail',
@@ -489,7 +491,7 @@ 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,
@@ -509,7 +511,7 @@ 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,
@@ -538,7 +540,7 @@ 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,
@@ -559,7 +561,7 @@ 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,
@@ -584,7 +586,7 @@ class BundleAddFromPatchTest(BundleTestBase):
def test_add_to_empty_bundle(self):
patch = self.patches[0]
- params = {'action': 'addtobundle',
+ params = {'action': 'Add',
'bundle_id': self.bundle.id}
response = self.client.post(
@@ -594,7 +596,7 @@ class BundleAddFromPatchTest(BundleTestBase):
self.assertContains(
response,
- 'added to bundle "%s"' % self.bundle.name,
+ 'added to bundle %s' % self.bundle.name,
count=1)
self.assertEqual(self.bundle.patches.count(), 1)
@@ -603,7 +605,7 @@ class BundleAddFromPatchTest(BundleTestBase):
def test_add_to_non_empty_bundle(self):
self.bundle.append_patch(self.patches[0])
patch = self.patches[1]
- params = {'action': 'addtobundle',
+ params = {'action': 'Add',
'bundle_id': self.bundle.id}
response = self.client.post(
@@ -613,7 +615,7 @@ class BundleAddFromPatchTest(BundleTestBase):
self.assertContains(
response,
- 'added to bundle "%s"' % self.bundle.name,
+ 'added to bundle %s' % self.bundle.name,
count=1)
self.assertEqual(self.bundle.patches.count(), 2)
@@ -650,7 +652,7 @@ 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,
'action': 'Create',
'project': self.project.id,
diff --git a/patchwork/tests/views/test_patch.py b/patchwork/tests/views/test_patch.py
index 1a1243c..483ab99 100644
--- a/patchwork/tests/views/test_patch.py
+++ b/patchwork/tests/views/test_patch.py
@@ -304,7 +304,7 @@ class PatchViewTest(TestCase):
class PatchUpdateTest(TestCase):
- properties_form_id = 'patchform-properties'
+ properties_form_id = 'patch-form-properties'
def setUp(self):
self.project = create_project()
@@ -318,7 +318,7 @@ class PatchUpdateTest(TestCase):
self.base_data = {
'action': 'Update',
'project': str(self.project.id),
- 'form': 'patchlistform',
+ 'form': 'patch-list-form',
'archived': '*',
'delegate': '*',
'state': '*'
diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
index cdd6f55..2223202 100644
--- a/patchwork/views/__init__.py
+++ b/patchwork/views/__init__.py
@@ -2,6 +2,7 @@
# Copyright (C) 2008 Jeremy Kerr <jk at ozlabs.org>
#
# SPDX-License-Identifier: GPL-2.0-or-later
+import json
from django.contrib import messages
from django.shortcuts import get_object_or_404
@@ -9,6 +10,7 @@ from django.db.models import Prefetch
from patchwork.filters import Filters
from patchwork.forms import MultiplePatchForm
+from patchwork.forms import CreateBundleForm
from patchwork.models import Bundle
from patchwork.models import BundlePatch
from patchwork.models import Patch
@@ -17,7 +19,6 @@ from patchwork.models import State
from patchwork.models import Check
from patchwork.paginator import Paginator
-
bundle_actions = ['create', 'add', 'remove']
@@ -109,48 +110,35 @@ class Order(object):
# TODO(stephenfin): Refactor this to break it into multiple, testable functions
-def set_bundle(request, project, action, data, patches, context):
+def set_bundle(request, project, action, data, patches):
# set up the bundle
bundle = None
user = request.user
if action == 'create':
bundle_name = data['bundle_name'].strip()
- if '/' in bundle_name:
- return ['Bundle names can\'t contain slashes']
-
- if not bundle_name:
- return ['No bundle name was specified']
-
- if Bundle.objects.filter(owner=user, name=bundle_name).count() > 0:
- return ['You already have a bundle called "%s"' % bundle_name]
-
bundle = Bundle(owner=user, project=project,
name=bundle_name)
- bundle.save()
- messages.success(request, "Bundle %s created" % bundle.name)
+ create_bundle_form = CreateBundleForm(instance=bundle,
+ data=request.POST)
+ if create_bundle_form.is_valid():
+ create_bundle_form.save()
+ addBundlePatches(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())
+ errors = [formErrors['name'][0]['message']]
+ return errors
elif action == 'add':
if not data['bundle_id']:
return ['No bundle was selected']
bundle = get_object_or_404(Bundle, id=data['bundle_id'])
+ addBundlePatches(request, patches, bundle)
elif action == 'remove':
bundle = get_object_or_404(Bundle, id=data['removed_bundle_id'])
-
- if not bundle:
- return ['no such bundle']
-
- for patch in patches:
- if action in ['create', 'add']:
- bundlepatch_count = BundlePatch.objects.filter(bundle=bundle,
- patch=patch).count()
- if bundlepatch_count == 0:
- bundle.append_patch(patch)
- messages.success(request, "Patch '%s' added to bundle %s" %
- (patch.name, bundle.name))
- else:
- messages.warning(request, "Patch '%s' already in bundle %s" %
- (patch.name, bundle.name))
- elif action == 'remove':
+ for patch in patches:
try:
bp = BundlePatch.objects.get(bundle=bundle, patch=patch)
bp.delete()
@@ -161,10 +149,21 @@ def set_bundle(request, project, action, data, patches, context):
request,
"Patch '%s' removed from bundle %s\n" % (patch.name,
bundle.name))
+ return []
- bundle.save()
- return []
+def addBundlePatches(request, patches, bundle):
+ for patch in patches:
+ bundlepatch_count = BundlePatch.objects.filter(bundle=bundle,
+ patch=patch).count()
+ if bundlepatch_count == 0:
+ bundle.append_patch(patch)
+ bundle.save()
+ messages.success(request, "Patch '%s' added to bundle %s" %
+ (patch.name, bundle.name))
+ else:
+ messages.warning(request, "Patch '%s' already in bundle %s" %
+ (patch.name, bundle.name))
def generic_list(request, project, view, view_args=None, filter_settings=None,
@@ -221,17 +220,20 @@ def generic_list(request, project, view, view_args=None, filter_settings=None,
data = None
user = request.user
properties_form = None
+ create_bundle_form = None
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':
data_tmp = data
properties_form = MultiplePatchForm(project, data=data_tmp)
+ if request.user.is_authenticated:
+ create_bundle_form = CreateBundleForm()
- if request.method == 'POST' and data.get('form') == 'patchlistform':
+ 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'
@@ -242,7 +244,7 @@ def generic_list(request, project, view, view_args=None, filter_settings=None,
ps = Patch.objects.filter(id__in=get_patch_ids(data))
if action in bundle_actions:
- errors = set_bundle(request, project, action, data, ps, context)
+ errors = set_bundle(request, project, action, data, ps)
elif properties_form and action == properties_form.action:
errors = process_multiplepatch_form(request, properties_form,
@@ -293,6 +295,7 @@ def generic_list(request, project, view, view_args=None, filter_settings=None,
context.update({
'page': paginator.current_page,
'patchform': properties_form,
+ 'createbundleform': create_bundle_form,
'project': project,
'order': order,
})
diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
index 3e6874a..5ef6916 100644
--- a/patchwork/views/patch.py
+++ b/patchwork/views/patch.py
@@ -14,11 +14,11 @@ from django.urls import reverse
from patchwork.forms import CreateBundleForm
from patchwork.forms import PatchForm
-from patchwork.models import Bundle
from patchwork.models import Cover
from patchwork.models import Patch
from patchwork.models import Project
from patchwork.views import generic_list
+from patchwork.views import set_bundle
from patchwork.views.utils import patch_to_mbox
from patchwork.views.utils import series_patch_to_mbox
@@ -60,6 +60,7 @@ def patch_detail(request, project_id, msgid):
form = None
createbundleform = None
+ errors = None
if editable:
form = PatchForm(instance=patch)
@@ -71,30 +72,10 @@ def patch_detail(request, project_id, msgid):
if action:
action = action.lower()
- if action == 'createbundle':
- bundle = Bundle(owner=request.user, project=project)
- createbundleform = CreateBundleForm(instance=bundle,
- data=request.POST)
- if createbundleform.is_valid():
- createbundleform.save()
- bundle.append_patch(patch)
- bundle.save()
- createbundleform = CreateBundleForm()
- messages.success(request, 'Bundle %s created' % bundle.name)
- elif action == 'addtobundle':
- bundle = get_object_or_404(
- Bundle, id=request.POST.get('bundle_id'))
- if bundle.append_patch(patch):
- messages.success(request,
- 'Patch "%s" added to bundle "%s"' % (
- patch.name, bundle.name))
- else:
- messages.error(request,
- 'Failed to add patch "%s" to bundle "%s": '
- 'patch is already in bundle' % (
- patch.name, bundle.name))
-
- # all other actions require edit privs
+ if action in ['create', 'add']:
+ errors = set_bundle(request, project, action,
+ request.POST, [patch])
+
elif not editable:
return HttpResponseForbidden()
@@ -133,6 +114,8 @@ def patch_detail(request, project_id, msgid):
context['project'] = patch.project
context['related_same_project'] = related_same_project
context['related_different_project'] = related_different_project
+ if errors:
+ context['errors'] = errors
return render(request, 'patchwork/submission.html', context)
diff --git a/templates/base.html b/templates/base.html
index 9bdb35f..e57e2d5 100644
--- a/templates/base.html
+++ b/templates/base.html
@@ -21,7 +21,7 @@
<script src="{% static "js/bootstrap.min.js" %}"></script>
<script src="{% static "js/selectize.min.js" %}"></script>
<script src="{% static "js/clipboard.min.js" %}"></script>
- <script src="{% static "js/js.cookie-2.2.1.min.js" %}"></script>
+ <script src="{% static "js/js.cookie.min.js" %}"></script>
<script>
$(document).ready(function() {
new Clipboard(document.querySelectorAll('button.btn-copy'));
--
2.32.0.432.gabb21c7263-goog
More information about the Patchwork
mailing list