Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add series list view #575

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 154 additions & 0 deletions patchwork/templates/patchwork/partials/series-list.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
{% load person %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't need to be a partial. The only reason we have a patch-list partial is because we re-use that partial for both the patch list view and the bundle view (bundles are just lists of patches). You can combine this into series.html.

{% load listurl %}
{% load patch %}
{% load project %}
{% load static %}

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

{% if order.editable %}
<table class="patchlist">
<tr>
<td class="patchlistreorder">
<form method="post" id="reorderform">
{% csrf_token %}
<input type="hidden" name="form" value="reorderform"/>
<input type="hidden" name="order_start" value="0"/>
<span id="reorderhelp"></span>
<input id="reorder-cancel" type="button" value="Cancel" onClick="order_cancel_click(this)"/>
<input id="reorder-change" type="button" value="Change order" onClick="order_button_click(this)"/>
</form>
</td>
</tr>
</table>
{% endif %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is for bundles and doesn't do anything here. It can be removed.


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

<script type="text/javascript">
$(document).ready(function() {
$('#serieslist').stickyTableHeaders();

$('#serieslist').checkboxes('range', true);

$('#check-all').change(function(e) {
if(this.checked) {
$('#serieslist > tbody').checkboxes('check');
} else {
$('#serieslist > tbody').checkboxes('uncheck');
}
e.preventDefault();
});
});
</script>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we doing anything with the checkboxes? Given we don't have any widgets for updating a series, I'd assume not? If so, do we need this?


<form method="post">
{% csrf_token %}
<input type="hidden" name="form" value="serieslistform"/>
<input type="hidden" name="project" value="{{project.id}}"/>
<table id="serieslist" class="table table-hover table-extra-condensed table-striped pw-list" data-toggle="checkboxes" data-range="true">
<thead>
<tr>
{% if user.is_authenticated %}
<th style="text-align: center;">
<input type="checkbox" id="check-all"/>
</th>
{% endif %}

{% if user.is_authenticated and user.profile.show_ids %}
<th>
ID
</th>
{% endif %}

<th>
Version
</th>

<th>
<span class="colinactive">Series</span>
</th>

<th>
{% if order.name == "date" %}
<a class="colactive" href="{% listurl order=order.reversed_name %}">
<span class="glyphicon glyphicon-chevron-{{ order.updown }}"></span>
</a>
<a class="colactive" href="{% listurl order=order.reversed_name %}">
Date
</a>
{% else %}
{% if not order.editable %}
<a class="colinactive" href="{% listurl order="date" %}">Date</a>
{% else %}
<span class="colinactive">Date</span>
{% endif %}
{% endif %}
</th>

<th>
{% if order.name == "submitter" %}
<a class="colactive" href="{% listurl order=order.reversed_name %}">
<span class="glyphicon glyphicon-chevron-{{ order.updown }}"></span>
</a>
<a class="colactive" href="{% listurl order=order.reversed_name %}">
Submitter
</a>
{% else %}
{% if not order.editable %}
<a class="colinactive" href="{% listurl order="submitter" %}">
Submitter
</a>
{% else %}
<span class="colinactive">Submitter</span>
{% endif %}
{% endif %}
</th>
</tr>
</thead>

<tbody>
{% for series in page.object_list %}
<tr id="series_row:{{series.id}}">
{% if user.is_authenticated %}
<td style="text-align: center;">
<input type="checkbox" name="series_id:{{series.id}}"/>
</td>
{% endif %}
{% if user.is_authenticated and user.profile.show_ids %}
<td>
<button type="button" class="btn btn-xs btn-copy" data-clipboard-text="{{ series.id }}" title="Copy to Clipboard">
{{ series.id }}
</button>
</td>
{% endif %}
<td>
{{ series.version|default:"-"}}
</td>
<td>
<a href="../list/?series={{series.id}}">
{{ series.name|default:"[no subject]"|truncatechars:100 }}
</a>
</td>
<td class="text-nowrap">{{ series.date|date:"Y-m-d" }}</td>
<td>{{ series.submitter|personify:project }}</td>
</tr>
{% empty %}
<tr>
<td colspan="8">No series to display</td>
</tr>
{% endfor %}
</tbody>
</table>

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

{% endif %}
</form>
13 changes: 13 additions & 0 deletions patchwork/templates/patchwork/series.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{% extends "base.html" %}

{% load person %}
{% load static %}

{% block title %}{{project.name}}{% endblock %}
{% block series_active %}active{% endblock %}

{% block body %}

{% include "patchwork/partials/series-list.html" %}

{% endblock %}
5 changes: 5 additions & 0 deletions patchwork/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@
patch_views.patch_list,
name='patch-list',
),
path(
'project/<project_id>/series-list/',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should align this with the URL structure of the rest of the URLs. For example, project/<project_id>/series/.

We'll also eventually need a series detail view (and a new-style mbox and patch view), but it's okay to tackle that separately.

patch_views.series_list,
name='series-list',
),
path(
'project/<project_id>/bundles/',
bundle_views.bundle_list,
Expand Down
87 changes: 53 additions & 34 deletions patchwork/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from patchwork.models import Bundle
from patchwork.models import BundlePatch
from patchwork.models import Patch
from patchwork.models import Series
from patchwork.models import Project
from patchwork.models import Check
from patchwork.paginator import Paginator
Expand Down Expand Up @@ -179,6 +180,7 @@ def generic_list(
filter_settings=None,
patches=None,
editable_order=False,
series_view=False
):
if not filter_settings:
filter_settings = []
Expand Down Expand Up @@ -273,48 +275,65 @@ def generic_list(
else:
context['filters'].set_status(filterclass, setting)

if patches is None:
patches = Patch.objects.filter(project=project)
if series_view:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given we're not modifying anything here, I don't think there's any reason to re-use this function. It's already way too complicated for it's own good. Better to split this out into a new view function, IMO.

series_list = Series.objects.filter(project=project)
series_list = series_list.only(
'submitter',
'project',
'version',
'name',
'date',
'id',
)
series_list = series_list.select_related('project')

# annotate with tag counts
patches = patches.with_tag_counts(project)
if not editable_order:
series_list = order.apply(series_list)

patches = context['filters'].apply(patches)
if not editable_order:
patches = order.apply(patches)
paginator = Paginator(request, series_list)
else:
if patches is None:
patches = Patch.objects.filter(project=project)

# we don't need the content, diff or headers for a list; they're text
# fields that can potentially contain a lot of data
patches = patches.defer('content', 'diff', 'headers')
# annotate with tag counts
patches = patches.with_tag_counts(project)

# but we will need to follow the state and submitter relations for
# rendering the list template
patches = patches.select_related(
'state', 'submitter', 'delegate', 'series'
)
patches = context['filters'].apply(patches)
if not editable_order:
patches = order.apply(patches)

patches = patches.only(
'state',
'submitter',
'delegate',
'project',
'series__name',
'name',
'date',
'msgid',
)
# we don't need the content, diff or headers for a list; they're text
# fields that can potentially contain a lot of data
patches = patches.defer('content', 'diff', 'headers')

# we also need checks and series
patches = patches.prefetch_related(
Prefetch(
'check_set',
queryset=Check.objects.only(
'context', 'user_id', 'patch_id', 'state', 'date'
),
# but we will need to follow the state and submitter relations for
# rendering the list template
patches = patches.select_related(
'state', 'submitter', 'delegate', 'series'
)

patches = patches.only(
'state',
'submitter',
'delegate',
'project',
'series__name',
'name',
'date',
'msgid',
)

# we also need checks and series
patches = patches.prefetch_related(
Prefetch(
'check_set',
queryset=Check.objects.only(
'context', 'user_id', 'patch_id', 'state', 'date'
),
)
)
)

paginator = Paginator(request, patches)
paginator = Paginator(request, patches)

context.update(
{
Expand Down
16 changes: 16 additions & 0 deletions patchwork/views/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,22 @@ def patch_list(request, project_id):
return render(request, 'patchwork/list.html', context)


def series_list(request, project_id):
project = get_object_or_404(Project, linkname=project_id)
context = generic_list(
request,
project,
'series-list',
view_args={'project_id': project.linkname},
series_view=True,
)

if request.user.is_authenticated:
context['bundles'] = request.user.bundles.all()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't used anywhere so it doesn't need to be here.


return render(request, 'patchwork/series.html', context)


def patch_detail(request, project_id, msgid):
project = get_object_or_404(Project, linkname=project_id)
db_msgid = Patch.decode_msgid(msgid)
Expand Down
6 changes: 6 additions & 0 deletions templates/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@
Patches
</a>
</li>
<li class="{% block series_active %}{% endblock %}">
<a href="{% url 'series-list' project_id=project.linkname %}">
<span class="glyphicon glyphicon-file"></span>
Series
</a>
</li>
<li class="{% block bundle_active %}{% endblock %}">
<a href="{% url 'bundle-list' project_id=project.linkname %}">
<span class="glyphicon glyphicon-gift"></span>
Expand Down