Daniel Axtens [Thu, 25 Jan 2018 02:43:11 +0000 (13:43 +1100)]
pagination: Fix quirks
There are a couple of pages where the clickable list of pages
would include missing or duplicate pages.
Write a test that ensures:
- you always have a link to the next/prev numbered page
- there are no duplicate page numbers
Fiddle with the pagination algorithm to get it to pass - required
tweaking a display parameter and a couple of comparison operators,
so all pretty minor.
Now, if there are 10 pages, the displayed page numbers for a given
page are as follows:
Stephen Finucane [Wed, 17 Jan 2018 19:39:36 +0000 (19:39 +0000)]
parser: Handle 'git-request-pull' mails from Git 2.14.3
Make the regex case insensitive to catch both 'git' and 'Git'.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: #159 Reviewed-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
docs/development: Fix tox invocation for listing targets
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Sun, 10 Dec 2017 17:30:32 +0000 (17:30 +0000)]
Modify sections used in release notes
Add an 'api' section and remove 'security' and 'issues'. The former will
helps us group REST API related changes, while the latter are not
currently used and are unlikely to ever be used.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Fri, 24 Nov 2017 19:37:42 +0000 (19:37 +0000)]
Remove support for Django 1.6, 1.7
These versions are massively outdated and the only reason for keeping
them was to allow installation on RHEL 7 using the version provided via
EPEL. No one's actually using this so just kill it.
This also allows us to remove support for django-filter 0.11, which was
only retained for use with these older versions of Django.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
Andrew Donnellan [Tue, 19 Dec 2017 05:32:03 +0000 (16:32 +1100)]
models, templatetags: Make tag count column in patch list optional per tag
Add a field, show_column, to the Tag model to determine whether the tag
gets a tag count column in the patch list view. This allows the creation of
tags that will be collated when generating mboxes but won't take up space
in the patch list.
show_column will default to True to maintain the current behaviour by
default.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au> Closes: #142 ("Ability to add tags that don't also have a column in the UI") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
Andrew Donnellan [Wed, 20 Dec 2017 06:34:24 +0000 (17:34 +1100)]
templates: Add click-to-copy patch ID ("mpe mode") to patch detail page
Similar to what we already do on the patch list page, display the patch ID
on the patch detail page and make it a click-to-copy button.
Closes: #115 ("Show copy-able patch ID ("mpe mode") on patch detail page") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
Tom Rini [Mon, 19 Dec 2016 21:54:17 +0000 (16:54 -0500)]
post-receive.hook: Handle failure to find patch number
When pwclient info -h fails to come up with the number for the change in
question it will exit with a non-zero exit code. This failure will
propagate upwards and exit the script there. Make our call to
get_patch_id or in true so that our script here will see an empty id and
we continue on with the list.
Signed-off-by: Tom Rini <trini@konsulko.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Andrew Donnellan [Tue, 19 Dec 2017 05:41:27 +0000 (16:41 +1100)]
views: Don't render token section of user profile if REST API disabled
In profile.html, if settings.ENABLE_REST_API == False, trying to render a
link to the generate_token page will raise a NoReverseMatch exception, so
we shouldn't render that. In any case, if the REST API is disabled, we
really shouldn't render the API token section of the page at all.
Only render the API token and generation link if settings.ENABLE_REST_API
is True.
Reported-by: Tomas Novotny <tomas@novotny.cz> Closes: #138 ("NoReverseMatch exception on user login with disabled REST API") Fixes: 85c8f369204a ("views: Provide a way to view, (re)generate tokens") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Thu, 23 Nov 2017 23:54:27 +0000 (10:54 +1100)]
postgres: Fix version to 9.6
Always version your dependencies, people.
Postgres 10 has been released, the data format is not backwards
compatible with Postgres 9.6. I don't think too many people have
moved to 10 yet, so test with 9.6 for now.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Fri, 19 May 2017 14:53:25 +0000 (15:53 +0100)]
tox: Add Django 1.11
Add support for the latest release of Django, 1.11. This is the next LTS
release (1.8 being the last one), so it's particularly important that we
maintain support for this going forward.
While neither the latest releases of django-rest-framework nor that of
django-filter explicitly support Django 1.11, it appears that they are
functional [1][2]. We can bump these packages separately when new
versions are released.
Stephen Finucane [Fri, 19 May 2017 14:53:22 +0000 (15:53 +0100)]
REST: Allow for mutability of request.POST
Using Django 1.11 yields the following error for the 'patchwork.tests
.test_rest_api.TestCheckAPI.test_create' test:
AttributeError: This QueryDict instance is immutable
This occurs due to our modification of data to allow users to create
instances using a slugified state instead of the underlying integer
value, e.g. 'success' instead of 1.
Resolve this by unsetting the immutability of that queryset. As
suggested in the linked SO answer, there is limited side effects to
doing this.
Signed-off-by: Stephen Finucane <stephen@that.guru> Tested-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Fri, 19 May 2017 14:54:08 +0000 (15:54 +0100)]
tox: Remove '--liveserver' parameter
This parameter is no longer supported in Django 1.11 [1]. Remove this
and instead set the 'DJANGO_LIVE_TEST_SERVER_ADDRESS' environment
variable, which will do the same thing for Django < 1.11 and be ignored
by Django >= 1.11.
Daniel Axtens [Fri, 20 Oct 2017 05:36:55 +0000 (16:36 +1100)]
Support testing with PostgreSQL
This allows us to easily test against PostgreSQL using the same
tooling we normally use. This is helpful in (for example) shaking
out the test failures that were observed on ozlabs.org
To use it:
docker-compose -f docker-compose-pg.yml <usual argument>
(You may find in necessary to do a 'docker-compose down' first,
depending on what state the system is in and what command you're
running.)
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Fri, 20 Oct 2017 05:36:53 +0000 (16:36 +1100)]
REST: Specify default ordering fields
This hides warnings likes the following:
UnorderedObjectListWarning: Pagination may yield inconsistent
results with an unordered object_list: <QuerySet [<User:
test_user_0>]>
Signed-off-by: Stephen Finucane <stephen@that.guru>
[dja: fix merge conflict in patch.py] Signed-off-by: Daniel Axtens <dja@axtens.net> Acked-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Thu, 28 Sep 2017 14:12:24 +0000 (00:12 +1000)]
xmlrpc/patch_list: only fetch required fields
OzLabs noticed *massive* slowdowns in queries like this one:
SELECT "patchwork_submission"."id", "patchwork_submission"."msgid",
"patchwork_submission"."date", "patchwork_submission"."headers",
"patchwork_submission"."submitter_id",
"patchwork_submission"."content", "patchwork_submission"."project_id",
"patchwork_submission"."name", "patchwork_patch"."submission_ptr_id",
"patchwork_patch"."diff", "patchwork_patch"."commit_ref",
"patchwork_patch"."pull_url", "patchwork_patch"."delegate_id",
"patchwork_patch"."state_id", "patchwork_patch"."archived",
"patchwork_patch"."hash" FROM "patchwork_patch" INNER JOIN
"patchwork_submission" ON ("patchwork_patch"."submission_ptr_id" =
"patchwork_submission"."id") WHERE
("patchwork_submission"."project_id" = 2 AND
"patchwork_patch"."state_id" = 1) ORDER BY
"patchwork_submission"."date" ASC
These appear to be a result of pwclient list operations. We *do not*
need content/headers/diff in this case - so do not fetch them as it
is incredibly expensive - queries in excess of 50s have been observed.
This should go to stable/2.0.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Reported-by: Michael Ellerman <mpe@ellerman.id.au> Cc: Jeremy Kerr <jk@ozlabs.org> Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Thu, 31 Aug 2017 08:48:51 +0000 (09:48 +0100)]
REST: Filter projects by 'linkname', not 'name'
Based on a report on the OVS mailing list, it appears that projects are
being filtered on the 'Project.name' field instead of the
'Project.linkname' field. Correct this and add regression tests to
prevent it happening again.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 08b2115 ("REST: Allow filtering by both project ID and linkname")
Closes-bug: #117 ("Projects are filtered on the wrong field") Cc: Daniel Axtens <dja@axtens.net>
[dja: drop mangling of value] Signed-off-by: Daniel Axtens <dja@axtens.net>
Andrew Donnellan [Thu, 31 Aug 2017 04:31:24 +0000 (14:31 +1000)]
views: Fix "Add to bundle" dropdown on patch list view
The "Add to bundle" option in the patch list view requires the user's
list of bundles to be added to the context. Before PatchworkRequestContext
was removed, this was done in a context processor. When that was
refactored out, the bundles list was re-added in the patch detail view,
but not in the patch list view.
Add the bundles list in the patch list view to rectify this.
Reported-by: David Miller <davem@davemloft.net> Reported-by: Michael Ellerman <mpe@ellerman.id.au> Reported-by: Paul Mackerras <paulus@ozlabs.org> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Closes #116 ("Add to existing bundle drop down has disappeared from bottom of patch list page") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Jeremy Kerr [Mon, 28 Aug 2017 11:39:18 +0000 (19:39 +0800)]
lib/sql: fix permissions for v2.0.0 on postgres
Some tables are no longer present, and others that are used by the web
interface and mail parser need access permissions added.
This change was required to get patchwork going on patchwork.ozlabs.org;
there may be other permissions required, that we haven't hit yet. So,
some review would be good here.
Also: it's unlikely that we need DELETE for the mail parser, but I'm not
confident enough to remove that at the moment.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Reviewed-by: Stephen Finucane <stephen@that.guru>
Jeremy Kerr [Mon, 28 Aug 2017 11:39:17 +0000 (19:39 +0800)]
tests: Run FuzzTest within a transaction
Currently, the FuzzTests fail for me with:
/backends/base/base.py", line 428, in validate_no_broken_transaction
"An error occurred in the current transaction. You can't "
TransactionManagementError: An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block.
- because the SQL inserts can fail, during an active transaction (the
first failure I see is attempting to insert \0 chars in
codec-null.mbox); this causes the setup for the next test case to fail.
Instead, run each test in its own transaction.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Mon, 28 Aug 2017 04:11:27 +0000 (14:11 +1000)]
Handle EmptyPage exceptions
If a user asks for a page beyond the range of pages, an EmptyPage
exception is thrown. Catch this and clamp the page number
appropriately.
Reported-by: Jeremy Kerr <jk@ozlabs.org> Signed-off-by: Daniel Axtens <dja@axtens.net> Tested-by: Jeremy Kerr <jk@ozlabs.org> Reviewed-by: Stephen Finucane <stephen@that.guru>
Andrew Donnellan [Sun, 20 Aug 2017 14:40:10 +0000 (00:40 +1000)]
models: Fix invocation of refresh_tag_counts() for Comments
In Comment.save() and Comment.delete(), we always call
Submission.refresh_tag_counts(), which is an empty stub, rather than
calling Patch.refresh_tag_counts() if the Submission is a Patch.
As such, tag counts are never updated on incoming comments.
Delete Submission.refresh_tag_counts(), as it's useless, and in
Comment.save()/delete(), invoke Patch.refresh_tag_counts() directly when
the submission is a Patch.
Reported-by: David Demelier <markand@malikania.fr> Fixes: 86172ccc161b ("models: Split Patch into two models")
Closes-bug: #111 ("A/R/T not updated on comments") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Tue, 11 Jul 2017 06:30:48 +0000 (16:30 +1000)]
cron: fix deletion of unactivated accounts
There is a test in expire_notifications() that tries to check if
the user's last login matches the date joined. (I think the login
date is not set until a post-activation login.) This does not work:
on patchwork.ozlabs.org there are 10k users that have never been
deleted.
Drop the date test: it should be sufficient that a user is not
active and their confirmation is not pending.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Fri, 7 Jul 2017 14:24:46 +0000 (00:24 +1000)]
parser: fix parsing of messages with empty subjects
The fuzz fixups made the test too strict ("if not subject" rather than
"if subject is None") in an attempt to catch broken subject headers.
This broke parsing of messages with an empty subject.
Fix it and add a test.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Sat, 1 Jul 2017 04:28:43 +0000 (14:28 +1000)]
parser: limit emails and names to 255 chars
Also picked up with afl-fuzz.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Wed, 28 Jun 2017 07:48:48 +0000 (17:48 +1000)]
parser: better date parsing
It turns out that there is a lot that can go wrong in parsing a
date. OverflowError, ValueError and OSError have all been observed.
If these go wrong, substitute the current datetime.
Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Daniel Axtens [Wed, 28 Jun 2017 07:48:47 +0000 (17:48 +1000)]
parser: deal with headers entirely failing to parse
It turns out that the attempts in clean_header() to convert
headers to strings are not guaranteed to work: you can end up with,
for example, a base64 decoding error which makes it impossible
to determine any header content.
In this case, sanitise_header() should return None, and thus
clean_header() should return None. We then need to plumb that
through.
Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Daniel Axtens [Wed, 28 Jun 2017 07:48:46 +0000 (17:48 +1000)]
parser: catch failures in decoding headers
Headers can fail to decode:
- if a part cannot be encoded as ascii
- if the coding hint names a codec that doesn't exist
- if there's a null byte in the codec name
Catch these errors.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Wed, 28 Jun 2017 07:48:44 +0000 (17:48 +1000)]
parser: don't assume headers are strings
In python3, mail.get() can return either a string, or an
email.header.Header type.
clean_header() is designed to clean headers into strings,
so make sure we use that everywhere.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Wed, 28 Jun 2017 07:48:43 +0000 (17:48 +1000)]
parser: fix charset 'guessing' algorithm
The charset guessing algorithm doesn't work if it has to guess
multiple charsets, as it overwrites the payload with None.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Wed, 28 Jun 2017 06:55:09 +0000 (16:55 +1000)]
Remove ResourceWarnings under Py3
This is just a matter of correctly closing files we open.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Aaron Conole [Wed, 14 Jun 2017 18:14:23 +0000 (14:14 -0400)]
events-api: allow filtering by date
This commit allows users of the REST API to query for events based on
the date field. This will allow utility writers to select a smaller
subset of events when polling.
Signed-off-by: Aaron Conole <aconole@bytheb.org> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Wed, 28 Jun 2017 19:54:03 +0000 (20:54 +0100)]
parser: Trivial fix of test docstring
Based on the 'References' and 'In-Reply-To' headers of the used mbox,
the docstring for the 'test_reply_nocover_noversion' test is incorrect.
Fix this.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Sean Farley <sean@farley.io>
Stephen Finucane [Wed, 28 Jun 2017 19:53:58 +0000 (20:53 +0100)]
parser: Support single-patch "series"
There are merits to supporting single-patch series, not least the
ability to provide two consistent interfaces that show _all_ patches in
the '/patches' endpoint and the '/series' endpoint.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Sean Farley <sean@farley.io>
Stephen Finucane [Sun, 25 Jun 2017 20:55:54 +0000 (21:55 +0100)]
parser: Use a series even if the version differs
Currently, once we've found a series for a new submission, we check to
ensure that the version of that series matches that of the submission,
and we discard the series if not. The original intention for this was
given in commit 'c21b30525', where this was added:
There are some things you probably shouldn't do on public mailing
lists, but which people do anyway.
- [PATCH 1/2] test: Add some lorem ipsum
- [PATCH 2/2] test: Convert to Markdown
- [PATCH v2 1/2] test: Add some lorem ipsum
- [PATCH v2 2/2] test: Convert to Markdown
We should correctly parse these...
This is unnecessary for two reasons:
- If the series is using proper references, then the mechanism that
we use to prevent this issue with unversioned follow ups (also added
in that patch) will work here: namely, we don't add submissions if an
submission with a given number already exists in that series.
- If the series is not using references, we already have a check for
version.
Seeing as this check is actually causing issues for legitimate typos, we
should just remove this check. Do so, adding a check to ensure we don't
regress.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Closes-bug: #105
Token authentication is generally viewed as a more secure option for API
authentication than storing a username and password.
Django REST Framework gives us a TokenAuthentication class and an authtoken
app that we can use to generate random tokens and authenticate to API
endpoints. Enable this support and add some tests to validate correct
behavior.
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
We provide our own, much smaller implementation of this currently.
However, we want to be able to implement slightly different variants of
this elsewhere and using an existing library helps avoid reinventing the
wheel and lets us use already battle-tested code.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Stephen Finucane [Tue, 30 May 2017 20:33:58 +0000 (21:33 +0100)]
models: Centralize generation of filenames
Move filename generation to a mixin. This allows us to reuse the code
for other items like cover letters. Some unncessary 'strip' calls are
removed as their unnecessary.
This allows us to change the file extension for diffs to 'diff', which
is a little more accurate.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Tue, 23 May 2017 14:08:13 +0000 (15:08 +0100)]
parser: Handle multiple reference headers
It's possible to duplicate message headers multiple times. One common
case is the 'Received' header, but it appears that multiple
'In-Reply-To' and 'References' headers are also a thing.
Handle these cases through the use of the 'Message.get_all' function,
which returns all matching headers, instead of the 'Message.get'
function previously used.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Wed, 24 May 2017 05:54:55 +0000 (06:54 +0100)]
parser: Strip whitespace from references
Some mail, particularly those generated with older versions of
git-send-email or written by hand, include some extra whitespace in the
'References' and 'In-Reply-To' lines. Ensure we always strip this,
preventing mismatches between this and 'Message-ID', which is already
stripped of whitespace, when looking up SeriesReference's.
Signed-off-by: Stephen Finucane <stephen@that.guru>