Stephen Finucane [Fri, 25 Mar 2016 17:29:29 +0000 (17:29 +0000)]
models: Split Patch into two models
There are a lot of similarities between cover letters and patches: so
many, in fact, that it would be helpful to occasionally treat them as
the same thing. Achieve this by extracting out the fields that would be
shared between a Patch and a hypothetical cover letter into a "sub
model". This allows us to do cool stuff like assigning comments to both
patches and cover letters or listing both patches and cover letters on
the main screen in a natural way.
The migrations for this are really the only complicated part. There are
three, broken up into schema and data migrations per Django customs,
and they works as follows:
* Rename the 'Patch' model to 'Submission', then create a subclass
called 'Patch' that includes duplicates of the patch-specific fields
of Submission (with changed names to prevent conflicts). Rename
non-patch specific references to the renamed 'Submission' model
as necessary.
* Duplicate the contents of the patch-specific fields from 'Submission'
to 'Patch'
* Remove the patch-specific fields from 'Submission', renaming the
'Patch' model to take their place. Update the patch-specific
references to point the new 'Patch' model, rather than 'Submission'.
This comes at the cost of an additional JOIN per item on the main
screen, but this seems a small price to pay for the additional
functionality gained. To minimise this, however, caching will be added.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Signed-off-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Fri, 25 Mar 2016 17:29:28 +0000 (17:29 +0000)]
utils: Rework how notifications are selected
The current method of filtering valid patches from invalid ones is
effective, but incompatible with forthcoming rework that splits
the 'Patch' model into two models. Rework this, removing the
soon-to-be non-existant 'User.patch' reference.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
parsearchive: Fix default value of verbosity argument
With the current parsearchive.py, the default value for the verbosity
argument does not exist in the VERBOSITY_LEVELS dict, so we get:
Traceback (most recent call last):
File "./patchwork/bin/parsemail.py", line 569, in <module>
sys.exit(main(sys.argv))
File "./patchwork/bin/parsemail.py", line 555, in main
logging.basicConfig(level=VERBOSITY_LEVELS[args['verbosity']])
KeyError: 20
This change uses an actual key instead.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
Stephen Finucane [Tue, 29 Mar 2016 16:10:20 +0000 (17:10 +0100)]
ui: Cleanup the navbar "brand"
The text shown in the top-left corner of a navbar, known as the "brand"
in Bootstrap parlance, normally brings you to the homepage. However,
when viewing patches or details about a project this name of this
project is included in this link. This gave some users the idea that
clicking this button would return them to the project patch list
instead. To resolve this confusion, break the project name and any
other non-Patchwork title out of this clickable link. A new
"navbar-subnav" style is introduced to do this, seeing as Bootstrap
does not appear to support something like this natively.
Once this is done, this also allows for the removal of the "All
Projects" button, which both duplicates this functionality and did
nothing when there was only one project available on an instance.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
Stephen Finucane [Tue, 29 Mar 2016 12:37:00 +0000 (13:37 +0100)]
ui: Higher density of patches
THe Bootstrap'ification of Patchwork signficantly reduced the
information density of the patch list page, but this results in less
patches per page. Resolve this by reducing the padding on each row.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
Damien Lespiau [Thu, 4 Feb 2016 13:56:51 +0000 (13:56 +0000)]
settings: Help caching with content-aware static file names
If we always serve the "style.css" file, even when its content changes,
we run into the situation where the browser (or any HTTP aware middle
man) will cache the file and not download a newer version.
This can be solved by appending the file hash to its filename. If the
content changes, the file name changes and the new version is used right
away. We can also use far future Expires headers with such static files,
they will really never change.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
---
v2: Disable for Django < 1.7
Jeremy Kerr [Mon, 28 Mar 2016 03:31:38 +0000 (11:31 +0800)]
parsemail: Fix return value for find_content error case
If we fail to decode a message payload, we'll fail with the following:
Traceback (most recent call last):
File "./patchwork/bin/parsemail.py", line 563, in <module>
sys.exit(main(sys.argv))
File "./patchwork/bin/parsemail.py", line 553, in main
return parse_mail(mail, args['list_id'])
File "./patchwork/bin/parsemail.py", line 464, in parse_mail
(patch, comment, filenames) = find_content(project, mail)
ValueError: need more than 2 values to unpack
- as the error condition for find_content only returns a 2-tuple. This
change fixes the error case to the 3-tuple return type.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Acked-by: Stephen Finucane <stephen.finucane@intel.com>
Jeremy Kerr [Sun, 27 Mar 2016 04:53:06 +0000 (12:53 +0800)]
templates: don't emit tags for empty URLs, allow both web_url & webscm_url
Rather than always outputting one of web_url or webscm_url, only output
when these are present. This prevents us from emitting empty links if
both are missing.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Acked-by: Stephen Finucane <stephen.finucane@intel.com>
Jeremy Kerr [Sun, 27 Mar 2016 04:53:04 +0000 (12:53 +0800)]
parsemail: Fix default value of verbosity argument
With the current parsemail.py, the default value for the verbosity
argument does not exist in the VERBOSITY_LEVELS dict, so we get:
Traceback (most recent call last):
File "./patchwork/bin/parsemail.py", line 569, in <module>
sys.exit(main(sys.argv))
File "./patchwork/bin/parsemail.py", line 555, in main
logging.basicConfig(level=VERBOSITY_LEVELS[args['verbosity']])
KeyError: 20
This change uses an actual key instead.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Acked-by: Stephen Finucane <stephen.finucane@intel.com>
When parsemail is used as a delivery command from a mail server like
postfix (as it is intended to be), a non-zero exit code will cause a
bounce message to be returned to the user. From the postfix manual:
When the command fails, a limited amount of command output is
mailed back to the sender. The file /usr/include/sysexits.h
defines the expected exit status codes.
For cases where patchwork is unavailable, we absolutely do not want to
start bouncing messages to all patch contributors.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Acked-by: Stephen Finucane <stephen.finucane@intel.com>
Stephen Finucane [Fri, 25 Mar 2016 13:47:16 +0000 (13:47 +0000)]
requirements: Use any supported Django version
Patchwork now supports Django 1.9. It continues to support Django 1.8,
1.7 and 1.6, though the latter two are not supported upstream and are
therefore not recommended. Update development requirements to reflect
this.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Reviewed-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Mon, 21 Mar 2016 17:30:16 +0000 (17:30 +0000)]
views: Use messages framework in 'generic_list'
Finalise the use of the messages framework by using it in the
aforementioned function. This allows for removal of messages-related
code from the 'PatchworkRequestContext' dictionary.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Tested-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Mon, 21 Mar 2016 16:31:11 +0000 (16:31 +0000)]
views: Use messages framework in 'patch'
Django provides the messages framework as part of the core library.
This framework allows for "toast"-style notifications to the user,
for things like post-form processing notifcations. At the moment
patchwork provides this functionality using custom code. However, this
code is not well tested and, like any code, incurs some degree of
maintenance overhead. It would be easier to use the "batteries" that
Django provides so begin doing just that.
This change only covers one of two places in which this custom
messages framework is currently used. By extension, it also covers one
of the two places in which 'render_to_response' is still used. This
"other place" will be addressed in a follow-up commit.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Tested-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Mon, 21 Mar 2016 17:37:42 +0000 (17:37 +0000)]
views: Use context dictionaries in '__init__'
Remove the use of PatchworkRequestContext in one function of this view
as it is not required and causes a 'RemovedInDjango110Warning' warning.
This requires the use of 'render', rather than 'render_to_response'.
The remaining use of this context option will be addressed in a
follow-up patch.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Tested-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Mon, 21 Mar 2016 13:54:56 +0000 (13:54 +0000)]
views: Use context dictionaries in 'bundle'
Remove the use of PatchworkRequestContext in this view as it is not
required and causes a 'RemovedInDjango110Warning' warning. This
requires the use of 'render', rather than 'render_to_response'.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Tested-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Mon, 21 Mar 2016 13:30:20 +0000 (13:30 +0000)]
views: Use context dictionaries in 'mail'
Remove the use of PatchworkRequestContext in this view as it is not
required and causes a 'RemovedInDjango110Warning' warning. This
requires the use of 'render', rather than 'render_to_response'. It
also requires adding a 'request' parameter to 'render_to_string' calls
so that the default context processors continue to be passed through.
This removes a context variable that was not used.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Tested-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Wed, 16 Mar 2016 16:00:18 +0000 (16:00 +0000)]
views: Use context dictionaries in 'user'
Remove the use of PatchworkRequestContext in this view as it is not
required and causes a 'RemovedInDjango110Warning' warning. This
requires the use of 'render', rather than 'render_to_response'. It
also requires adding a 'request' parameter to 'render_to_string' calls
so that the default context processors continue to be passed through.
This includes a typo fix for another template.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Tested-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Tue, 15 Mar 2016 22:05:33 +0000 (22:05 +0000)]
views: Use context dictionaries in 'pwclient'
Remove the use of PatchworkRequestContext in this view as it is not
required and causes a 'RemovedInDjango110Warning' warning. This
requires the use of 'render', rather than a combo of 'response.write'
and 'render_to_string'.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Tested-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Tue, 15 Mar 2016 22:05:15 +0000 (22:05 +0000)]
views: Use context dictionaries in 'help'
Remove the use of PatchworkRequestContext in this view as it is not
required and causes a 'RemovedInDjango110Warning' warning. This
requires the use of 'render', rather than 'render_to_response'.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Tested-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Tue, 15 Mar 2016 21:17:22 +0000 (21:17 +0000)]
views: Use context dictionaries in 'project'
Remove the use of PatchworkRequestContext in this view as it is not
required and causes a 'RemovedInDjango110Warning' warning. This
requires the use of 'render', rather than 'render_to_response'.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Tested-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Thu, 24 Mar 2016 11:05:45 +0000 (11:05 +0000)]
compat: Wrap 'render_to_string'
The 'dictionary' and 'context_instance' parameters for the
'render_to_string' function produce 'RemovedInDjango110' warnings
in Django 1.9 and will be removed in Django 1.10. However, these
parameters are still valid for Django < 1.8. Provide a shim to
allow use of the Django>=1.8-style parameters, while retaining
backwards compatibility with Django < 1.8.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Tested-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Mon, 21 Mar 2016 14:07:01 +0000 (14:07 +0000)]
context: Add 'site' context processor
This allows us to use the current "site" value in all templates without
requiring PatchworkRequestContext. As a custom context processor is
being enabled, the 'TEMPLATE_CONTEXT_PROCESSORS' value must be
specified for versions of Django < 1.8 (it's already specified for
other versions).
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Tested-by: Andy Doan <andy.doan@linaro.org>
Andy Doan [Thu, 24 Mar 2016 18:20:43 +0000 (13:20 -0500)]
migrations: Optimize 0007 migration
By handling comment copying/deletion via SQL we can make migration take
< 3 minutes rather than > 30 minutes for big instances. The SQL used is
vendor specific, but covers the two DBs supported by Patchwork.
Signed-off-by: Andy Doan <andy.doan@linaro.org> Reviewed-by: Stephen Finucane <stephen.finucane@intel.com>
Stephen Finucane [Wed, 23 Mar 2016 18:24:03 +0000 (18:24 +0000)]
settings: Use explicit setup for the Debug Toolbar
The 'django-debug-toolbar' application provides an automatic method of
configuring the plugin. However, as noted in the documentation [1],
this can cause issues like circular imports. In this case, a
"Table 'patchwork.patchwork_state' doesn't exist" exception was being
raised when attempting to do an initial migration.
Resolve this by using the manual configuration provided in the docs.
Stephen Finucane [Wed, 23 Mar 2016 11:37:35 +0000 (11:37 +0000)]
settings: Disable Debug Toolbar for all Django<1.7
The django-debug-toolbar project doesn't support Django 1.6, but the
check used to disable this plugin for this version only works for
Django 1.6.0 and less. Enable it for all versions of Django < 1.7.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
Stephen Finucane [Tue, 15 Mar 2016 21:14:39 +0000 (21:14 +0000)]
templates: Fix regression in reversed URL
'f8cc68d' introduced a regression on the changes made in 'fd3bfaa'.
Namely, Python path-style reversing was used, rather than reverse by
name. Resolve this regression.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
Stephen Finucane [Tue, 15 Mar 2016 11:39:35 +0000 (11:39 +0000)]
views: Prefetch 'Patch.check_set'
The checks for each Patch are queried each time patches are listed.
This causes a deluge of queries (around one per patch). Mitigate this
by instead prefetching this related attribute. This reduces the number
of queries for a list of 100 patches from an average of 119 to 20.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Reviewed-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Thu, 15 Oct 2015 23:32:01 +0000 (00:32 +0100)]
models: Pull common email behavior into mixin
A "Patch" and "Comment" share a common ancestor: emails. As a result
of this many fields are similar between the two. Rather than
duplicating code, pull the similar code out of the aforementioned
classes and into a Mixin.
This has the side effect of removing restrictions on Comment.content,
but this is probably OK: it's possible that someone could send an
empty mail in reply to a patch and we should show that.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Reviewed-by: Andy Doan <andy.doan@linaro.org>
At the moment a patch is split into two model entries: a Patch and a
linked Comment. The original rationale for this was that a Patch is
really a sub-class of Comment. A comment is a record of the text
content of an incoming mail, while a patch is that, plus the patch
content too. Hence the separation and a one-to-one relationship when a
patch is present. However, this decision was made before Django added
support for model inheritance and is no longer necessary. This change
flatten the models in preparation for some email subclassing work. This
is achieved by copying over the non-duplicated fields from the Comment
to the linked Patch, then deleting the Comment.
The migrations are broken into two steps: a schema migration and a data
migration, per the recommendations of the Django documentation [1]. SQL
migration scripts, where necessary, will need to be created manually as
there appears to be no way to do this in a way that is
RDBMS-independant [2][3].
Stephen Finucane [Fri, 11 Mar 2016 18:31:01 +0000 (18:31 +0000)]
fields: Simplify HashField
The HashField was written to be configurable. It supports customisable
hash types and provides fallbacks for a missing 'hashlib', which was
only introduced in Python 2.5. However, the customisable hash types
are not used anywhere (the actual hashing is hardcoded to use 'sha1')
and Python 2.7/3.3+ are the only versions of Python currently
supported. As a result, it is possible to remove much of the code
without losing any real functionality. Do this.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Reviewed-by: Andy Doan <andy.doan@linaro.org>
Stephen Finucane [Tue, 15 Mar 2016 10:57:06 +0000 (10:57 +0000)]
requirements: Loosen testing requirement versions
There's not really any reason to pin the requirements for dependencies
only used in tests, so don't do it. Replace these specific versions
with broader, major-version checks.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
This tool is exceptionally helpful for debugging issues with Django:
install it as part of the 'dev' configuration. This only works on
Django > 1.6, due to a lack of support for older versions in the
upstream.
A note is included to help users avoid the issues seen when running
patchwork on a different machine.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
Andy Doan [Thu, 25 Feb 2016 20:49:29 +0000 (14:49 -0600)]
xmlrpc: Add a check_create function
This changes adds the ability to create Check objects via the XMLRPC
interface. It includes a corresponding helper to the pwclient script.
The command can be used like:
Stephen Finucane [Thu, 18 Feb 2016 21:26:55 +0000 (21:26 +0000)]
views: Redirect when only one TODO exists
At the moment, the page listing all TODOs will display like the page
detailing a single TODO. This is misleading. Instead, use the same
approach as the index page, where the user is automatically redirected
to the appropriate project's page when only one project exists.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
Stephen Finucane [Thu, 18 Feb 2016 15:59:02 +0000 (15:59 +0000)]
tests: Remove check for 'tbody'
As part of 'f8cc68', the response for the patch list page changed
slightly. This means one of the things we were checking in a test is
no longer valid. Remove this check.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
The Django documentation suggests using environment variables as a way
to allow devs to commit settings files via source control without
including confidential information therein [1]. As such, change the
provided 'production' settings to do this. This should promote best
practices while also ensuring provisioning tools like
'ansible-django-stack' [2] work as expected.
The existing deployment could do with a little bit of fleshing out.
Do this, adding notes about the possibility of sidestepping much of
the process using deployment tools or PaaS solutions.
This also drops notes on deploying using MySQL/MariaDB or non-Ubuntu
distros. While it would be good to cover all use cases, it simply
isn't practical. Do one thing and do it well, providing notes for
users who want other options.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Suggested-by: Thomas Monjalon <thomas.monjalon@6wind.com>
Stephen Finucane [Wed, 10 Feb 2016 19:41:55 +0000 (19:41 +0000)]
docs: Combine MySQL init/reinit instructions
Apply some learnings for the Vagrant work and combine the commands used
to configure a MySQL database for first time use with those used to
reset an existing database. This should simplify installation.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
vagrant: Add Vagrant-based development environment
Integrate Vagrant by way of a Vagrantfile and a simple install script.
This follows many of the instructions found in the development guide
with the exceptions of the superuser creation, which is optional, the
loading of archives, which requires per-user authentication, and the
use of virtual environments, which are unnecessary in a teardown
environment such as this.
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
Stephen Finucane [Wed, 10 Feb 2016 19:58:47 +0000 (19:58 +0000)]
requirements: Remove psycopg2
It would be good to provide the required packages for devs using both
MySQL and PostgreSQL. However, there does not appear to be any way to
specify an exclusive-or condition in pip requirements files and the
existing behavior of including dependencies for both RDBMSs means
system packages have to be installed for both, when you are likely
only using one. This is particularly problematic when you want to
set up a fast dev environment using something like Vagrant.
Settle on MySQL for now, until such a time as the consensus is to go
all-in with PostgreSQL (including docs, for example).
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>
Damien Lespiau [Sun, 20 Dec 2015 03:53:53 +0000 (03:53 +0000)]
ui: Switch to HTML5 doctype and fix issues
A few common things:
- Remove the 'role' attribute of <nav>
- Don't use the self closing syntax ('/>') on non void elements
- Don't use the language attribution in <script>
- Remove spurious attributes
- The 'check' attribute of <input> doesn't take any value
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> Acked-by: Stephen Finucane <stephen.finucane@intel.com>
Damien Lespiau [Fri, 18 Dec 2015 17:34:29 +0000 (17:34 +0000)]
ui: Merge the TODO list into the user dropdown
We still leave the badge on the top bar so we can see, at a glance, the
number of items on the TODO list. The actual menu item is folded into
the dropdown.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> Acked-by: Stephen Finucane <stephen.finucane@intel.com>
Damien Lespiau [Mon, 14 Dec 2015 18:04:23 +0000 (18:04 +0000)]
ui: Let the bootstrap inverse navbar style shine through
Time for another iteration of the design. Let's start by using the
inverse bootsrap style with a slightly whiter color for font for better
contrast.
That color change has repercussions in the style sheet, mostly in the
custom containers/boxes we're still using on the secondary pages
(login/register/...)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> Acked-by: Stephen Finucane <stephen.finucane@intel.com>
I'm really not anything of a UX designer, nor a native speaker, but I
can try to make tools a bit less 'dry'. 'Number of patches' would have
worked as well, but opted for a shorter one.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> Acked-by: Stephen Finucane <stephen.finucane@intel.com>
Damien Lespiau [Mon, 1 Dec 2014 15:07:28 +0000 (15:07 +0000)]
ui: Move the background color of the table headers to a class
As we are using "sticky headers", the column headers stay apparent when
scrolling down, we need to specify a white background to hide what's
underneath.
With the addition of Seres, the patchlist table is not the only table we
want this behaviour on, so let's make it a CSS class rather then a
selector on the patchlist id.
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> Acked-by: Stephen Finucane <stephen.finucane@intel.com>