Skip to content

Commit

Permalink
views: Move and refactor patch-forms
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Signed-off-by: Stephen Finucane <[email protected]>
[stephenfin: Address merge conflicts]
  • Loading branch information
@raxelg @stephenfin
raxelg authored and stephenfin committed Nov 1, 2024
1 parent 0cea2e4 commit 17726d7
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 254 deletions.
11 changes: 9 additions & 2 deletions patchwork/forms.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class BundleForm(forms.ModelForm):
regex=r'^[^/]+$',
min_length=1,
max_length=50,
label='Name',
required=False,
error_messages={'invalid': "Bundle names can't contain slashes"},
)

Expand All @@ -76,12 +76,19 @@ class Meta:
class CreateBundleForm(BundleForm):
def clean_name(self):
name = self.cleaned_data['name']
if not name:
raise forms.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
'A bundle called %(name)s already exists',
code='invalid',
params={'name': name},
)
return name

Expand Down
49 changes: 49 additions & 0 deletions patchwork/templates/patchwork/partials/patch-forms.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<div class="patch-forms" id="patch-forms">
{% if patch_form %}
<div id="patch-form-properties" class="patch-form">
<div id="patch-form-state">
{{ patch_form.state.errors }}
{{ patch_form.state }}
</div>
<div id="patch-form-delegate">
{{ patch_form.delegate.errors }}
{{ patch_form.delegate }}
</div>
<div id="patch-form-archive">
{{ patch_form.archived.errors }}
{{ patch_form.archived.label_tag }} {{ patch_form.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">
{{ create_bundle_form.name.errors }}
{{ create_bundle_form.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>
95 changes: 7 additions & 88 deletions patchwork/templates/patchwork/partials/patch-list.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
{% endblock %}

{% include "patchwork/partials/filters.html" %}
{% include "patchwork/partials/pagination.html" %}

{% if order.editable %}
<table class="patch-list">
Expand All @@ -28,18 +27,15 @@
</table>
{% endif %}

{% if page.paginator.long_page and user.is_authenticated %}
<div class="floaty">
<a title="jump to form" href="#patch-forms">
<span style="font-size: 120%">&#9662;</span>
</a>
</div>
{% endif %}

<form method="post">
<form id="patch-list-form" method="post">
{% csrf_token %}
<input type="hidden" name="form" value="patch-list-form"/>
<input type="hidden" name="project" value="{{project.id}}"/>

<div class="patch-list-actions">
{% include "patchwork/partials/patch-forms.html" %}
{% include "patchwork/partials/pagination.html" %}
</div>
<table id="patch-list" class="table table-hover table-extra-condensed table-striped pw-list" data-toggle="checkboxes" data-range="true">
<thead>
<tr>
Expand Down Expand Up @@ -159,7 +155,7 @@

<tbody>
{% for patch in page.object_list %}
<tr id="patch-row:{{patch.id}}">
<tr id="patch-row:{{patch.id}}" data-patch-id="{{patch.id}}">
{% if user.is_authenticated %}
<td id="select-patch:{{patch.id}}" style="text-align: center;">
<input type="checkbox" name="patch_id:{{patch.id}}"/>
Expand Down Expand Up @@ -201,82 +197,5 @@

{% if page.paginator.count %}
{% include "patchwork/partials/pagination.html" %}

<div class="patch-forms" id="patch-forms">

{% if patch_form %}
<div class="patch-form patch-form-properties">
<h3>Properties</h3>
<table class="form">
<tr>
<th>Change state:</th>
<td>
{{ patch_form.state }}
{{ patch_form.state.errors }}
</td>
</tr>
<tr>
<th>Delegate to:</th>
<td>
{{ patch_form.delegate }}
{{ patch_form.delegate.errors }}
</td>
</tr>
<tr>
<th>Archive:</th>
<td>
{{ patch_form.archived }}
{{ patch_form.archived.errors }}
</td>
</tr>
<tr>
<td></td>
<td>
<input type="submit" name="action" value="{{patch_form.action}}"/>
</td>
</tr>
</table>
</div>
{% endif %}

{% if user.is_authenticated %}
<div class="patch-form patch-form-bundle">
<h3>Bundling</h3>
<table class="form">
<tr>
<td>Create bundle:</td>
<td>
<input type="text" name="bundle_name"/>
<input name="action" value="Create" type="submit"/>
</td>
</tr>
{% if bundles %}
<tr>
<td>Add to bundle:</td>
<td>
<select name="bundle_id">
{% for bundle in bundles %}
<option value="{{bundle.id}}">{{bundle.name}}</option>
{% endfor %}
</select>
<input name="action" value="Add" type="submit"/>
</td>
</tr>
{% endif %}
{% if bundle %}
<tr>
<td>Remove from bundle:</td>
<td>
<input type="hidden" name="removed_bundle_id" value="{{bundle.id}}"/>
<input name="action" value="Remove" type="submit"/>
</td>
</tr>
{% endif %}
</table>
</div>
{% endif %}
<div style="clear: both;">
</div>
</div>
{% endif %}
</form>
87 changes: 4 additions & 83 deletions patchwork/templates/patchwork/submission.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,89 +132,10 @@ <h1>{{ submission.name }}</h1>
{% endif %}
</table>

<div class="patch-forms">
{% if patch_form %}
<div class="patch-form patch-form-properties">
<h3>Patch Properties</h3>
<form method="post">
{% csrf_token %}
<table class="form">
<tr>
<th>Change state:</th>
<td>
{{ patch_form.state }}
{{ patch_form.state.errors }}
</td>
</tr>
<tr>
<th>Delegate to:</th>
<td>
{{ patch_form.delegate }}
{{ patch_form.delegate.errors }}
</td>
</tr>
<tr>
<th>Archived:</th>
<td>
{{ patch_form.archived }}
{{ patch_form.archived.errors }}
</td>
</tr>
<tr>
<td></td>
<td>
<input type="submit" value="Update">
</td>
</tr>
</table>
</form>
</div>
{% endif %}

{% if create_bundle_form %}
<div class="patch-form patch-form-bundle">
<h3>Bundling</h3>
<table class="form">
<tr>
<td>Create bundle:</td>
<td>
{% if create_bundle_form.non_field_errors %}
<dd class="errors">{{create_bundle_form.non_field_errors}}</dd>
{% endif %}
<form method="post">
{% csrf_token %}
<input type="hidden" name="action" value="createbundle"/>
{% if create_bundle_form.name.errors %}
<dd class="errors">{{create_bundle_form.name.errors}}</dd>
{% endif %}
{{ create_bundle_form.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>
Expand Down
Loading

0 comments on commit 17726d7

Please sign in to comment.