Skip to content

Commit 17726d7

Browse files
Raxel Gutierrezstephenfin
Raxel Gutierrez
authored and
stephenfin
committed
views: Move and refactor patch-forms
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. 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.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. Signed-off-by: Raxel Gutierrez <raxel@google.com> Signed-off-by: Stephen Finucane <stephen@that.guru> [stephenfin: Address merge conflicts]
1 parent 0cea2e4 commit 17726d7

7 files changed

Lines changed: 135 additions & 254 deletions

File tree

‎patchwork/forms.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class BundleForm(forms.ModelForm):
6464
regex=r'^[^/]+$',
6565
min_length=1,
6666
max_length=50,
67-
label='Name',
67+
required=False,
6868
error_messages={'invalid': "Bundle names can't contain slashes"},
6969
)
7070

@@ -76,12 +76,19 @@ class Meta:
7676
class CreateBundleForm(BundleForm):
7777
def clean_name(self):
7878
name = self.cleaned_data['name']
79+
if not name:
80+
raise forms.ValidationError(
81+
'No bundle name was specified', code='invalid'
82+
)
83+
7984
count = Bundle.objects.filter(
8085
owner=self.instance.owner, name=name
8186
).count()
8287
if count > 0:
8388
raise forms.ValidationError(
84-
'A bundle called %s already exists' % name
89+
'A bundle called %(name)s already exists',
90+
code='invalid',
91+
params={'name': name},
8592
)
8693
return name
8794

‎patchwork/templates/patchwork/partials/patch-forms.html‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<div class="patch-forms" id="patch-forms">
2+
{% if patch_form %}
3+
<div id="patch-form-properties" class="patch-form">
4+
<div id="patch-form-state">
5+
{{ patch_form.state.errors }}
6+
{{ patch_form.state }}
7+
</div>
8+
<div id="patch-form-delegate">
9+
{{ patch_form.delegate.errors }}
10+
{{ patch_form.delegate }}
11+
</div>
12+
<div id="patch-form-archive">
13+
{{ patch_form.archived.errors }}
14+
{{ patch_form.archived.label_tag }} {{ patch_form.archived }}
15+
</div>
16+
<button class="patch-form-submit btn btn-primary" name="action" value="update">
17+
Update
18+
</button>
19+
</div>
20+
{% endif %}
21+
{% if user.is_authenticated %}
22+
<div id="patch-form-bundle" class="patch-form">
23+
<div id="create-bundle">
24+
{{ create_bundle_form.name.errors }}
25+
{{ create_bundle_form.name }}
26+
<input class="patch-form-submit btn btn-primary" name="action" value="Create" type="submit"/>
27+
</div>
28+
{% if bundles %}
29+
<div id="add-to-bundle">
30+
<select class="add-bundle" name="bundle_id">
31+
<option value="" selected>Add to bundle</option>
32+
{% for bundle in bundles %}
33+
<option value="{{bundle.id}}">{{bundle.name}}</option>
34+
{% endfor %}
35+
</select>
36+
<input class="patch-form-submit btn btn-primary" name="action" value="Add" type="submit"/>
37+
</div>
38+
{% endif %}
39+
{% if bundle %}
40+
<div id="remove-bundle">
41+
<input type="hidden" name="removed_bundle_id" value="{{bundle.id}}"/>
42+
<button class="patch-form-submit btn btn-primary" name="action" value="Remove">
43+
Remove from bundle
44+
</button>
45+
</div>
46+
{% endif %}
47+
</div>
48+
{% endif %}
49+
</div>

‎patchwork/templates/patchwork/partials/patch-list.html‎

Lines changed: 7 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
{% endblock %}
1010

1111
{% include "patchwork/partials/filters.html" %}
12-
{% include "patchwork/partials/pagination.html" %}
1312

1413
{% if order.editable %}
1514
<table class="patch-list">
@@ -28,18 +27,15 @@
2827
</table>
2928
{% endif %}
3029

31-
{% if page.paginator.long_page and user.is_authenticated %}
32-
<div class="floaty">
33-
<a title="jump to form" href="#patch-forms">
34-
<span style="font-size: 120%">&#9662;</span>
35-
</a>
36-
</div>
37-
{% endif %}
38-
39-
<form method="post">
30+
<form id="patch-list-form" method="post">
4031
{% csrf_token %}
4132
<input type="hidden" name="form" value="patch-list-form"/>
4233
<input type="hidden" name="project" value="{{project.id}}"/>
34+
35+
<div class="patch-list-actions">
36+
{% include "patchwork/partials/patch-forms.html" %}
37+
{% include "patchwork/partials/pagination.html" %}
38+
</div>
4339
<table id="patch-list" class="table table-hover table-extra-condensed table-striped pw-list" data-toggle="checkboxes" data-range="true">
4440
<thead>
4541
<tr>
@@ -159,7 +155,7 @@
159155

160156
<tbody>
161157
{% for patch in page.object_list %}
162-
<tr id="patch-row:{{patch.id}}">
158+
<tr id="patch-row:{{patch.id}}" data-patch-id="{{patch.id}}">
163159
{% if user.is_authenticated %}
164160
<td id="select-patch:{{patch.id}}" style="text-align: center;">
165161
<input type="checkbox" name="patch_id:{{patch.id}}"/>
@@ -201,82 +197,5 @@
201197

202198
{% if page.paginator.count %}
203199
{% include "patchwork/partials/pagination.html" %}
204-
205-
<div class="patch-forms" id="patch-forms">
206-
207-
{% if patch_form %}
208-
<div class="patch-form patch-form-properties">
209-
<h3>Properties</h3>
210-
<table class="form">
211-
<tr>
212-
<th>Change state:</th>
213-
<td>
214-
{{ patch_form.state }}
215-
{{ patch_form.state.errors }}
216-
</td>
217-
</tr>
218-
<tr>
219-
<th>Delegate to:</th>
220-
<td>
221-
{{ patch_form.delegate }}
222-
{{ patch_form.delegate.errors }}
223-
</td>
224-
</tr>
225-
<tr>
226-
<th>Archive:</th>
227-
<td>
228-
{{ patch_form.archived }}
229-
{{ patch_form.archived.errors }}
230-
</td>
231-
</tr>
232-
<tr>
233-
<td></td>
234-
<td>
235-
<input type="submit" name="action" value="{{patch_form.action}}"/>
236-
</td>
237-
</tr>
238-
</table>
239-
</div>
240-
{% endif %}
241-
242-
{% if user.is_authenticated %}
243-
<div class="patch-form patch-form-bundle">
244-
<h3>Bundling</h3>
245-
<table class="form">
246-
<tr>
247-
<td>Create bundle:</td>
248-
<td>
249-
<input type="text" name="bundle_name"/>
250-
<input name="action" value="Create" type="submit"/>
251-
</td>
252-
</tr>
253-
{% if bundles %}
254-
<tr>
255-
<td>Add to bundle:</td>
256-
<td>
257-
<select name="bundle_id">
258-
{% for bundle in bundles %}
259-
<option value="{{bundle.id}}">{{bundle.name}}</option>
260-
{% endfor %}
261-
</select>
262-
<input name="action" value="Add" type="submit"/>
263-
</td>
264-
</tr>
265-
{% endif %}
266-
{% if bundle %}
267-
<tr>
268-
<td>Remove from bundle:</td>
269-
<td>
270-
<input type="hidden" name="removed_bundle_id" value="{{bundle.id}}"/>
271-
<input name="action" value="Remove" type="submit"/>
272-
</td>
273-
</tr>
274-
{% endif %}
275-
</table>
276-
</div>
277-
{% endif %}
278-
<div style="clear: both;">
279-
</div>
280-
</div>
281200
{% endif %}
282201
</form>

‎patchwork/templates/patchwork/submission.html‎

Lines changed: 4 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -132,89 +132,10 @@ <h1>{{ submission.name }}</h1>
132132
{% endif %}
133133
</table>
134134

135-
<div class="patch-forms">
136-
{% if patch_form %}
137-
<div class="patch-form patch-form-properties">
138-
<h3>Patch Properties</h3>
139-
<form method="post">
140-
{% csrf_token %}
141-
<table class="form">
142-
<tr>
143-
<th>Change state:</th>
144-
<td>
145-
{{ patch_form.state }}
146-
{{ patch_form.state.errors }}
147-
</td>
148-
</tr>
149-
<tr>
150-
<th>Delegate to:</th>
151-
<td>
152-
{{ patch_form.delegate }}
153-
{{ patch_form.delegate.errors }}
154-
</td>
155-
</tr>
156-
<tr>
157-
<th>Archived:</th>
158-
<td>
159-
{{ patch_form.archived }}
160-
{{ patch_form.archived.errors }}
161-
</td>
162-
</tr>
163-
<tr>
164-
<td></td>
165-
<td>
166-
<input type="submit" value="Update">
167-
</td>
168-
</tr>
169-
</table>
170-
</form>
171-
</div>
172-
{% endif %}
173-
174-
{% if create_bundle_form %}
175-
<div class="patch-form patch-form-bundle">
176-
<h3>Bundling</h3>
177-
<table class="form">
178-
<tr>
179-
<td>Create bundle:</td>
180-
<td>
181-
{% if create_bundle_form.non_field_errors %}
182-
<dd class="errors">{{create_bundle_form.non_field_errors}}</dd>
183-
{% endif %}
184-
<form method="post">
185-
{% csrf_token %}
186-
<input type="hidden" name="action" value="createbundle"/>
187-
{% if create_bundle_form.name.errors %}
188-
<dd class="errors">{{create_bundle_form.name.errors}}</dd>
189-
{% endif %}
190-
{{ create_bundle_form.name }}
191-
<input value="Create" type="submit"/>
192-
</form>
193-
</td>
194-
</tr>
195-
{% if bundles %}
196-
<tr>
197-
<td>Add to bundle:</td>
198-
<td>
199-
<form method="post">
200-
{% csrf_token %}
201-
<input type="hidden" name="action" value="addtobundle"/>
202-
<select name="bundle_id"/>
203-
{% for bundle in bundles %}
204-
<option value="{{bundle.id}}">{{bundle.name}}</option>
205-
{% endfor %}
206-
</select>
207-
<input value="Add" type="submit"/>
208-
</form>
209-
</td>
210-
</tr>
211-
{% endif %}
212-
</table>
213-
</div>
214-
{% endif %}
215-
<div style="clear: both;">
216-
</div>
217-
</div>
135+
<form id="patch-list-form" method="POST">
136+
{% csrf_token %}
137+
{% include "patchwork/partials/patch-forms.html" %}
138+
</form>
218139

219140
{% if submission.pull_url %}
220141
<h2>Pull-request</h2>

0 commit comments

Comments
 (0)