Re: [PATCH v1 3/9] parser: Parse "Depends-on" tags in emails
On Fri, 2024-07-12 at 15:28 -0400, Adam Hassick wrote: > On Fri, Jul 12, 2024 at 12:02 PM Stephen Finucane wrote: > > > > On Mon, 2024-06-17 at 18:18 -0400, Adam Hassick wrote: > > > Add a new function to parse "Depends-on" tags to the parser. The value > > > may either be a "reference" to a patch or series object or the web URL > > > to the object. For example, a reference may look like "patch-1234" or > > > "series-5678". When this tag is found, the parser will add the series > > > (or the series the patch belongs to) as a dependency to the patch series > > > it is creating. > > > > I know the DPDK folks are using this integer ID-based format for > > dependencies > > (per [1]), but I'm not a huge fan of it. My concerns are two-fold: firstly, > > we've been trying to abstract/hide the integer IDs in favour of message IDs > > over > > recent releases and we no longer expose in most of the web UI (they're still > > exposed via the REST API but that's for historical reasons). This would be a > > step backwards on this path. Secondly, something like 'series-5678' gives > > the > > casual observer very little information about the location of that > > dependency. > > You'd need to know that Patchwork was being used as well as the specific > > location of the Patchwork instance in question. I wonder if, rather than > > supporting this syntax, we could support Message IDs (and URLs) instead? > > That's > > a unique identifier that is searchable both online and off (assuming you're > > saving mail locally). DPDK could of course choose to add patches on top to > > support their existing syntax, though they could also choose to migrate to > > the > > new pattern. wdyt? > > Sure. We can use message IDs instead of the reference format we came > up with. I'll reach out to the DPDK community and see if they are > receptive to changing the procedure. My guess is they would prefer > that over running Patchwork with custom patches applied. Any updates on this? I'm assuming we're waiting on that before I can expect a v2? Or is the v2 gone elsewhere and I simply haven't spotted it? Stephen > > > Some other comments on this patch as-is below. > > > > [1] https://github.com/getpatchwork/git-pw/issues/71 > > > > > > > > Signed-off-by: Adam Hassick > > > --- > > > patchwork/parser.py | 109 +++- > > > 1 file changed, 108 insertions(+), 1 deletion(-) > > > > > > diff --git a/patchwork/parser.py b/patchwork/parser.py > > > index 09a53a0..90ec63b 100644 > > > --- a/patchwork/parser.py > > > +++ b/patchwork/parser.py > > > @@ -14,11 +14,14 @@ from email.errors import HeaderParseError > > > from fnmatch import fnmatch > > > import logging > > > import re > > > +from urllib.parse import urlparse > > > > > > from django.contrib.auth.models import User > > > from django.db.utils import IntegrityError > > > from django.db import transaction > > > from django.utils import timezone as tz_utils > > > +from django.urls import resolve, Resolver404 > > > +from django.conf import settings > > > > > > from patchwork.models import Cover > > > from patchwork.models import CoverComment > > > @@ -32,7 +35,6 @@ from patchwork.models import Series > > > from patchwork.models import SeriesReference > > > from patchwork.models import State > > > > > > - > > > _msgid_re = re.compile(r'<[^>]+>') > > > _hunk_re = re.compile(r'^\@\@ -\d+(?:,(\d+))? \+\d+(?:,(\d+))? \@\@') > > > _filename_re = re.compile(r'^(---|\+\+\+) (\S+)') > > > @@ -1054,6 +1056,102 @@ def parse_pull_request(content): > > > return None > > > > > > > > > +def find_series_from_url(url): > > > +""" > > > +Get series or patch from URL. > > > +""" > > > + > > > +parse_result = urlparse(url) > > > + > > > +# Resolve the URL path to see if this is a patch or series detail > > > URL. > > > +try: > > > +result = resolve(parse_result.path) > > > +except Resolver404: > > > +return None > > > + > > > +if result.view_name == 'patch-list' and parse_result.query: > > > +# Pars
Re: [PATCH v1 9/9] release-notes: Add release notes
On Mon, 2024-06-17 at 18:18 -0400, Adam Hassick wrote: > Signed-off-by: Adam Hassick Again, potentially needs updates based on the depends-on discussions. Also, there's a small typo. Address both and I'm good. > --- > ...-series-dependencies-6696458586e795c7.yaml | 20 +++ > 1 file changed, 20 insertions(+) > create mode 100644 > releasenotes/notes/add-series-dependencies-6696458586e795c7.yaml > > diff --git a/releasenotes/notes/add-series-dependencies-6696458586e795c7.yaml > b/releasenotes/notes/add-series-dependencies-6696458586e795c7.yaml > new file mode 100644 > index 000..ad7cfda > --- /dev/null > +++ b/releasenotes/notes/add-series-dependencies-6696458586e795c7.yaml > @@ -0,0 +1,20 @@ > +--- > +features: > + - | > +Series may now depend on eachother. Patchwork clients may use this s/eachother/each other/ > +information to download and apply the dependencies when applying a > series. > +This dependency relationship is shallow; the dependencies of a dependency > +should not be applied. Multiple dependencies are allowed. The order they > +are applied in is the order they appear in the content from top to > bottom. > + - | > +Patchwork will now look for "Depends-on" entries when parsing mails. > +This may be done by referring to a patch or series ID in the commit > message > +of a patch or the cover letter content: > +``Depends-on: patch-1234`` or ``Depends-on: series-5678`` > +Alternatively, the web URL of the patch or series may be given: > +``Depends-on: > http://patchwork.example.com/project/test/list?series=`` > +This feature is disabled by default, and may be enabled by adding > +``ENABLE_DEPENDS_ON_PARSING=True`` to the settings.py. > +api: > + - The API version has been updated to v1.4. > + - Add the "dependencies" and "dependents" fields to the series detail view. ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH v1 8/9] tests: Add tests for new functionality
On Mon, 2024-06-17 at 18:18 -0400, Adam Hassick wrote:
> * Adds tests for the API and parser changes.
> * Add new test mbox files.
> * Add new patch series tests.
>
> Signed-off-by: Adam Hassick
This is pretty thorough, nice work. It might need changes depending on the
outcome of the 'Depends-on' format discussion but otherwise LGTM.
Reviewed-by: Stephen Finucane
> ---
> patchwork/tests/api/test_series.py| 33 ++-
> .../tests/series/dependency-base-patch.mbox | 102 +++
> .../series/dependency-multi-2.mbox.template | 110
> .../series/dependency-multi.mbox.template | 109
> .../series/dependency-one-cover.mbox.template | 128 +
> .../dependency-one-first-patch.mbox.template | 125 +
> patchwork/tests/test_parser.py| 49
> patchwork/tests/test_series.py| 260 ++
> 8 files changed, 915 insertions(+), 1 deletion(-)
> create mode 100644 patchwork/tests/series/dependency-base-patch.mbox
> create mode 100644 patchwork/tests/series/dependency-multi-2.mbox.template
> create mode 100644 patchwork/tests/series/dependency-multi.mbox.template
> create mode 100644 patchwork/tests/series/dependency-one-cover.mbox.template
> create mode 100644
> patchwork/tests/series/dependency-one-first-patch.mbox.template
>
> diff --git a/patchwork/tests/api/test_series.py
> b/patchwork/tests/api/test_series.py
> index 730678a..86954b0 100644
> --- a/patchwork/tests/api/test_series.py
> +++ b/patchwork/tests/api/test_series.py
> @@ -44,6 +44,20 @@ class TestSeriesAPI(utils.APITestCase):
> self.assertIn(series_obj.get_mbox_url(), series_json['mbox'])
> self.assertIn(series_obj.get_absolute_url(), series_json['web_url'])
>
> +for dep, item in zip(
> +series_obj.dependencies.all(), series_json['dependencies']
> +):
> +self.assertIn(
> +reverse('api-series-detail', kwargs={'pk': dep.id}), item
> +)
> +
> +for dep, item in zip(
> +series_obj.dependents.all(), series_json['dependents']
> +):
> +self.assertIn(
> +reverse('api-series-detail', kwargs={'pk': dep.id}), item
> +)
> +
> # nested fields
>
> self.assertEqual(series_obj.project.id, series_json['project']['id'])
> @@ -95,6 +109,21 @@ class TestSeriesAPI(utils.APITestCase):
> series_rsp = resp.data[0]
> self.assertSerialized(series, series_rsp)
>
> +def test_dependencies(self):
> +project_obj = create_project(linkname='myproject')
> +person_obj = create_person(email='t...@example.com')
> +series1 = create_series(project=project_obj, submitter=person_obj)
> +create_cover(series=series1)
> +create_patch(series=series1)
> +series2 = create_series(project=project_obj, submitter=person_obj)
> +create_cover(series=series2)
> +create_patch(series=series2)
> +series1.add_dependencies([series2])
> +resp = self.client.get(self.api_url())
> +self.assertEqual(2, len(resp.data))
> +self.assertSerialized(series2, resp.data[1])
> +self.assertSerialized(series1, resp.data[0])
> +
> def test_list_filter_project(self):
> """Filter series by project."""
> series = self._create_series()
> @@ -152,7 +181,7 @@ class TestSeriesAPI(utils.APITestCase):
> create_cover(series=series_obj)
> create_patch(series=series_obj)
>
> -with self.assertNumQueries(6):
> +with self.assertNumQueries(8):
> self.client.get(self.api_url())
>
> @utils.store_samples('series-detail')
> @@ -175,6 +204,8 @@ class TestSeriesAPI(utils.APITestCase):
> self.assertNotIn('web_url', resp.data['cover_letter'])
> self.assertNotIn('mbox', resp.data['cover_letter'])
> self.assertNotIn('web_url', resp.data['patches'][0])
> +self.assertNotIn('dependents', resp.data)
> +self.assertNotIn('dependencies', resp.data)
>
> def test_detail_non_existent(self):
> """Ensure we get a 404 for a non-existent series."""
> diff --git a/patchwork/tests/series/dependency-base-patch.mbox
> b/patchwork/tests/series/dependency-base-patch.mbox
> new file mode 100644
> index 000..e7f9e94
> --- /dev/null
> +++ b/patchwork/tests/series/
Re: [PATCH v1 7/9] migrations: Add migration for new field
On Mon, 2024-06-17 at 18:18 -0400, Adam Hassick wrote:
> Signed-off-by: Adam Hassick
This needs to be squashed into the change that actually adds the fields to keep
things bisectable. One other nit below but change itself looks fine otherwise.
> ---
> .../migrations/0047_series_dependencies.py| 23 +++
> 1 file changed, 23 insertions(+)
> create mode 100644 patchwork/migrations/0047_series_dependencies.py
>
> diff --git a/patchwork/migrations/0047_series_dependencies.py
> b/patchwork/migrations/0047_series_dependencies.py
> new file mode 100644
> index 000..5abbcc3
> --- /dev/null
> +++ b/patchwork/migrations/0047_series_dependencies.py
> @@ -0,0 +1,23 @@
> +# Generated by Django 5.0.6 on 2024-06-07 02:58
Can you drop this? It's auto-generated and no real help. I need to clean them
out of other files but haven't gotten around to it yet.
> +
> +from django.db import migrations, models
> +
> +
> +class Migration(migrations.Migration):
> +dependencies = [
> +('patchwork', '0046_patch_comment_events'),
> +]
> +
> +operations = [
> +migrations.AddField(
> +model_name='series',
> +name='dependencies',
> +field=models.ManyToManyField(
> +blank=True,
> +help_text='Optional dependencies on this patch.',
> +related_name='dependents',
> +related_query_name='dependent',
> +to='patchwork.series',
> +),
> +),
> +]
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH v1 5/9] urls: Increment API version
On Mon, 2024-06-17 at 18:18 -0400, Adam Hassick wrote: > Add version 1.4 because new fields were added to the API. Can you move this to the front of the series and squash in the non-schema docs changes from the next patch into this one (we can keep the schema changes separate since they're so big)? Otherwise this LGTM. Reviewed-by: Stephen Finucane > --- > patchwork/urls.py | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/patchwork/urls.py b/patchwork/urls.py > index ecd3668..11cd8e7 100644 > --- a/patchwork/urls.py > +++ b/patchwork/urls.py > @@ -347,12 +347,16 @@ if settings.ENABLE_REST_API: > > urlpatterns += [ > re_path( > -r'^api/(?:(?P(1.0|1.1|1.2|1.3))/)?', > include(api_patterns) > +r'^api/(?:(?P(1.0|1.1|1.2|1.3|1.4))/)?', > +include(api_patterns), > ), > re_path( > -r'^api/(?:(?P(1.1|1.2|1.3))/)?', > include(api_1_1_patterns) > +r'^api/(?:(?P(1.1|1.2|1.3|1.4))/)?', > +include(api_1_1_patterns), > +), > +re_path( > +r'^api/(?:(?P(1.3|1.4))/)?', include(api_1_3_patterns) > ), > -re_path(r'^api/(?:(?P(1.3))/)?', include(api_1_3_patterns)), > # token change > path( > 'user/generate-token/', ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH v1 4/9] settings: Add flag to toggle "Depends-on" parsing
On Mon, 2024-06-17 at 18:18 -0400, Adam Hassick wrote: > Some operators of Patchwork instances may not want to allow series > dependencies. By providing this setting, these operators may disable > this behavior on their instance. > > Signed-off-by: Adam Hassick Not sure what happened here but this looks like a dupe. One to fix if we end up keeping this patch going into v2 (though I don't think we should). Stephen > --- > patchwork/settings/base.py | 3 +++ > patchwork/settings/dev.py | 2 ++ > 2 files changed, 5 insertions(+) > > diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py > index dccab6c..8b05d57 100644 > --- a/patchwork/settings/base.py > +++ b/patchwork/settings/base.py > @@ -263,6 +263,9 @@ ENABLE_XMLRPC = False > # Set to True to enable the Patchwork REST API > ENABLE_REST_API = True > > +# Set to True to enable parsing "Depends-on" tags. > +ENABLE_DEPENDS_ON_PARSING = False > + > REST_RESULTS_PER_PAGE = 30 > MAX_REST_RESULTS_PER_PAGE = 250 > > diff --git a/patchwork/settings/dev.py b/patchwork/settings/dev.py > index 75c4b34..3688b24 100644 > --- a/patchwork/settings/dev.py > +++ b/patchwork/settings/dev.py > @@ -80,3 +80,5 @@ if dbbackup: > ENABLE_XMLRPC = True > > ENABLE_REST_API = True > + > +ENABLE_DEPENDS_ON_PARSING = True ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH v1 4/9] settings: Add flag disable "Depends-on" parsing
On Mon, 2024-06-17 at 18:18 -0400, Adam Hassick wrote: > Some operators of Patchwork instances may not want to allow series > dependencies. By providing this setting, these operators may disable > this behavior on their instance. > > Signed-off-by: Adam Hassick This is only one of three feature flags - ENABLE_XMLRPC_API and ENABLE_REST_API being the other two. We actually plan on dropping the latter of these at some point since we consider the REST API to be mature at this point, and the only reason it ever existed was on off-chance that we introduced a serious security or performance issue. I don't think we need to worry about either here so I'm wondering if we need that flag. Are there any operators that you're aware of that have strong feelings about the feature? If not, can we drop this flag and default to always parsing dependencies? Stephen > --- > patchwork/settings/base.py | 3 +++ > patchwork/settings/dev.py | 2 ++ > 2 files changed, 5 insertions(+) > > diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py > index dccab6c..8b05d57 100644 > --- a/patchwork/settings/base.py > +++ b/patchwork/settings/base.py > @@ -263,6 +263,9 @@ ENABLE_XMLRPC = False > # Set to True to enable the Patchwork REST API > ENABLE_REST_API = True > > +# Set to True to enable parsing "Depends-on" tags. > +ENABLE_DEPENDS_ON_PARSING = False > + > REST_RESULTS_PER_PAGE = 30 > MAX_REST_RESULTS_PER_PAGE = 250 > > diff --git a/patchwork/settings/dev.py b/patchwork/settings/dev.py > index 75c4b34..3688b24 100644 > --- a/patchwork/settings/dev.py > +++ b/patchwork/settings/dev.py > @@ -80,3 +80,5 @@ if dbbackup: > ENABLE_XMLRPC = True > > ENABLE_REST_API = True > + > +ENABLE_DEPENDS_ON_PARSING = True ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH v1 3/9] parser: Parse "Depends-on" tags in emails
On Mon, 2024-06-17 at 18:18 -0400, Adam Hassick wrote:
> Add a new function to parse "Depends-on" tags to the parser. The value
> may either be a "reference" to a patch or series object or the web URL
> to the object. For example, a reference may look like "patch-1234" or
> "series-5678". When this tag is found, the parser will add the series
> (or the series the patch belongs to) as a dependency to the patch series
> it is creating.
I know the DPDK folks are using this integer ID-based format for dependencies
(per [1]), but I'm not a huge fan of it. My concerns are two-fold: firstly,
we've been trying to abstract/hide the integer IDs in favour of message IDs over
recent releases and we no longer expose in most of the web UI (they're still
exposed via the REST API but that's for historical reasons). This would be a
step backwards on this path. Secondly, something like 'series-5678' gives the
casual observer very little information about the location of that dependency.
You'd need to know that Patchwork was being used as well as the specific
location of the Patchwork instance in question. I wonder if, rather than
supporting this syntax, we could support Message IDs (and URLs) instead? That's
a unique identifier that is searchable both online and off (assuming you're
saving mail locally). DPDK could of course choose to add patches on top to
support their existing syntax, though they could also choose to migrate to the
new pattern. wdyt?
Some other comments on this patch as-is below.
[1] https://github.com/getpatchwork/git-pw/issues/71
>
> Signed-off-by: Adam Hassick
> ---
> patchwork/parser.py | 109 +++-
> 1 file changed, 108 insertions(+), 1 deletion(-)
>
> diff --git a/patchwork/parser.py b/patchwork/parser.py
> index 09a53a0..90ec63b 100644
> --- a/patchwork/parser.py
> +++ b/patchwork/parser.py
> @@ -14,11 +14,14 @@ from email.errors import HeaderParseError
> from fnmatch import fnmatch
> import logging
> import re
> +from urllib.parse import urlparse
>
> from django.contrib.auth.models import User
> from django.db.utils import IntegrityError
> from django.db import transaction
> from django.utils import timezone as tz_utils
> +from django.urls import resolve, Resolver404
> +from django.conf import settings
>
> from patchwork.models import Cover
> from patchwork.models import CoverComment
> @@ -32,7 +35,6 @@ from patchwork.models import Series
> from patchwork.models import SeriesReference
> from patchwork.models import State
>
> -
> _msgid_re = re.compile(r'<[^>]+>')
> _hunk_re = re.compile(r'^\@\@ -\d+(?:,(\d+))? \+\d+(?:,(\d+))? \@\@')
> _filename_re = re.compile(r'^(---|\+\+\+) (\S+)')
> @@ -1054,6 +1056,102 @@ def parse_pull_request(content):
> return None
>
>
> +def find_series_from_url(url):
> +"""
> +Get series or patch from URL.
> +"""
> +
> +parse_result = urlparse(url)
> +
> +# Resolve the URL path to see if this is a patch or series detail URL.
> +try:
> +result = resolve(parse_result.path)
> +except Resolver404:
> +return None
> +
> +if result.view_name == 'patch-list' and parse_result.query:
> +# Parse the query string.
> +# This can be replaced with something much friendlier once the
> +# series detail view is implemented.
> +series_query_param = next(
> +filter(
> +lambda x: len(x) == 2 and x[0] == 'series',
> +map(lambda x: x.split('='), parse_result.query.split('&')),
> +)
> +)
> +
> +if series_query_param:
nit:
if not series_query_param:
return None
series_id = series_query_param[1]
...
> +series_id = series_query_param[1]
> +
> +try:
> +series_id_num = int(series_id)
> +except ValueError:
> +return None
> +
> +return Series.objects.filter(id=series_id_num).first()
> +elif result.view_name == 'patch-detail':
> +msgid = Patch.decode_msgid(result.kwargs['msgid'])
> +patch = Patch.objects.filter(msgid=msgid).first()
> +
> +if patch:
> +return patch.series
> +
> +
> +def find_series_from_ref(match):
> +(obj_type, obj_id_str) = match
nit: don't need those extra brackets
> +
> +try:
> +object_id = int(obj_id_str)
> +except ValueError:
> +return None
> +
> +if obj_type == 'series':
> +series = Series.objects.filter(id=object_id).first()
nit:
return Series.objects.filter(...)
> +elif obj_type == 'patch':
> +patch = Patch.objects.filter(id=object_id).first()
> +
> +if not patch:
> +return None
> +
> +series = patch.series
nit:
return patch.series
> +
> +return series
> +
> +
> +def parse_depends_on(content):
> +"""Parses any dependency hints in the comments."""
> +depends_patterns = [
> +(
> +re.compi
Re: [PATCH v1 2/9] models: Add field for series dependencies
On Mon, 2024-06-17 at 18:18 -0400, Adam Hassick wrote: > Add a ManyToMany field to represent a dependency relationship between > patch series. > > Signed-off-by: Adam Hassick Two comments below but LGTM. Reviewed-by: Stephen Finucane > --- > patchwork/models.py | 17 + > 1 file changed, 17 insertions(+) > > diff --git a/patchwork/models.py b/patchwork/models.py > index 9a619bc..6f6a32d 100644 > --- a/patchwork/models.py > +++ b/patchwork/models.py > @@ -840,6 +840,16 @@ class Series(FilenameMixin, models.Model): > Cover, related_name='series', null=True, on_delete=models.CASCADE > ) > > +# dependencies > +dependencies = models.ManyToManyField( > +'self', > +symmetrical=False, > +blank=True, > +help_text='Optional dependencies on this patch.', > +related_name='dependents', > +related_query_name='dependent', > +) > + > # metadata > name = models.CharField( > max_length=255, > @@ -880,6 +890,13 @@ class Series(FilenameMixin, models.Model): > def received_all(self): > return self.total <= self.received_total > > +def add_dependencies(self, dependencies): > +# for dependency in dependencies: > +# dependency.dependents.add(self) > +# dependency.save() I assume this is from testing and should be dropped? > +self.dependencies.add(*dependencies) I have questions around validating this to prevent e.g. linking to self, but that's for later. > +self.save() > + > def add_cover_letter(self, cover): > """Add a cover letter to the series. > ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH v1 1/9] api: Add fields to series detail view
On Mon, 2024-06-17 at 18:18 -0400, Adam Hassick wrote:
> Adds the "dependencies" and "dependents" fields to the series detail
> view.
>
> Signed-off-by: Adam Hassick
This looks like it should be [2/N] in the series since it can't work until the
patch is merged. I also haven't assessed the performance of this yet but that's
DB-specific. From a pure API perspective though, this looks good so far.
Reviewed-by: Stephen Finucane
> ---
> patchwork/api/series.py | 26 --
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/patchwork/api/series.py b/patchwork/api/series.py
> index b88ed1f..2b23e48 100644
> --- a/patchwork/api/series.py
> +++ b/patchwork/api/series.py
> @@ -5,7 +5,10 @@
>
> from rest_framework.generics import ListAPIView
> from rest_framework.generics import RetrieveAPIView
> -from rest_framework.serializers import SerializerMethodField
> +from rest_framework.serializers import (
> +SerializerMethodField,
> +HyperlinkedRelatedField,
> +)
>
> from patchwork.api.base import BaseHyperlinkedModelSerializer
> from patchwork.api.base import PatchworkPermission
> @@ -24,6 +27,12 @@ class SeriesSerializer(BaseHyperlinkedModelSerializer):
> mbox = SerializerMethodField()
> cover_letter = CoverSerializer(read_only=True)
> patches = PatchSerializer(read_only=True, many=True)
> +dependencies = HyperlinkedRelatedField(
> +read_only=True, view_name='api-series-detail', many=True
> +)
> +dependents = HyperlinkedRelatedField(
> +read_only=True, view_name='api-series-detail', many=True
> +)
>
> def get_web_url(self, instance):
> request = self.context.get('request')
> @@ -50,6 +59,8 @@ class SeriesSerializer(BaseHyperlinkedModelSerializer):
> 'mbox',
> 'cover_letter',
> 'patches',
> +'dependencies',
> +'dependents',
> )
> read_only_fields = (
> 'date',
> @@ -60,9 +71,15 @@ class SeriesSerializer(BaseHyperlinkedModelSerializer):
> 'mbox',
> 'cover_letter',
> 'patches',
> +'dependencies',
> +'dependents',
> )
> versioned_fields = {
> '1.1': ('web_url',),
> +'1.4': (
> +'dependencies',
> +'dependents',
> +),
> }
> extra_kwargs = {
> 'url': {'view_name': 'api-series-detail'},
> @@ -76,7 +93,12 @@ class SeriesMixin(object):
> def get_queryset(self):
> return (
> Series.objects.all()
> -.prefetch_related('patches__project', 'cover_letter__project')
> +.prefetch_related(
> +'patches__project',
> +'cover_letter__project',
> +'dependencies',
> +'dependents',
> +)
> .select_related('submitter', 'project')
> )
>
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH] models: Add covering index for Patch.hash
Signed-off-by: Stephen Finucane
Closes: #579
Cc: Mauro Carvalho Chehab
---
.../migrations/0047_add_database_indexes.py| 18 ++
patchwork/models.py| 2 +-
2 files changed, 19 insertions(+), 1 deletion(-)
create mode 100644 patchwork/migrations/0047_add_database_indexes.py
diff --git a/patchwork/migrations/0047_add_database_indexes.py
b/patchwork/migrations/0047_add_database_indexes.py
new file mode 100644
index ..42c11997
--- /dev/null
+++ b/patchwork/migrations/0047_add_database_indexes.py
@@ -0,0 +1,18 @@
+import patchwork.fields
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+dependencies = [
+('patchwork', '0046_patch_comment_events'),
+]
+
+operations = [
+migrations.AlterField(
+model_name='patch',
+name='hash',
+field=patchwork.fields.HashField(
+blank=True, db_index=True, max_length=40, null=True
+),
+),
+]
diff --git a/patchwork/models.py b/patchwork/models.py
index 9a619bc5..38b52b05 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -501,7 +501,7 @@ class Patch(SubmissionMixin):
)
state = models.ForeignKey(State, null=True, on_delete=models.CASCADE)
archived = models.BooleanField(default=False)
-hash = HashField(null=True, blank=True)
+hash = HashField(null=True, blank=True, db_index=True)
# series metadata
--
2.43.0
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
Re: [Patchwork-maintainers] patchwork.ozlabs.org downtime for maintenance - 15/16 August
On Mon, 2024-01-22 at 15:52 +0800, Jeremy Kerr wrote:
> Hi all,
> > I'll try and get to that over the weekend.
>
> Looks like the heaviest database load is due to API request to the
> global patches view, which is a bit of an odd use-case; that all
> appears to be mostly spider traffic.
Just as an aside, many if not all of the performance issues related to this API
in particular should be resolved in the 3.0 release, owing to the removal of the
Submission table. The DPDK folks are running 3.x in production for a couple of
months now (at https://patches.dpdk.org/) and I'm only aware of one minor issue
[1] that they've encountered. Could be worth lining up the upgrade at some
point...
Cheers,
Stephen
PS: URLs API v2.0 will be almost entirely project-oriented (e.g.
'/project/{projectID}/patches'), but I haven't got there yet.
[1] https://github.com/getpatchwork/patchwork/issues/556
>
> Konstantin: I'm not sure your new index would help in that case, we're
> not looking up delegates for those views.
>
> Looking through the access logs, there seem to be three clients that
> are causing around 40-50% of patchwork load:
>
> - one IP from an "Alibaba Cloud HK" AS, various UAs
> - one IP from a Red Hat AS, curl/7.61.1 UA
> - the Bytedance "Bytespider" UA
>
> All three seem to be scraping the patchwork site.
>
> I have blocked all three for now, but it would be worthwhile setting up
> a more fair robots.txt and/or a reasonable ratelimit for the latter
> case.
>
> If anyone knows what might be up with that Red Hat crawler, please get
> in touch with me.
>
> I'll keep an eye on things here; there's still likely a bunch of
> potential configuration optimisation we can do too. Let me know if your
> observations change though.
>
> Cheers,
>
>
> Jeremy
> ___
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH] Correctly append tags on patches without commit details
Only a commit summary (a.k.a. patch subject) is necessary in Git: we
don't need details. The regex we were using to search for postscripts
however assumed that there would be _something_ before the postscript,
resulting in a newline. This wasn't the case. Correct this assumption by
instead using 're.MULTILINE' and matching on '^' and '$' for newlines
instead of '\n'.
Signed-off-by: Stephen Finucane
Closes: #516
Cc: siddh...@gotplt.org
---
patchwork/tests/views/test_utils.py | 23 +++
patchwork/views/utils.py| 4 ++--
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git patchwork/tests/views/test_utils.py
patchwork/tests/views/test_utils.py
index 6b980f9d..8b795815 100644
--- patchwork/tests/views/test_utils.py
+++ patchwork/tests/views/test_utils.py
@@ -66,6 +66,29 @@ class MboxPatchResponseTest(TestCase):
mbox = utils.patch_to_mbox(self.patch)
self.assertIn('Acked-by: 1\nAcked-by: 2\n', mbox)
+def test_bug_516(self):
+"""Test that tags are appended if a patch description is unset."""
+patch = create_patch(
+content=(
+'---\n'
+' manual/string.texi | 2 +-\n'
+' 1 file changed, 1 insertion(+), 1 deletion(-)'
+),
+)
+create_patch_comment(patch=patch, content='Acked-by: 2\n')
+
+mbox = utils.patch_to_mbox(patch)
+# the epilog comes after the tags
+self.assertIn(
+(
+'Acked-by: 2\n'
+'---\n'
+' manual/string.texi | 2 +-\n'
+' 1 file changed, 1 insertion(+), 1 deletion(-)\n'
+),
+mbox,
+)
+
def _test_header_passthrough(self, header):
patch = create_patch(headers=header + '\n')
mbox = utils.patch_to_mbox(patch)
diff --git patchwork/views/utils.py patchwork/views/utils.py
index 1f7ee0da..91b2ef1b 100644
--- patchwork/views/utils.py
+++ patchwork/views/utils.py
@@ -48,7 +48,7 @@ def _submission_to_mbox(submission):
"""
is_patch = isinstance(submission, Patch)
-postscript_re = re.compile('\n-{2,3} ?\n')
+postscript_re = re.compile('^-{2,3} ?$', re.MULTILINE)
body = ''
if submission.content:
@@ -71,7 +71,7 @@ def _submission_to_mbox(submission):
body += comment.patch_responses
if postscript:
-body += '---\n' + postscript + '\n'
+body += '---' + postscript + '\n'
if is_patch and submission.diff:
body += '\n' + submission.diff
--
2.39.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
Patchwork v3.1.1 Available
We're pleased to announce the release of Patchwork v3.1.1. This release is part of the "Hessian" release series: https://github.com/getpatchwork/patchwork/releases/tag/v3.1.1 This release is a PATCH release that focuses on bugfixes. For more details, please see below. Happy patchworking! --- Changes in Patchwork v3.1.0..v3.1.1 --- c1f897a4 Release 3.1.1 04b7b671 tests: Change from expectedFailure to skip a2f322dc REST: De-duplicate handling of nested resource URLs 27153773 REST: Fix issues with comment-related events 40bf7ca6 manage: Check Django version on startup 106242ab urls: Encode slashes in message IDs 8a0031bf trivial: Fix style issues b89ba00e docs: Actually configure reno to use the main branch 13f86fb0 Replace references to master with main ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Patchwork v3.0.6 Available
We're pleased to announce the release of Patchwork v3.0.6. This release is part of the "Grosgrain" release series: https://github.com/getpatchwork/patchwork/releases/tag/v3.0.6 This release is a PATCH release that focuses on bugfixes. For more details, please see below. Happy patchworking! --- Changes in Patchwork v3.0.5..v3.0.6 --- 297399bc Release 3.0.6 c51425bc docs: Actually configure reno to use the main branch 948c51ce Replace references to master with main 78e257db parser: Handle binary git patches f4c05031 [StableOnly] Fix docs job 12cd2a89 lib: fix table names f8cbc778 Post-release version bump ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 01/10] tests: Change from expectedFailure to skip
On Fri, 2022-09-30 at 17:19 +0100, Stephen Finucane wrote: > Python 3.10 recognises unexpected passes as failures now. > > Signed-off-by: Stephen Finucane I added some release notes and merged this whole series. Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 10/10] urls: Encode slashes in message IDs
On Fri, 2022-09-30 at 17:19 +0100, Stephen Finucane wrote: > We were attempting to work around the fact that message IDs could > contain slashes which in some cases broke our ability to generate > meaningful URLs. Rather than doing this, insist that users encode these > slashes so that we can distinguish between semantically meaningful > slashes and those that form the URL. This is a slightly breaking change, > but the current behavior is already broken (see the linked bug) so this > seems reasonable. > > Signed-off-by: Stephen Finucane > Closes: #433 > Cc: d...@axtens.net > Cc: siddh...@gotplt.org > Whoops. This was already sent and shouldn't have been included in the series. Apologies for the noise. Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH 10/10] urls: Encode slashes in message IDs
We were attempting to work around the fact that message IDs could
contain slashes which in some cases broke our ability to generate
meaningful URLs. Rather than doing this, insist that users encode these
slashes so that we can distinguish between semantically meaningful
slashes and those that form the URL. This is a slightly breaking change,
but the current behavior is already broken (see the linked bug) so this
seems reasonable.
Signed-off-by: Stephen Finucane
Closes: #433
Cc: d...@axtens.net
Cc: siddh...@gotplt.org
---
notes/issue-433-5f048abbe3789556.yaml | 19 +++
patchwork/models.py | 23 ++-
.../patchwork/partials/download-buttons.html | 6 ++---
.../patchwork/partials/patch-list.html| 2 +-
patchwork/templates/patchwork/submission.html | 8 +++
patchwork/tests/api/test_cover.py | 2 +-
patchwork/tests/api/test_patch.py | 2 +-
patchwork/tests/views/test_bundles.py | 8 +++
patchwork/tests/views/test_cover.py | 10
patchwork/tests/views/test_patch.py | 22 +-
patchwork/urls.py | 20 +---
patchwork/views/comment.py| 4 ++--
patchwork/views/cover.py | 4 ++--
patchwork/views/patch.py | 8 +++
14 files changed, 80 insertions(+), 58 deletions(-)
create mode 100644 notes/issue-433-5f048abbe3789556.yaml
diff --git notes/issue-433-5f048abbe3789556.yaml
notes/issue-433-5f048abbe3789556.yaml
new file mode 100644
index ..1d0c1553
--- /dev/null
+++ notes/issue-433-5f048abbe3789556.yaml
@@ -0,0 +1,19 @@
+---
+fixes:
+ - |
+Message IDs containing slashes will now have these slashes percent-encoded.
+Previously, attempts to access submissions whose Message IDs contained
+slashes would result in a HTTP 404 on some Django versions. If you wish to
+access such a submission, you must now percent-encode the slashes first.
+For example, to access a patch, cover letter or comment with the following
+message ID:
+
+ bug-28101-10460-nydynmf...@http.sourceware.org/bugzilla/
+
+You should now use:
+
+ bug-28101-10460-nydynmf...@http.sourceware.org%2Dbugzilla%2D
+
+Both the web UI and REST API have been updated to generate URLs in this
+format so this should only be noticable to users manually generating such
+URLs.
diff --git patchwork/models.py patchwork/models.py
index d2507d4f..20ec9f06 100644
--- patchwork/models.py
+++ patchwork/models.py
@@ -369,12 +369,24 @@ class EmailMixin(models.Model):
@property
def url_msgid(self):
-"""A trimmed messageid, suitable for inclusion in URLs"""
+"""A trimmed Message ID, suitable for inclusion in URLs"""
if settings.DEBUG:
assert self.msgid[0] == '<' and self.msgid[-1] == '>'
return self.msgid.strip('<>')
+@property
+def encoded_msgid(self):
+"""Like 'url_msgid' but with slashes percentage encoded."""
+# We don't want to encode all characters (i.e. use urllib.parse.quote)
+# because that would result in us encoding the '@' present in all
+# message IDs. Instead we only percent-encode any slashes present [1].
+# These are not common so this is very much expected to be an edge
+# case.
+#
+# [1] https://datatracker.ietf.org/doc/html/rfc3986.html#section-2
+return self.url_msgid.replace('/', '%2F')
+
def save(self, *args, **kwargs):
# Modifying a submission via admin interface changes '\n' newlines in
# message content to '\r\n'. We need to fix them to avoid problems,
@@ -436,7 +448,7 @@ class Cover(SubmissionMixin):
'cover-detail',
kwargs={
'project_id': self.project.linkname,
-'msgid': self.url_msgid,
+'msgid': self.encoded_msgid,
},
)
@@ -445,7 +457,7 @@ class Cover(SubmissionMixin):
'cover-mbox',
kwargs={
'project_id': self.project.linkname,
-'msgid': self.url_msgid,
+'msgid': self.encoded_msgid,
},
)
@@ -671,7 +683,7 @@ class Patch(SubmissionMixin):
'patch-detail',
kwargs={
'project_id': self.project.linkname,
-'msgid': self.url_msgid,
+'msgid': self.encoded_msgid,
},
)
@@ -680,7 +692,7 @@ class Patch(SubmissionMixin):
'patch-mbox',
kwargs={
[PATCH 09/10] REST: Add missing 'url' parameter for comments
This should be present on all resources.
Signed-off-by: Stephen Finucane
Fixes: 88f56051 ("api: add comments detail endpoint")
---
docs/api/schemas/latest/patchwork.yaml | 5 +
docs/api/schemas/patchwork.j2 | 9 +
docs/api/schemas/v1.0/patchwork.yaml | 5 -
docs/api/schemas/v1.1/patchwork.yaml | 5 -
docs/api/schemas/v1.2/patchwork.yaml | 5 -
docs/api/schemas/v1.3/patchwork.yaml | 5 +
patchwork/api/base.py | 14 ++
patchwork/api/bundle.py| 2 +-
patchwork/api/comment.py | 24 ++--
patchwork/api/embedded.py | 2 ++
patchwork/api/patch.py | 1 +
11 files changed, 51 insertions(+), 26 deletions(-)
diff --git docs/api/schemas/latest/patchwork.yaml
docs/api/schemas/latest/patchwork.yaml
index 3a1fdd3a..b3de0db5 100644
--- docs/api/schemas/latest/patchwork.yaml
+++ docs/api/schemas/latest/patchwork.yaml
@@ -1627,6 +1627,11 @@ components:
title: ID
type: integer
readOnly: true
+url:
+ title: URL
+ type: string
+ format: uri
+ readOnly: true
web_url:
title: Web URL
type: string
diff --git docs/api/schemas/patchwork.j2 docs/api/schemas/patchwork.j2
index b9786654..68655348 100644
--- docs/api/schemas/patchwork.j2
+++ docs/api/schemas/patchwork.j2
@@ -1683,6 +1683,13 @@ components:
title: ID
type: integer
readOnly: true
+{% if version >= (1, 3) %}
+url:
+ title: URL
+ type: string
+ format: uri
+ readOnly: true
+{% endif %}
{% if version >= (1, 1) %}
web_url:
title: Web URL
@@ -2528,11 +2535,13 @@ components:
title: ID
type: integer
readOnly: true
+{% if version >= (1, 3) %}
url:
title: URL
type: string
format: uri
readOnly: true
+{% endif %}
{% if version >= (1, 1) %}
web_url:
title: Web URL
diff --git docs/api/schemas/v1.0/patchwork.yaml
docs/api/schemas/v1.0/patchwork.yaml
index 817b2f2a..6c3893ec 100644
--- docs/api/schemas/v1.0/patchwork.yaml
+++ docs/api/schemas/v1.0/patchwork.yaml
@@ -1993,11 +1993,6 @@ components:
title: ID
type: integer
readOnly: true
-url:
- title: URL
- type: string
- format: uri
- readOnly: true
msgid:
title: Message ID
type: string
diff --git docs/api/schemas/v1.1/patchwork.yaml
docs/api/schemas/v1.1/patchwork.yaml
index 574a8ad8..7e2299c5 100644
--- docs/api/schemas/v1.1/patchwork.yaml
+++ docs/api/schemas/v1.1/patchwork.yaml
@@ -2044,11 +2044,6 @@ components:
title: ID
type: integer
readOnly: true
-url:
- title: URL
- type: string
- format: uri
- readOnly: true
web_url:
title: Web URL
type: string
diff --git docs/api/schemas/v1.2/patchwork.yaml
docs/api/schemas/v1.2/patchwork.yaml
index 7a4e8e8e..93c3e97e 100644
--- docs/api/schemas/v1.2/patchwork.yaml
+++ docs/api/schemas/v1.2/patchwork.yaml
@@ -2287,11 +2287,6 @@ components:
title: ID
type: integer
readOnly: true
-url:
- title: URL
- type: string
- format: uri
- readOnly: true
web_url:
title: Web URL
type: string
diff --git docs/api/schemas/v1.3/patchwork.yaml
docs/api/schemas/v1.3/patchwork.yaml
index 6bd0419d..8663406d 100644
--- docs/api/schemas/v1.3/patchwork.yaml
+++ docs/api/schemas/v1.3/patchwork.yaml
@@ -1627,6 +1627,11 @@ components:
title: ID
type: integer
readOnly: true
+url:
+ title: URL
+ type: string
+ format: uri
+ readOnly: true
web_url:
title: Web URL
type: string
diff --git patchwork/api/base.py patchwork/api/base.py
index 0f5c44a2..16e5cb8d 100644
--- patchwork/api/base.py
+++ patchwork/api/base.py
@@ -151,19 +151,17 @@ class
NestedHyperlinkedIdentityField(HyperlinkedIdentityField):
class BaseHyperlinkedModelSerializer(HyperlinkedModelSerializer):
def to_representation(self, instance):
-data = super(BaseHyperlinkedModelSerializer, self).to_representation(
-instance
-)
-
request = self.context.get('request')
for version in getattr(self.Meta, 'versioned_fields', {}):
# if the user has requested a version lower that than in which the
# field was added, we drop it
if not utils.has_version(request, version):
for field in self.Meta.versioned_fields[version]:
-# After a PATCH with an older API version, we may not see
-# thes
[PATCH 06/10] REST: Fix issues with comment-related events
When we introduced this functionality, we missed the fact that these
resources use nested-style URLs that need to be specially handled. Fix
this now.
Signed-off-by: Stephen Finucane
Fixes: e3d8f7548 ("REST: Add 'patch-comment-created', 'cover-comment-created'
events")
Cc: Siddhesh Poyarekar
Cc: DJ Delorie
Cc: Carlos O'Donell
---
patchwork/api/base.py | 34 +++
patchwork/api/embedded.py | 10 +++--
patchwork/api/event.py| 25 ---
patchwork/tests/api/test_event.py | 30 +--
4 files changed, 83 insertions(+), 16 deletions(-)
diff --git patchwork/api/base.py patchwork/api/base.py
index 6268f67d..3ed4182c 100644
--- patchwork/api/base.py
+++ patchwork/api/base.py
@@ -139,6 +139,40 @@ class
CheckHyperlinkedIdentityField(HyperlinkedIdentityField):
)
+class CoverCommentHyperlinkedIdentityField(HyperlinkedIdentityField):
+def get_url(self, obj, view_name, request, format):
+# Unsaved objects will not yet have a valid URL.
+if obj.pk is None:
+return None
+
+return self.reverse(
+view_name,
+kwargs={
+'cover_id': obj.cover.id,
+'comment_id': obj.id,
+},
+request=request,
+format=format,
+)
+
+
+class PatchCommentHyperlinkedIdentityField(HyperlinkedIdentityField):
+def get_url(self, obj, view_name, request, format):
+# Unsaved objects will not yet have a valid URL.
+if obj.pk is None:
+return None
+
+return self.reverse(
+view_name,
+kwargs={
+'patch_id': obj.patch.id,
+'comment_id': obj.id,
+},
+request=request,
+format=format,
+)
+
+
class BaseHyperlinkedModelSerializer(HyperlinkedModelSerializer):
def to_representation(self, instance):
data = super(BaseHyperlinkedModelSerializer, self).to_representation(
diff --git patchwork/api/embedded.py patchwork/api/embedded.py
index c41511fe..485ed6f7 100644
--- patchwork/api/embedded.py
+++ patchwork/api/embedded.py
@@ -17,6 +17,8 @@ from rest_framework.serializers import SerializerMethodField
from patchwork.api.base import BaseHyperlinkedModelSerializer
from patchwork.api.base import CheckHyperlinkedIdentityField
+from patchwork.api.base import CoverCommentHyperlinkedIdentityField
+from patchwork.api.base import PatchCommentHyperlinkedIdentityField
from patchwork import models
@@ -127,6 +129,9 @@ class CoverSerializer(SerializedRelatedField):
class CoverCommentSerializer(SerializedRelatedField):
class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer):
+
+url = CoverCommentHyperlinkedIdentityField('api-cover-comment-detail')
+
class Meta:
model = models.CoverComment
fields = (
@@ -136,7 +141,6 @@ class CoverCommentSerializer(SerializedRelatedField):
'msgid',
'list_archive_url',
'date',
-'name',
)
read_only_fields = fields
versioned_fields = {
@@ -177,6 +181,9 @@ class PatchSerializer(SerializedRelatedField):
class PatchCommentSerializer(SerializedRelatedField):
class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer):
+
+url = PatchCommentHyperlinkedIdentityField('api-patch-comment-detail')
+
class Meta:
model = models.PatchComment
fields = (
@@ -186,7 +193,6 @@ class PatchCommentSerializer(SerializedRelatedField):
'msgid',
'list_archive_url',
'date',
-'name',
)
read_only_fields = fields
versioned_fields = {
diff --git patchwork/api/event.py patchwork/api/event.py
index c1b09ab9..6d08b6ee 100644
--- patchwork/api/event.py
+++ patchwork/api/event.py
@@ -19,6 +19,7 @@ from patchwork.api.embedded import ProjectSerializer
from patchwork.api.embedded import SeriesSerializer
from patchwork.api.embedded import UserSerializer
from patchwork.api.filters import EventFilterSet
+from patchwork.api import utils
from patchwork.models import Event
@@ -140,7 +141,7 @@ class EventList(ListAPIView):
ordering = '-date'
def get_queryset(self):
-return Event.objects.all().prefetch_related(
+events = Event.objects.all().prefetch_related(
'project',
'patch__project',
'series__project',
@@ -150,6 +151,24 @@ class EventList(ListAPIView):
'previous_delegate',
'current_delegate',
'c
[PATCH 08/10] models: Cache 'list_archive_url' property
We really need to get rid of this from the embedded view. It's way too slow. For now, we just cache it and leave a note for future us. Signed-off-by: Stephen Finucane --- patchwork/api/embedded.py | 12 patchwork/models.py | 6 +++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git patchwork/api/embedded.py patchwork/api/embedded.py index 7105da08..4cfdf8e6 100644 --- patchwork/api/embedded.py +++ patchwork/api/embedded.py @@ -113,6 +113,8 @@ class CoverSerializer(SerializedRelatedField): 'url', 'web_url', 'msgid', +# TODO(stephenfin): Drop this in a future API version - it is +# too slow to calculate and not necessary here. 'list_archive_url', 'date', 'name', @@ -149,6 +151,8 @@ class CoverCommentSerializer(SerializedRelatedField): 'url', 'web_url', 'msgid', +# TODO(stephenfin): Drop this in a future API version - it is +# too slow to calculate and not necessary here. 'list_archive_url', 'date', ) @@ -174,6 +178,8 @@ class PatchSerializer(SerializedRelatedField): 'url', 'web_url', 'msgid', +# TODO(stephenfin): Drop this in a future API version - it is +# too slow to calculate and not necessary here. 'list_archive_url', 'date', 'name', @@ -207,6 +213,8 @@ class PatchCommentSerializer(SerializedRelatedField): 'url', 'web_url', 'msgid', +# TODO(stephenfin): Drop this in a future API version - it is +# too slow to calculate and not necessary here. 'list_archive_url', 'date', ) @@ -253,8 +261,12 @@ class ProjectSerializer(SerializedRelatedField): 'web_url', 'scm_url', 'webscm_url', +# TODO(stephenfin): Drop this in a future API version - it is +# too slow to calculate and not necessary here. 'list_archive_url', +# TODO(stephenfin): Ditto 'list_archive_url_format', +# TODO(stephenfin): Ditto 'commit_url_format', ) read_only_fields = fields diff --git patchwork/models.py patchwork/models.py index 264af532..d2507d4f 100644 --- patchwork/models.py +++ patchwork/models.py @@ -406,7 +406,7 @@ class SubmissionMixin(FilenameMixin, EmailMixin, models.Model): name = models.CharField(max_length=255) -@property +@cached_property def list_archive_url(self): if not self.project.list_archive_url_format: return None @@ -719,7 +719,7 @@ class CoverComment(EmailMixin, models.Model): ) addressed = models.BooleanField(null=True) -@property +@cached_property def list_archive_url(self): if not self.cover.project.list_archive_url_format: return None @@ -770,7 +770,7 @@ class PatchComment(EmailMixin, models.Model): ) addressed = models.BooleanField(null=True) -@property +@cached_property def list_archive_url(self): if not self.patch.project.list_archive_url_format: return None -- 2.37.3 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH 07/10] REST: De-duplicate handling of nested resource URLs
These were all doing the same thing. Make things more generic.
We also speed up test (inadvertently) by using the 'patch_id' attribute
of the 'Check' model rather than 'patch.id', thus avoiding the JOIN.
Signed-off-by: Stephen Finucane
---
patchwork/api/base.py | 52 +--
patchwork/api/check.py| 10 --
patchwork/api/embedded.py | 28 +
patchwork/tests/api/test_event.py | 2 +-
4 files changed, 45 insertions(+), 47 deletions(-)
diff --git patchwork/api/base.py patchwork/api/base.py
index 3ed4182c..0f5c44a2 100644
--- patchwork/api/base.py
+++ patchwork/api/base.py
@@ -9,8 +9,8 @@ from django.conf import settings
from django.shortcuts import get_object_or_404
from rest_framework import permissions
from rest_framework.pagination import PageNumberPagination
+from rest_framework.relations import HyperlinkedIdentityField
from rest_framework.response import Response
-from rest_framework.serializers import HyperlinkedIdentityField
from rest_framework.serializers import HyperlinkedModelSerializer
from rest_framework.utils.urls import replace_query_param
@@ -122,52 +122,28 @@ class MultipleFieldLookupMixin(object):
return get_object_or_404(queryset, **filter_kwargs)
-class CheckHyperlinkedIdentityField(HyperlinkedIdentityField):
-def get_url(self, obj, view_name, request, format):
-# Unsaved objects will not yet have a valid URL.
-if obj.pk is None:
-return None
-
-return self.reverse(
-view_name,
-kwargs={
-'patch_id': obj.patch.id,
-'check_id': obj.id,
-},
-request=request,
-format=format,
-)
+class NestedHyperlinkedIdentityField(HyperlinkedIdentityField):
+"""A variant of HyperlinkedIdentityField that supports nested resources."""
+def __init__(self, view_name, lookup_field_mapping, **kwargs):
+self.lookup_field_mapping = lookup_field_mapping
+super().__init__(view_name, **kwargs)
-class CoverCommentHyperlinkedIdentityField(HyperlinkedIdentityField):
def get_url(self, obj, view_name, request, format):
# Unsaved objects will not yet have a valid URL.
-if obj.pk is None:
+if hasattr(obj, 'pk') and obj.pk in (None, ''):
return None
-return self.reverse(
-view_name,
-kwargs={
-'cover_id': obj.cover.id,
-'comment_id': obj.id,
-},
-request=request,
-format=format,
-)
-
-
-class PatchCommentHyperlinkedIdentityField(HyperlinkedIdentityField):
-def get_url(self, obj, view_name, request, format):
-# Unsaved objects will not yet have a valid URL.
-if obj.pk is None:
-return None
+kwargs = {}
+for (
+lookup_url_kwarg,
+lookup_field,
+) in self.lookup_field_mapping.items():
+kwargs[lookup_url_kwarg] = getattr(obj, lookup_field)
return self.reverse(
view_name,
-kwargs={
-'patch_id': obj.patch.id,
-'comment_id': obj.id,
-},
+kwargs=kwargs,
request=request,
format=format,
)
diff --git patchwork/api/check.py patchwork/api/check.py
index c28d89f7..f5461fc6 100644
--- patchwork/api/check.py
+++ patchwork/api/check.py
@@ -14,8 +14,8 @@ from rest_framework.serializers import HiddenField
from rest_framework.serializers import HyperlinkedModelSerializer
from rest_framework.serializers import ValidationError
-from patchwork.api.base import CheckHyperlinkedIdentityField
from patchwork.api.base import MultipleFieldLookupMixin
+from patchwork.api.base import NestedHyperlinkedIdentityField
from patchwork.api.base import CurrentPatchDefault
from patchwork.api.embedded import UserSerializer
from patchwork.api.filters import CheckFilterSet
@@ -25,7 +25,13 @@ from patchwork.models import Patch
class CheckSerializer(HyperlinkedModelSerializer):
-url = CheckHyperlinkedIdentityField('api-check-detail')
+url = NestedHyperlinkedIdentityField(
+'api-check-detail',
+lookup_field_mapping={
+'patch_id': 'patch_id',
+'check_id': 'id',
+},
+)
patch = HiddenField(default=CurrentPatchDefault())
user = UserSerializer(default=CurrentUserDefault())
diff --git patchwork/api/embedded.py patchwork/api/embedded.py
index 485ed6f7..7105da08 100644
--- patchwork/api/embedded.py
+++ patchwork/api/embedded.py
@@ -16,9 +16,7 @@ from rest_framework.serializers import PrimaryKeyRelatedField
from rest_framework.serializers import SerializerMethodField
[PATCH 04/10] requirements: Bump Django to 3.2.x, djangorestframework to 4.16.0
There are two issues to be addressed:
RemovedInDjango50Warning: Passing response to assertFormError() is
deprecated. Use the form object directly:
RemovedInDjango50Warning: The "default.html" templates for forms and
formsets will be removed. These were proxies to the equivalent
"table.html" templates, but the new "div.html" templates will be the
default from Django 5.0. Transitional renderers are provided to allow
you to opt-in to the new output style now. See
https://docs.djangoproject.com/en/4.1/releases/4.1/ for more details
Nothing complicated in fixing either of these. For the former, we must
do as we're told and use the form object directly. For the latter, we
need to configure our own form renderer so we can continue using the
table form renderer for now.
Signed-off-by: Stephen Finucane
---
patchwork/forms.py| 8 ++
patchwork/settings/base.py| 2 +
patchwork/tests/views/test_mail.py| 91 +---
patchwork/tests/views/test_patch.py | 23 ++--
patchwork/tests/views/test_user.py| 102 ++
.../django-4-1-support-bcbe65a71d235b43.yaml | 5 +
tox.ini | 9 +-
7 files changed, 197 insertions(+), 43 deletions(-)
create mode 100644 releasenotes/notes/django-4-1-support-bcbe65a71d235b43.yaml
diff --git patchwork/forms.py patchwork/forms.py
index 84448586..1c5a29be 100644
--- patchwork/forms.py
+++ patchwork/forms.py
@@ -5,8 +5,10 @@
from django.contrib.auth.models import User
from django import forms
+from django.forms import renderers
from django.db.models import Q
from django.db.utils import ProgrammingError
+from django.template.backends import django as django_template_backend
from patchwork.models import Bundle
from patchwork.models import Patch
@@ -14,6 +16,12 @@ from patchwork.models import State
from patchwork.models import UserProfile
+class PatchworkTableRenderer(renderers.EngineMixin, renderers.BaseRenderer):
+backend = django_template_backend.DjangoTemplates
+form_template_name = 'django/forms/table.html'
+formset_template_name = 'django/forms/formsets/table.html'
+
+
class RegistrationForm(forms.Form):
first_name = forms.CharField(max_length=30, required=False)
last_name = forms.CharField(max_length=30, required=False)
diff --git patchwork/settings/base.py patchwork/settings/base.py
index 045f262f..965c949f 100644
--- patchwork/settings/base.py
+++ patchwork/settings/base.py
@@ -71,6 +71,8 @@ TEMPLATES = [
},
]
+FORM_RENDERER = 'patchwork.forms.PatchworkTableRenderer'
+
# TODO(stephenfin): Consider changing to BigAutoField when we drop support for
# Django < 3.2
DEFAULT_AUTO_FIELD = 'django.db.models.AutoField'
diff --git patchwork/tests/views/test_mail.py patchwork/tests/views/test_mail.py
index de9df3d2..ae0b2c38 100644
--- patchwork/tests/views/test_mail.py
+++ patchwork/tests/views/test_mail.py
@@ -5,6 +5,7 @@
import re
+import django
from django.core import mail
from django.test import TestCase
from django.urls import reverse
@@ -33,15 +34,37 @@ class MailSettingsTest(TestCase):
response = self.client.post(reverse('mail-settings'), {'email': ''})
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(response, 'patchwork/mail.html')
-self.assertFormError(
-response, 'form', 'email', 'This field is required.'
-)
+if django.VERSION >= (4, 1):
+self.assertFormError(
+response.context['form'],
+'email',
+'This field is required.',
+)
+else:
+self.assertFormError(
+response,
+'form',
+'email',
+'This field is required.',
+)
def test_post_invalid(self):
response = self.client.post(reverse('mail-settings'), {'email': 'foo'})
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(response, 'patchwork/mail.html')
-self.assertFormError(response, 'form', 'email', error_strings['email'])
+if django.VERSION >= (4, 1):
+self.assertFormError(
+response.context['form'],
+'email',
+error_strings['email'],
+)
+else:
+self.assertFormError(
+response,
+'form',
+'email',
+error_strings['email'],
+)
def test_post_optin(self):
email = 'f...@example.com&#x
[PATCH 05/10] manage: Check Django version on startup
This was recently reported as an issue. Add a simple check to ensure
people update their dependencies as expected.
Signed-off-by: Stephen Finucane
Cc: Siddhesh Poyarekar
Cc: DJ Delorie
Cc: Carlos O'Donell
---
manage.py | 5 +
1 file changed, 5 insertions(+)
diff --git manage.py manage.py
index 033c8ae4..e227481e 100755
--- manage.py
+++ manage.py
@@ -7,6 +7,11 @@ if __name__ == "__main__":
"DJANGO_SETTINGS_MODULE", "patchwork.settings.production"
)
+import django
+
+if django.VERSION < (3, 2):
+raise Exception('patchwork requires Django 3.2 or greater')
+
from django.core.management import execute_from_command_line
execute_from_command_line(sys.argv)
--
2.37.3
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH 03/10] tox: Output test times, more verbose output
Just a bit more useful for CI logs
Signed-off-by: Stephen Finucane
---
tox.ini | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git tox.ini tox.ini
index 0ab0ab84..aa434a72 100644
--- tox.ini
+++ tox.ini
@@ -24,7 +24,7 @@ passenv =
DATABASE_TYPE DATABASE_USER DATABASE_PASSWORD DATABASE_HOST
DATABASE_PORT DATABASE_NAME DJANGO_TEST_PROCESSES
commands =
-python {toxinidir}/manage.py test --noinput --parallel --
{posargs:patchwork}
+python {toxinidir}/manage.py test --noinput --parallel -v 2 --timing --
{posargs:patchwork}
[testenv:bashate]
deps = bashate
--
2.37.3
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH 01/10] tests: Change from expectedFailure to skip
Python 3.10 recognises unexpected passes as failures now.
Signed-off-by: Stephen Finucane
---
patchwork/tests/test_series.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git patchwork/tests/test_series.py patchwork/tests/test_series.py
index 8d8c4e14..d3e20e08 100644
--- patchwork/tests/test_series.py
+++ patchwork/tests/test_series.py
@@ -178,7 +178,7 @@ class BaseSeriesTest(_BaseTestCase):
self.assertSerialized(patches, [2])
self.assertSerialized(covers, [1])
-@unittest.expectedFailure
+@unittest.skip('Flaky test')
def test_duplicated(self):
"""Series received on multiple mailing lists.
--
2.37.3
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH 02/10] trivial: Remove unnecessary unicode prefixes
Signed-off-by: Stephen Finucane
---
docs/conf.py| 6 +++---
patchwork/forms.py | 8 +---
patchwork/tests/test_parser.py | 12 ++--
patchwork/tests/views/test_mail.py | 16
patchwork/tests/views/test_patch.py | 2 +-
patchwork/tests/views/test_utils.py | 6 +++---
6 files changed, 26 insertions(+), 24 deletions(-)
diff --git docs/conf.py docs/conf.py
index 30a6b006..0b303c73 100644
--- docs/conf.py
+++ docs/conf.py
@@ -29,9 +29,9 @@ html_theme = 'sphinx_rtd_theme'
master_doc = 'index'
# General information about the project.
-project = u'Patchwork'
-copyright = u'2018-2019, Patchwork Developers'
-author = u'Patchwork Developers'
+project = 'Patchwork'
+copyright = '2018-, Patchwork Developers'
+author = 'Patchwork Developers'
# The version info for the project you're documenting, acts as replacement for
# |version| and |release|, also used in various other places throughout the
diff --git patchwork/forms.py patchwork/forms.py
index 7c9781af..84448586 100644
--- patchwork/forms.py
+++ patchwork/forms.py
@@ -18,9 +18,11 @@ class RegistrationForm(forms.Form):
first_name = forms.CharField(max_length=30, required=False)
last_name = forms.CharField(max_length=30, required=False)
username = forms.RegexField(
-regex=r'^\w+$', max_length=30, label=u'Username'
+regex=r'^\w+$',
+max_length=30,
+label='Username',
)
-email = forms.EmailField(max_length=100, label=u'Email address')
+email = forms.EmailField(max_length=100, label='Email address')
password = forms.CharField(widget=forms.PasswordInput(), label='Password')
def clean_username(self):
@@ -57,7 +59,7 @@ class BundleForm(forms.ModelForm):
regex=r'^[^/]+$',
min_length=1,
max_length=50,
-label=u'Name',
+label='Name',
error_messages={'invalid': 'Bundle names can\'t contain slashes'},
)
diff --git patchwork/tests/test_parser.py patchwork/tests/test_parser.py
index 108d8b9b..b731fe78 100644
--- patchwork/tests/test_parser.py
+++ patchwork/tests/test_parser.py
@@ -279,25 +279,25 @@ class SenderEncodingTest(TestCase):
def test_ascii_encoding(self):
from_header = 'example user '
-sender_name = u'example user'
+sender_name = 'example user'
sender_email = 'u...@example.com'
self._test_encoding(from_header, sender_name, sender_email)
def test_utf8qp_encoding(self):
from_header = '=?utf-8?q?=C3=A9xample=20user?= '
-sender_name = u'\xe9xample user'
+sender_name = '\xe9xample user'
sender_email = 'u...@example.com'
self._test_encoding(from_header, sender_name, sender_email)
def test_utf8qp_split_encoding(self):
from_header = '=?utf-8?q?=C3=A9xample?= user '
-sender_name = u'\xe9xample user'
+sender_name = '\xe9xample user'
sender_email = 'u...@example.com'
self._test_encoding(from_header, sender_name, sender_email)
def test_utf8b64_encoding(self):
from_header = '=?utf-8?B?w6l4YW1wbGUgdXNlcg==?= '
-sender_name = u'\xe9xample user'
+sender_name = '\xe9xample user'
sender_email = 'u...@example.com'
self._test_encoding(from_header, sender_name, sender_email)
@@ -552,12 +552,12 @@ class SubjectEncodingTest(TestCase):
def test_subject_utf8qp_encoding(self):
subject_header = '=?utf-8?q?test=20s=c3=bcbject?='
-subject = u'test s\xfcbject'
+subject = 'test s\xfcbject'
self._test_encoding(subject_header, subject)
def test_subject_utf8qp_multiple_encoding(self):
subject_header = 'test =?utf-8?q?s=c3=bcbject?='
-subject = u'test s\xfcbject'
+subject = 'test s\xfcbject'
self._test_encoding(subject_header, subject)
diff --git patchwork/tests/views/test_mail.py patchwork/tests/views/test_mail.py
index f2b19973..de9df3d2 100644
--- patchwork/tests/views/test_mail.py
+++ patchwork/tests/views/test_mail.py
@@ -23,7 +23,7 @@ class MailSettingsTest(TestCase):
self.assertTrue(response.context['form'])
def test_post(self):
-email = u'f...@example.com'
+email = 'f...@example.com'
response = self.client.post(reverse('mail-settings'), {'email': email})
self.assertEqual(response.status_code, 200)
self.assertTemplateUsed(response, 'patchwork/mail-settings.html')
[PATCH 1/2] docs: Add references to VSCode Patchwork plugin
Signed-off-by: Stephen Finucane Cc: Florent Revest --- README.rst | 20 docs/usage/clients.rst | 32 +++- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git README.rst README.rst index 94ae431d..979801fb 100644 --- README.rst +++ README.rst @@ -38,6 +38,26 @@ subsystems of the Linux kernel. Although Patchwork has been developed with the kernel workflow in mind, the aim is to be flexible enough to suit the majority of community projects. +Usage +- + +Patchwork provides a web UI, a REST API, and a XML-RPC API (*deprecated*). You +can use the APIs to `build you own application`__ or you can use one the many +`existing clients`__. + +There are many existing Patchwork instances. Unless you're a larger project +that is already hosting many of its own resources, you may wish to request a +project on one of these instances. + +- patchwork.ozlabs.org +- patchwork.kernel.org +- patches.linaro.org +- patchwork.sourceware.org +- patchwork.open-mesh.org + +__ https://patchwork.readthedocs.io/en/latest/development/api/ +__ https://patchwork.readthedocs.io/en/latest/usage/clients/ + Requirements diff --git docs/usage/clients.rst docs/usage/clients.rst index 01dd62a2..47ddb3a1 100644 --- docs/usage/clients.rst +++ docs/usage/clients.rst @@ -4,6 +4,13 @@ Clients A number of clients are available for interacting with Patchwork's various APIs. +.. note:: + + Got a client that you think might be useful to the broader community? Feel + free to add it to this page by `submitting a patch`__. + + __ https://patchwork.readthedocs.io/en/latest/development/contributing/ + pwclient @@ -31,7 +38,7 @@ __ https://github.com/getpatchwork/pwclient/ git-pw -- -The :program:`git-pw` application can be used to integrate Git with Patchwork. +The :program:`git-pw` application can be used to integrate Patchwork with Git. The :program:`git-pw` application relies on the REST API and can be used to interact to list, download and apply series, bundles and individual patches. @@ -42,13 +49,28 @@ __ https://git-pw.readthedocs.io/ __ https://github.com/getpatchwork/git-pw/ +VSCode-Patchwork + + +The *Patchwork* VSCode plugin can be used to integrate Patchwork with VSCode. +This plugin relies on the REST API and can be used to view both patches and +series and to apply them locally. You can also browse patches and series and +look at replies. + +More information on the *Patchwork* VSCode plugin can be found on the `VSCode +Marketplace`__ and the `GitHub repo`__. + +__ https://marketplace.visualstudio.com/items?itemName=florent-revest.patchwork +__ https://github.com/FlorentRevest/vscode-patchwork + + snowpatch - -The :program:`snowpatch` application is a bridge between Patchwork and the -Jenkins continuous integration automation server. It monitors the REST API -for incoming patches, applies them on top of an existing git tree, triggers -appropriate builds and test suites, and reports the results back to Patchwork. +The *snowpatch* application is a bridge between Patchwork and the Jenkins +continuous integration automation server. It monitors the REST API for incoming +patches, applies them on top of an existing git tree, triggers appropriate +builds and test suites, and reports the results back to Patchwork. Find out more about :program:`snowpatch` at its `GitHub repo`__. -- 2.37.3 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH 2/2] docs: Add info on contributing docs
Noticed while whipping up a patch to document the new VSCode extension. Signed-off-by: Stephen Finucane --- docs/development/contributing.rst | 40 +++ 1 file changed, 40 insertions(+) diff --git docs/development/contributing.rst docs/development/contributing.rst index 16b3740d..7bf5e1ca 100644 --- docs/development/contributing.rst +++ docs/development/contributing.rst @@ -141,6 +141,43 @@ for more information. ` using the ``api`` section. +Documentation +- + +All documentation files including release notes are authored in +`reStructuredText`_ (rST). This is similar Markdown but significantly more +powerful and with a slightly trickier syntax. `Sphinx`_ is used for building +the documentation tree and the built documentation is published on `Read the +Docs`_. + +The documentation tree is broadly broken down into five categories: + +`api` + Documentation for the APIs. + +`deployment` + Documentation for people that wish to deploy or maintain a Patchwork + instance. + +`development` + Documentation for people that wish to contribute to Patchwork itself. + +`releases` + Release notes. + +`usage` + Documentation for people that wish to use and interact with an existing + Patchwork instance. + +When contributing documentation, consider where your new documents should live. +You should also ensure the documentation builds after your modifications. This +can be done using the ``docs`` *tox* target. + +.. code-block:: shell + + $ tox -e docs + + Reporting Issues @@ -186,5 +223,8 @@ Further information about the Patchwork mailing list is available can be found o .. _reno: https://docs.openstack.org/developer/reno/ .. _QEMU guidelines: http://wiki.qemu.org/Contribute/SubmitAPatch .. _Django REST Framework documentation: http://www.django-rest-framework.org/api-guide/versioning/ +.. _reStructuredText: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html +.. _Sphinx: https://www.sphinx-doc.org/en/master/ +.. _Read the Docs: https://readthedocs.org .. _GitHub issue tracker: https://github.com/getpatchwork/patchwork .. _lists.ozlabs.org: https://lists.ozlabs.org/listinfo/patchwork -- 2.37.3 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Patchwork v3.1.0 Available
We're delighted to announce the release of Patchwork v3.1.0 ("Hessian"):
https://github.com/getpatchwork/patchwork/releases/tag/v3.1.0
The v3.1.0 release is one of the smaller releases. For full information on the
options available, you should look at the full release notes in detail. The
below are the highlights:
* Added the ability to mark comments as addressed/unaddressed, allowing
maintainers to track work items.
* Added two new event types to the `/events` API: `cover-comment-created` and
`patch-comment-created`.
* Added support for recent versions of Python (3.10) and Django (3.2, 4.0),
while dropping support for older Python (3.6) and Django (2.2, 3.0, 3.1)
versions.
For further information on these features and the other changes in this release,
review the full release notes [1]. You can also refer to the documentation for
this release [2].
Our next version is expected to be another minor one, where we'll improve our
tagging support and generally improve on the features present. For more info, or
to provide feedback on this and future releases, be sure to subscribe to the
mailing list [3]
Happy patchworking!
---
Patchwork is a patch tracking system for community-based projects. It is
intended to make the patch management process easier for both the project's
contributors and maintainers, leaving time for the more important (and more
interesting) stuff. You can find out more on the Patchwork homepage [4], the
GitHub repo [5], or the docs [6].
[1]: https://patchwork.readthedocs.io/en/latest/releases/hessian/
[2]: https://patchwork.readthedocs.io/en/v3.1.0/
[3]: https://lists.ozlabs.org/listinfo/patchwork
[4]: http://jk.ozlabs.org/projects/patchwork/
[5]: https://github.com/getpatchwork/patchwork
[6]: https://patchwork.readthedocs.org/
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
Re: Stepping down as a Patchwork maintainer
On Wed, 2022-06-29 at 23:30 +1000, Daniel Axtens wrote: > Hi all, > > I think it's time for me to officially step down from Patchwork > maintainership. > > I've been in a new job for 3 months now, and this company doesn't use > mailing lists for development. So I'm no longer using Patchwork as a > patch submitter, and the low volume of patches to the Patchwork mailing > list meant I never used it very much as a maintainer either. I think > that someone who doesn't use a piece of software probably shouldn't be a > maintainer of that software. So, with some sadness, it's time for me to > relinquish the role. > > Additionally, the change in job means I no longer have any excuses to > use work time for Patchwork, and my free time continues to be very > limited. I think it's better for everyone if I don't stay in a role that > I'm not meaningfully able to keep up with. > > I'll do my best to reply to Patchwork mail where I'm personally listed > in the To:, but I'm probably going to (continue to) be pretty bad at > responding to mail to the list generally. I'm happy to keep my commit > bit for as long as Stephen and any future maintainers deem appropriate. > > Stephen, thanks for inviting me to the maintainership and for the trust > you've placed in me. I wish you all the best for the future of > Patchwork! > > To our long-suffering Patchwork administrators at kernel.org and > ozlabs.org, and to the smaller ones around the web - thank you for > trusting us and apologies for the regular migration dramas! Thanks > especially to jk for kicking off the patchwork project. Finally, to all > our contributors: thank you, you have in a very real way made Patchwork > what it is today. Thanks for your help these last few years, Daniel! Stephen > Kind regards, > Daniel ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH] urls: Encode slashes in message IDs
We were attempting to work around the fact that message IDs could
contain slashes which in some cases broke our ability to generate
meaningful URLs. Rather than doing this, insist that users encode these
slashes so that we can distinguish between semantically meaningful
slashes and those that form the URL. This is a slightly breaking change,
but the current behavior is already broken (see the linked bug) so this
seems reasonable.
Signed-off-by: Stephen Finucane
Closes: #433
Cc: d...@axtens.net
Cc: siddh...@gotplt.org
---
patchwork/models.py | 23 ++-
.../patchwork/partials/download-buttons.html | 6 ++---
.../patchwork/partials/patch-list.html| 2 +-
patchwork/templates/patchwork/submission.html | 8 +++
patchwork/tests/api/test_cover.py | 2 +-
patchwork/tests/api/test_patch.py | 2 +-
patchwork/tests/views/test_bundles.py | 8 +++
patchwork/tests/views/test_cover.py | 10
patchwork/tests/views/test_patch.py | 22 +-
patchwork/urls.py | 20 +---
patchwork/views/comment.py| 4 ++--
patchwork/views/cover.py | 4 ++--
patchwork/views/patch.py | 8 +++
.../notes/issue-433-5f048abbe3789556.yaml | 19 +++
14 files changed, 80 insertions(+), 58 deletions(-)
create mode 100644 releasenotes/notes/issue-433-5f048abbe3789556.yaml
diff --git patchwork/models.py patchwork/models.py
index 264af532..deac33f8 100644
--- patchwork/models.py
+++ patchwork/models.py
@@ -369,12 +369,24 @@ class EmailMixin(models.Model):
@property
def url_msgid(self):
-"""A trimmed messageid, suitable for inclusion in URLs"""
+"""A trimmed Message ID, suitable for inclusion in URLs"""
if settings.DEBUG:
assert self.msgid[0] == '<' and self.msgid[-1] == '>'
return self.msgid.strip('<>')
+@property
+def encoded_msgid(self):
+"""Like 'url_msgid' but with slashes percentage encoded."""
+# We don't want to encode all characters (i.e. use urllib.parse.quote)
+# because that would result in us encoding the '@' present in all
+# message IDs. Instead we only percent-encode any slashes present [1].
+# These are not common so this is very much expected to be an edge
+# case.
+#
+# [1] https://datatracker.ietf.org/doc/html/rfc3986.html#section-2
+return self.url_msgid.replace('/', '%2F')
+
def save(self, *args, **kwargs):
# Modifying a submission via admin interface changes '\n' newlines in
# message content to '\r\n'. We need to fix them to avoid problems,
@@ -436,7 +448,7 @@ class Cover(SubmissionMixin):
'cover-detail',
kwargs={
'project_id': self.project.linkname,
-'msgid': self.url_msgid,
+'msgid': self.encoded_msgid,
},
)
@@ -445,7 +457,7 @@ class Cover(SubmissionMixin):
'cover-mbox',
kwargs={
'project_id': self.project.linkname,
-'msgid': self.url_msgid,
+'msgid': self.encoded_msgid,
},
)
@@ -671,7 +683,7 @@ class Patch(SubmissionMixin):
'patch-detail',
kwargs={
'project_id': self.project.linkname,
-'msgid': self.url_msgid,
+'msgid': self.encoded_msgid,
},
)
@@ -680,7 +692,7 @@ class Patch(SubmissionMixin):
'patch-mbox',
kwargs={
'project_id': self.project.linkname,
-'msgid': self.url_msgid,
+'msgid': self.encoded_msgid,
},
)
@@ -760,7 +772,6 @@ class CoverComment(EmailMixin, models.Model):
class PatchComment(EmailMixin, models.Model):
-# parent
patch = models.ForeignKey(
Patch,
diff --git patchwork/templates/patchwork/partials/download-buttons.html
patchwork/templates/patchwork/partials/download-buttons.html
index 149bbc62..34c5f8fc 100644
--- patchwork/templates/patchwork/partials/download-buttons.html
+++ patchwork/templates/patchwork/partials/download-buttons.html
@@ -4,16 +4,16 @@
{{ submission.id }}
{% if submission.diff %}
-
diff
-
mbox
{% else %}
-
mbox
diff --git patchwork/templates/patchwork/partials/patch-list.html
patchwork/templates/patchwork/partials/patch-list.html
index a9a262eb..a882cd9d
Patchwork v3.0.5 Available
We're pleased to announce the release of Patchwork v3.0.5. This release is part of the "Grosgrain" release series: https://github.com/getpatchwork/patchwork/releases/tag/v3.0.5 This release is a PATCH release that focuses on bugfixes. For more details, please see below. Happy patchworking! --- Changes in Patchwork v3.0.4..v3.0.5 --- baaea03b Release 3.0.5 1057b9b1 Switch from Travis CI to GitHub Actions 62b1946d migrations: ignore flake8 on 0041_python3 52bfa263 requirements: Pin jsonschema d33bc955 parser: Ignore CFWS in Message-ID header cd8f14fb parser: Ignore CFWS in In-Reply-To, References headers 215cede5 Post-release version bump ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 2/n] parser: Ignore CFWS in Message-ID header
On Fri, 2022-05-06 at 18:49 +0100, Stephen Finucane wrote:
> We recently started stripping comments and folding white space from the
> In-Reply-To and References headers. Do so also for the Message-ID
> header.
>
> Signed-off-by: Stephen Finucane
> Related: #399
> ---
> patchwork/parser.py| 43 ++
> patchwork/tests/test_parser.py | 32 +
> 2 files changed, 65 insertions(+), 10 deletions(-)
>
> diff --git patchwork/parser.py patchwork/parser.py
> index 17cc2325..f219f466 100644
> --- patchwork/parser.py
> +++ patchwork/parser.py
> @@ -236,15 +236,14 @@ def _find_series_by_references(project, mail):
> name, prefixes = clean_subject(subject, [project.linkname])
> version = parse_version(name, prefixes)
>
> -refs = find_references(mail)
> -h = clean_header(mail.get('Message-Id'))
> -if h:
> -refs = [h] + refs
> +msg_id = find_message_id(mail)
> +refs = [msg_id] + find_references(mail)
>
> for ref in refs:
> try:
> series = SeriesReference.objects.get(
> -msgid=ref[:255], project=project).series
> +msgid=ref[:255], project=project,
> +).series
>
> if series.version != version:
> # if the versions don't match, at least make sure these were
> @@ -473,6 +472,34 @@ def find_headers(mail):
> return '\n'.join(strings)
>
>
> +def find_message_id(mail):
> +"""Extract the 'message-id' headers from a given mail and validate it.
> +
> +The validation here is simply checking that the Message-ID is correctly
> +formatted per RFC-2822. However, even if it's not we'll attempt to use
> what
> +we're given because a patch tracked in Patchwork with janky threading is
> +better than no patch whatsoever.
> +"""
> +header = clean_header(mail.get('Message-Id'))
> +if not header:
> +raise ValueError("Broken 'Message-Id' header")
> +
> +msgid = _msgid_re.search(header)
> +if msgid:
> +msgid = msgid.group(0)
> +else:
> +# This is only info level since the admin likely can't do anything
> +# about this
> +logger.info(
> +"Malformed 'Message-Id' header. The 'msg-id' component should be
> "
> +"surrounded by angle brackets. Saving raw header. This may "
> +"include comments and extra comments."
> +)
I wonder if I should do this also for the 'In-Reply-To' field? I can't do it
easily for the 'References' field since there's zero way to delineate things if
I do, but that shouldn't matter since as long as we don't drop patches and
things appear roughly in order, we don't really need the 'References' field:
'In-Reply-To' is enough to find *a* parent.
Stephen
> +msgid = header
> +
> +return msgid[:255]
> +
> +
> def find_references(mail):
> """Construct a list of possible reply message ids.
>
> @@ -1062,11 +1089,7 @@ def parse_mail(mail, list_id=None):
>
> # parse metadata
>
> -msgid = clean_header(mail.get('Message-Id'))
> -if not msgid:
> -raise ValueError("Broken 'Message-Id' header")
> -msgid = msgid[:255]
> -
> +msgid = find_message_id(mail)
> subject = mail.get('Subject')
> name, prefixes = clean_subject(subject, [project.linkname])
> is_comment = subject_check(subject)
> diff --git patchwork/tests/test_parser.py patchwork/tests/test_parser.py
> index f65ad4b1..980a8afb 100644
> --- patchwork/tests/test_parser.py
> +++ patchwork/tests/test_parser.py
> @@ -1265,6 +1265,38 @@ class DuplicateMailTest(TestCase):
> self.assertEqual(Cover.objects.count(), 1)
>
>
> +class TestFindMessageID(TestCase):
> +
> +def test_find_message_id__missing_header(self):
> +email = create_email('test')
> +del email['Message-Id']
> +email['Message-Id'] = ''
> +
> +with self.assertRaises(ValueError) as cm:
> +parser.find_message_id(email)
> +self.assertIn("Broken 'Message-Id' header", str(cm.exeception))
> +
> +def test_find_message_id__header_with_comments(self):
> +"""Test that we strip comments from the Message-ID field."""
> +message_id = ' (message ID
[PATCH 2/n] parser: Ignore CFWS in Message-ID header
We recently started stripping comments and folding white space from the
In-Reply-To and References headers. Do so also for the Message-ID
header.
Signed-off-by: Stephen Finucane
Related: #399
---
patchwork/parser.py| 43 ++
patchwork/tests/test_parser.py | 32 +
2 files changed, 65 insertions(+), 10 deletions(-)
diff --git patchwork/parser.py patchwork/parser.py
index 17cc2325..f219f466 100644
--- patchwork/parser.py
+++ patchwork/parser.py
@@ -236,15 +236,14 @@ def _find_series_by_references(project, mail):
name, prefixes = clean_subject(subject, [project.linkname])
version = parse_version(name, prefixes)
-refs = find_references(mail)
-h = clean_header(mail.get('Message-Id'))
-if h:
-refs = [h] + refs
+msg_id = find_message_id(mail)
+refs = [msg_id] + find_references(mail)
for ref in refs:
try:
series = SeriesReference.objects.get(
-msgid=ref[:255], project=project).series
+msgid=ref[:255], project=project,
+).series
if series.version != version:
# if the versions don't match, at least make sure these were
@@ -473,6 +472,34 @@ def find_headers(mail):
return '\n'.join(strings)
+def find_message_id(mail):
+"""Extract the 'message-id' headers from a given mail and validate it.
+
+The validation here is simply checking that the Message-ID is correctly
+formatted per RFC-2822. However, even if it's not we'll attempt to use what
+we're given because a patch tracked in Patchwork with janky threading is
+better than no patch whatsoever.
+"""
+header = clean_header(mail.get('Message-Id'))
+if not header:
+raise ValueError("Broken 'Message-Id' header")
+
+msgid = _msgid_re.search(header)
+if msgid:
+msgid = msgid.group(0)
+else:
+# This is only info level since the admin likely can't do anything
+# about this
+logger.info(
+"Malformed 'Message-Id' header. The 'msg-id' component should be "
+"surrounded by angle brackets. Saving raw header. This may "
+"include comments and extra comments."
+)
+msgid = header
+
+return msgid[:255]
+
+
def find_references(mail):
"""Construct a list of possible reply message ids.
@@ -1062,11 +1089,7 @@ def parse_mail(mail, list_id=None):
# parse metadata
-msgid = clean_header(mail.get('Message-Id'))
-if not msgid:
-raise ValueError("Broken 'Message-Id' header")
-msgid = msgid[:255]
-
+msgid = find_message_id(mail)
subject = mail.get('Subject')
name, prefixes = clean_subject(subject, [project.linkname])
is_comment = subject_check(subject)
diff --git patchwork/tests/test_parser.py patchwork/tests/test_parser.py
index f65ad4b1..980a8afb 100644
--- patchwork/tests/test_parser.py
+++ patchwork/tests/test_parser.py
@@ -1265,6 +1265,38 @@ class DuplicateMailTest(TestCase):
self.assertEqual(Cover.objects.count(), 1)
+class TestFindMessageID(TestCase):
+
+def test_find_message_id__missing_header(self):
+email = create_email('test')
+del email['Message-Id']
+email['Message-Id'] = ''
+
+with self.assertRaises(ValueError) as cm:
+parser.find_message_id(email)
+self.assertIn("Broken 'Message-Id' header", str(cm.exeception))
+
+def test_find_message_id__header_with_comments(self):
+"""Test that we strip comments from the Message-ID field."""
+message_id = ' (message ID with a comment)'
+email = create_email('test', msgid=message_id)
+
+expected = ''
+actual = parser.find_message_id(email)
+
+self.assertEqual(expected, actual)
+
+def test_find_message_id__invalid_header_fallback(self):
+"""Test that we accept badly formatted Message-ID fields."""
+message_id = '5899d592-8c87-47d9-92b6-d34260ce1...@radware.com>'
+email = create_email('test', msgid=message_id)
+
+expected = '5899d592-8c87-47d9-92b6-d34260ce1...@radware.com>'
+actual = parser.find_message_id(email)
+
+self.assertEqual(expected, actual)
+
+
class TestFindReferences(TestCase):
def test_find_references__header_with_comments(self):
--
2.35.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH] parser: Ignore CFWS in In-Reply-To, References headers
RFC2822 states that [1] a comment or folding white space is permitted to
be inserted before or after a msg-id in in the Message-ID, In-Reply-To
or References fields. Allow for this.
[1] https://tools.ietf.org/html/rfc2822#section-3.6.4
Signed-off-by: Stephen Finucane
Closes: #399
---
patchwork/parser.py | 13 ++-
patchwork/tests/test_parser.py| 79 +--
.../notes/issue-399-09d6f17aa54b14b2.yaml | 11 +++
3 files changed, 89 insertions(+), 14 deletions(-)
create mode 100644 releasenotes/notes/issue-399-09d6f17aa54b14b2.yaml
diff --git patchwork/parser.py patchwork/parser.py
index e6e1a7fb..17cc2325 100644
--- patchwork/parser.py
+++ patchwork/parser.py
@@ -31,6 +31,7 @@ from patchwork.models import SeriesReference
from patchwork.models import State
+_msgid_re = re.compile(r'<[^>]+>')
_hunk_re = re.compile(r'^\@\@ -\d+(?:,(\d+))? \+\d+(?:,(\d+))? \@\@')
_filename_re = re.compile(r'^(---|\+\+\+) (\S+)')
list_id_headers = ['List-ID', 'X-Mailing-List', 'X-list']
@@ -483,19 +484,15 @@ def find_references(mail):
if 'In-Reply-To' in mail:
for in_reply_to in mail.get_all('In-Reply-To'):
-r = clean_header(in_reply_to)
-if r:
-refs.append(r)
+ref = _msgid_re.search(clean_header(in_reply_to))
+if ref:
+refs.append(ref.group(0))
if 'References' in mail:
for references_header in mail.get_all('References'):
-h = clean_header(references_header)
-if not h:
-continue
-references = h.split()
+references = _msgid_re.findall(clean_header(references_header))
references.reverse()
for ref in references:
-ref = ref.strip()
if ref not in refs:
refs.append(ref)
diff --git patchwork/tests/test_parser.py patchwork/tests/test_parser.py
index 12f984bc..f65ad4b1 100644
--- patchwork/tests/test_parser.py
+++ patchwork/tests/test_parser.py
@@ -74,6 +74,7 @@ def _create_email(
sender=None,
listid=None,
in_reply_to=None,
+references=None,
headers=None,
):
msg['Message-Id'] = msgid or make_msgid()
@@ -84,6 +85,9 @@ def _create_email(
if in_reply_to:
msg['In-Reply-To'] = in_reply_to
+if references:
+msg['References'] = references
+
for header in headers or {}:
msg[header] = headers[header]
@@ -97,12 +101,21 @@ def create_email(
sender=None,
listid=None,
in_reply_to=None,
+references=None,
headers=None,
):
msg = MIMEText(content, _charset='us-ascii')
return _create_email(
-msg, msgid, subject, sender, listid, in_reply_to, headers)
+msg,
+msgid,
+subject,
+sender,
+listid,
+in_reply_to,
+references,
+headers,
+)
def parse_mail(*args, **kwargs):
@@ -1216,7 +1229,7 @@ class DuplicateMailTest(TestCase):
def test_duplicate_patch(self):
diff = read_patch('0001-add-line.patch')
-m = create_email(diff, listid=self.listid, msgid='1...@example.com')
+m = create_email(diff, listid=self.listid, msgid='<1...@example.com>')
self._test_duplicate_mail(m)
@@ -1224,18 +1237,26 @@ class DuplicateMailTest(TestCase):
def test_duplicate_comment(self):
diff = read_patch('0001-add-line.patch')
-m1 = create_email(diff, listid=self.listid, msgid='1...@example.com')
+m1 = create_email(
+diff,
+listid=self.listid,
+msgid='<1...@example.com>',
+)
_parse_mail(m1)
-m2 = create_email('test', listid=self.listid, msgid='2...@example.com',
- in_reply_to='1...@example.com')
+m2 = create_email(
+'test',
+listid=self.listid,
+msgid='<2...@example.com>',
+in_reply_to='<1...@example.com>',
+)
self._test_duplicate_mail(m2)
self.assertEqual(Patch.objects.count(), 1)
self.assertEqual(PatchComment.objects.count(), 1)
def test_duplicate_coverletter(self):
-m = create_email('test', listid=self.listid, msgid='1...@example.com')
+m = create_email('test', listid=self.listid,
msgid='<1...@example.com>')
del m['Subject']
m['Subject'] = '[PATCH 0/1] test cover letter'
@@ -1244,6 +1265,52 @@ class DuplicateMailTest(TestCase):
self.assertEqual(Cover.objects.count(), 1)
+class TestFindReferences(TestCase):
Re: [PATCH] Align check-get with other commands operating on patch IDs
On Wed, 2022-03-30 at 12:40 -0500, Rob Herring wrote: > check-get doesn't support specifying the project and doesn't support > more than one patch ID like other commands which take patch ID as an > argument. Align check-get with other commands like view and get. > > Cc: Olof Johansson > Signed-off-by: Rob Herring Reviewed-by: Stephen Finucane Thanks. Applied and included in the new 2.1.0 release. Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 2/2] REST: Add 'patch-comment-created', 'cover-comment-created' events
On Tue, 2022-03-29 at 19:57 +0100, Stephen Finucane wrote: > From: DJ Delorie > > This patch stitches in "comment created" events for patches and cover > letter into the event queue. > > Signed-off-by: DJ Delorie > Signed-off-by: Stephen Finucane > Closes: #424 > [stephenfin: Extend to cover letters also. Fix some formatting issues. > Add a release note] Just posting this in case anyone is curious. Assuming no one spots anything crazy, say, this week, I'll merge and try cut a 3.1 release in a reasonable timeframe. Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 1/2] docs: Don't version events
On Tue, 2022-03-29 at 19:57 +0100, Stephen Finucane wrote:
> We don't actually version the types of events we emit. Doing so would be
> possible, but this ship has already sailed (with API version 1.2).
>
> We also remove some unnecessary line breaks.
>
> Signed-off-by: Stephen Finucane
fwiw, I think we should drop this versioning silliness in API v{next} and just
expose the server version as a header. It was a (generally) fun experiment but
it's a pain in the a*** and of questionable benefit. We've got (IMO) excellent
API documentation + release notes so that should do the trick for the few API
clients that exist.
Stephen
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH 2/2] REST: Add 'patch-comment-created', 'cover-comment-created' events
From: DJ Delorie This patch stitches in "comment created" events for patches and cover letter into the event queue. Signed-off-by: DJ Delorie Signed-off-by: Stephen Finucane Closes: #424 [stephenfin: Extend to cover letters also. Fix some formatting issues. Add a release note] --- docs/api/schemas/latest/patchwork.yaml| 71 +++ docs/api/schemas/patchwork.j2 | 77 + docs/api/schemas/v1.0/patchwork.yaml | 59 + docs/api/schemas/v1.1/patchwork.yaml | 64 ++ docs/api/schemas/v1.2/patchwork.yaml | 69 +++ docs/api/schemas/v1.3/patchwork.yaml | 71 +++ patchwork/api/embedded.py | 36 patchwork/api/event.py| 86 +++ .../migrations/0046_patch_comment_events.py | 56 patchwork/models.py | 16 patchwork/signals.py | 30 +++ patchwork/tests/test_signals.py | 36 +++- .../comment-events-2bc9b754cad1fb32.yaml | 6 ++ 13 files changed, 657 insertions(+), 20 deletions(-) create mode 100644 patchwork/migrations/0046_patch_comment_events.py create mode 100644 releasenotes/notes/comment-events-2bc9b754cad1fb32.yaml diff --git docs/api/schemas/latest/patchwork.yaml docs/api/schemas/latest/patchwork.yaml index 43edfb04..06be99d3 100644 --- docs/api/schemas/latest/patchwork.yaml +++ docs/api/schemas/latest/patchwork.yaml @@ -420,6 +420,8 @@ paths: - check-created - series-created - series-completed + - cover-comment-created + - patch-comment-created - in: query name: series description: An ID of a series to filter events by. @@ -459,6 +461,8 @@ paths: - $ref: '#/components/schemas/EventCheckCreated' - $ref: '#/components/schemas/EventSeriesCreated' - $ref: '#/components/schemas/EventSeriesCompleted' +- $ref: '#/components/schemas/EventCoverCommentCreated' +- $ref: '#/components/schemas/EventPatchCommentCreated' discriminator: propertyName: category mapping: @@ -471,6 +475,8 @@ paths: check-created: '#/components/schemas/EventCheckCreated' series-created: '#/components/schemas/EventSeriesCreated' series-completed: '#/components/schemas/EventSeriesCompleted' + cover-comment-created: '#/components/schemas/EventCoverCommentCreated' + patch-comment-created: '#/components/schemas/EventPatchCommentCreated' tags: - events /api/patches/: @@ -1919,6 +1925,34 @@ components: properties: series: $ref: '#/components/schemas/SeriesEmbedded' +EventCoverCommentCreated: + allOf: +- $ref: '#/components/schemas/EventBase' +- type: object + properties: +category: + enum: +- cover-comment-created +payload: + properties: +cover: + $ref: '#/components/schemas/CoverEmbedded' +comment: + $ref: '#/components/schemas/CommentEmbedded' +EventPatchCommentCreated: + allOf: +- $ref: '#/components/schemas/EventBase' +- type: object + properties: +category: + enum: +- patch-comment-created +payload: + properties: +patch: + $ref: '#/components/schemas/PatchEmbedded' +comment: + $ref: '#/components/schemas/CommentEmbedded' PatchList: required: - state @@ -2394,6 +2428,43 @@ components: maxLength: 255 minLength: 1 readOnly: true +CommentEmbedded: + type: object + properties: +id: + title: ID + type: integer + readOnly: true +url: + title: URL + type: string + format: uri + readOnly: true +web_url: + title: Web URL + type: string + format: uri + readOnly: true +msgid: + title: Message ID + type: string + readOnly: true + minLength: 1 +list_archive_url: + title: List archive URL + type: string + readOnly: true + nullable: true +date: + title: Date + type: string + format: iso8601 + readOnly:
[PATCH 1/2] docs: Don't version events
We don't actually version the types of events we emit. Doing so would be
possible, but this ship has already sailed (with API version 1.2).
We also remove some unnecessary line breaks.
Signed-off-by: Stephen Finucane
---
docs/api/schemas/latest/patchwork.yaml | 21 +++--
docs/api/schemas/patchwork.j2 | 29 +++---
docs/api/schemas/v1.0/patchwork.yaml | 41 --
docs/api/schemas/v1.1/patchwork.yaml | 41 --
docs/api/schemas/v1.2/patchwork.yaml | 21 +++--
docs/api/schemas/v1.3/patchwork.yaml | 21 +++--
6 files changed, 104 insertions(+), 70 deletions(-)
diff --git docs/api/schemas/latest/patchwork.yaml
docs/api/schemas/latest/patchwork.yaml
index 2a98c179..43edfb04 100644
--- docs/api/schemas/latest/patchwork.yaml
+++ docs/api/schemas/latest/patchwork.yaml
@@ -402,7 +402,11 @@ paths:
type: string
- in: query
name: category
- description: An event category to filter events by.
+ description: >
+An event category to filter events by. These categories are subject
+to change depending on the version of Patchwork deployed and are
+not subject to the versionining constraints present across the rest
+of the API.
schema:
title: ''
type: string
@@ -460,18 +464,13 @@ paths:
mapping:
cover-created: '#/components/schemas/EventCoverCreated'
patch-created: '#/components/schemas/EventPatchCreated'
- patch-completed: >
-'#/components/schemas/EventPatchCompleted'
- patch-state-changed: >
-'#/components/schemas/EventPatchStateChanged'
- patch-relation-changed: >
-'#/components/schemas/EventPatchRelationChanged'
- patch-delegated: >
-'#/components/schemas/EventPatchDelegated'
+ patch-completed:
'#/components/schemas/EventPatchCompleted'
+ patch-state-changed:
'#/components/schemas/EventPatchStateChanged'
+ patch-relation-changed:
'#/components/schemas/EventPatchRelationChanged'
+ patch-delegated:
'#/components/schemas/EventPatchDelegated'
check-created: '#/components/schemas/EventCheckCreated'
series-created: '#/components/schemas/EventSeriesCreated'
- series-completed: >
-'#/components/schemas/EventSeriesCompleted'
+ series-completed:
'#/components/schemas/EventSeriesCompleted'
tags:
- events
/api/patches/:
diff --git docs/api/schemas/patchwork.j2 docs/api/schemas/patchwork.j2
index 02aa9f72..df72c1ac 100644
--- docs/api/schemas/patchwork.j2
+++ docs/api/schemas/patchwork.j2
@@ -413,7 +413,11 @@ paths:
type: string
- in: query
name: category
- description: An event category to filter events by.
+ description: >
+An event category to filter events by. These categories are subject
+to change depending on the version of Patchwork deployed and are
+not subject to the versionining constraints present across the rest
+of the API.
schema:
title: ''
type: string
@@ -422,9 +426,7 @@ paths:
- patch-created
- patch-completed
- patch-state-changed
-{% if version >= (1, 2) %}
- patch-relation-changed
-{% endif %}
- patch-delegated
- check-created
- series-created
@@ -463,9 +465,7 @@ paths:
- $ref: '#/components/schemas/EventPatchCreated'
- $ref: '#/components/schemas/EventPatchCompleted'
- $ref: '#/components/schemas/EventPatchStateChanged'
-{% if version >= (1, 2) %}
- $ref: '#/components/schemas/EventPatchRelationChanged'
-{% endif %}
- $ref: '#/components/schemas/EventPatchDelegated'
- $ref: '#/components/schemas/EventCheckCreated'
- $ref: '#/components/schemas/EventSeriesCreated'
@@ -475,20 +475,13 @@ paths:
mapping:
cover-created: '#/components/schemas/EventCoverCreated'
patch-created: '#/components/schemas/EventPatchCreated'
- patch-completed: >
-
Testing, please ignore
I've seen reports that the mailing list is rejecting emails. Let's see if that's true for everyone. Please ignore. Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] docker: rebase on vicamo/pyenv
On Thu, 2021-12-23 at 12:16 +0800, You-Sheng Yang wrote: > Stephen Finucane 於 2021年12月23日 週四 上午6:05寫道： > > > > On Sun, 2021-12-12 at 14:17 +0800, You-Sheng Yang wrote: > > > Rebuild pyenv environments can be time-consuming and irrelevant to this > > > project. Use a prebuild image to save some time here. > > > > > > Signed-off-by: You-Sheng Yang > > > > This is a good idea. I've created a similar image at [1] which (I think) > > will do > > the same thing, simply so we can quickly iterate on it if we need to in the > > future. Let me know if you spot anything amiss. If not, I'll merge this > > (with > > the different base image) next week. > > I think that looks good to me. Thanks for looking at this. I completely forgot about it /o\ I've applied this now. > For those `apt-cache search` lines, I was to cover some major > Debian/Ubuntu releases. You might not need them. Good point. I stripped those (and added a credit linking back to your project) > Besides, while python3.10 is out and 3.11 is coming, I was personally > trying to rewrite a bit to allow automatic builds of newly available > python versions in my own repo. Add some build argument to the build > stage of the Dockerfile allows you to test that. Taking a step > further, split build stage as builder and build-all stages as I did in > [1], then you have a handy image for manual testing. Pretty convenient > for me. Thanks for the tips. I'll take a look into doing this. We need to do some work on our supported Python versions for Patchwork proper so align nicely. Thanks again, Stephen > > [1]:https://github.com/vicamo/docker-pyenv/blob/5159e6c94d2f173f6bcc3759448ce5dea228ab08/Dockerfile-debian.template ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] docker: rebase on vicamo/pyenv
On Sun, 2021-12-12 at 14:17 +0800, You-Sheng Yang wrote: > Rebuild pyenv environments can be time-consuming and irrelevant to this > project. Use a prebuild image to save some time here. > > Signed-off-by: You-Sheng Yang This is a good idea. I've created a similar image at [1] which (I think) will do the same thing, simply so we can quickly iterate on it if we need to in the future. Let me know if you spot anything amiss. If not, I'll merge this (with the different base image) next week. Thanks again, Stephen [1] https://github.com/getpatchwork/pyenv/ ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] docs: pin mistune version due to breakage
On Mon, 2021-12-06 at 16:42 +0800, You-Sheng Yang wrote: > mistune made a new release which breaks API referenced by Sphinx v4.3.1. > > Closes: #442 > Signed-off-by: You-Sheng Yang Reviewed-by: Stephen Finucane I was hoping to be able to fix the underlying issue myself, but the changes in mistune are rather significant. I've applied this. Thank you! For anyone curious, this is brought in by sphinxcontrib-openapi, which in turn includes m2r (to convert markdown to reStructuredText), which in turn includes the mistune library to handle the markdown parsing. Given the changes in mistune 2.0, I'm not sure how sphinxcontrib-openapi will resolve this. Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] patch-list: Link to delegate's assigned patches
On Mon, 2021-11-22 at 18:20 -0500, Sean Anderson wrote:
> This adds a link to all patches currently delegated to a user. This can
> be much easier than going through the delegate filter, especially if you
> already have a patch delegated to that user. This also replaces username
> text with their real name or email (if they are populated). This can
> help submitters figure out who their patches are assigned to (in cases
> where the username and real name of the delegate significantly diverge).
Thanks for the patch. Some comments below. If you can address these, I'd be
happy to apply this.
Stephen
PS: This didn't appear on Patchwork. I've no idea why...
> Signed-off-by: Sean Anderson
> ---
>
> .../templates/patchwork/partials/patch-list.html| 2 +-
> patchwork/templatetags/person.py| 13 -
> .../notes/link-to-delegates-366d5b18f1367fd6.yaml | 5 +
> 3 files changed, 18 insertions(+), 2 deletions(-)
> create mode 100644 releasenotes/notes/link-to-delegates-366d5b18f1367fd6.yaml
>
> diff --git a/patchwork/templates/patchwork/partials/patch-list.html
> b/patchwork/templates/patchwork/partials/patch-list.html
> index 02d6dff..4918aeb 100644
> --- a/patchwork/templates/patchwork/partials/patch-list.html
> +++ b/patchwork/templates/patchwork/partials/patch-list.html
> @@ -204,7 +204,7 @@ $(document).ready(function() {
> {{ patch|patch_checks }}
> {{ patch.date|date:"Y-m-d" }}
> {{ patch.submitter|personify:project }}
> - {{ patch.delegate.username }}
> + {{ patch.delegate|delegatify:project }}
> {{ patch.state }}
>
> {% empty %}
> diff --git a/patchwork/templatetags/person.py
> b/patchwork/templatetags/person.py
> index 61937d9..433d713 100644
> --- a/patchwork/templatetags/person.py
> +++ b/patchwork/templatetags/person.py
> @@ -8,7 +8,7 @@ from django.urls import reverse
> from django.utils.html import escape
> from django.utils.safestring import mark_safe
>
> -from patchwork.filters import SubmitterFilter
> +from patchwork.filters import SubmitterFilter, DelegateFilter
>
>
> register = template.Library()
> @@ -28,3 +28,14 @@ def personify(person, project):
> url, SubmitterFilter.param, escape(person.id), linktext)
>
> return mark_safe(out)
> +
> +@register.filter
> +def delegatify(delegate, project):
> +
> +linktext = escape(delegate.name or delegate.email or delegate.username)
delegate will be None if the patch hasn't been delegated to anyone. You need to
handle this.
> +url = reverse('patch-list',
> + kwargs={'project_id': project.linkname})
style nit: I think this would all fit on one line?
> +out = '%s' % (
> +url, DelegateFilter.param, escape(delegate.id), linktext)
> +
> +return mark_safe(out)
It would be great if there was some unit tests for this filter to ensure things
are working as expected. I don't think we have any tests for this stuff
currently but it should be pretty trivial to add, either in
'patchwork/tests/views/test_patch.py' or in a new
'patchwork/tests/templatetags/test_person.py' file. Heck, why not both, since
the former are really more functional test'y than the latter. Note that you can
build on an existing test for the former and refer to this StackOverflow answer
for a guide on how to do the latter [1].
> diff --git a/releasenotes/notes/link-to-delegates-366d5b18f1367fd6.yaml
> b/releasenotes/notes/link-to-delegates-366d5b18f1367fd6.yaml
> new file mode 100644
> index 000..aa69df3
> --- /dev/null
> +++ b/releasenotes/notes/link-to-delegates-366d5b18f1367fd6.yaml
> @@ -0,0 +1,5 @@
> +---
> +features:
> + - |
> +The delegate column in the patch list now links to all patches delegated
> to
> +that user.
[1] https://stackoverflow.com/a/49604862/613428
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH v2] ci: add docker-compose tests
On Sat, 2021-11-27 at 17:40 +0800, You-Sheng Yang wrote: > v2: fix intermittent error. > > Signed-off-by: You-Sheng Yang This is great work. Thank you! I removed the release note since this isn't end- user facing and then applied this to master. Cheers, Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 1/2] REST: Don't error if a versioned field we would remove is absent
On Fri, 2021-10-22 at 08:28 -0400, Konstantin Ryabitsev wrote:
> On Fri, 20 Aug 2021 at 17:59, Stephen Finucane wrote:
> > > We remove fields that shouldn't be seen on old versions of the API.
> > > This was done with `pop(field name)`, which will throw an exception
> > > if the named field is absent from the data. However, sometimes if
> > > a patch request is via an old API version, we hit this line without
> > > ever having the field present.
> > >
> > > This is odd, but not harmful and we definitely shouldn't 500.
> > >
> > > Fixes: d944f17ec059 ("REST: Use versioning for modified responses")
> > > Signed-off-by: Daniel Axtens
> >
> > Looks good to me.
> >
> > Reviewed-by: Stephen Finucane
> >
> > I squashed the test into this (with a tweak in the name) and applied it.
>
> Hi, all:
>
> Could this please be backported to stable/2.2 and maybe released as 2.2.6?
>
> -K
Hey,
v2.2.6 is available now. Let me know if there are any issues.
Somewhat related: are there any large blockers preventing you moving to v3.0.x?
Is it the major version bump or removal of Python 2.7 support that's preventing
the upgrade, or is there something else? In particular, is there anything that
we could address? I'd like to get v3.1.0 out before the end of year but I'm
slightly concerned that no one (?) has rolled forward to v3.0.x yet :D
Cheers,
Stephen
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
Patchwork v2.2.6 Available
We're pleased to announce the release of Patchwork v2.2.6. This release is part of the "Flannel" release series: https://github.com/getpatchwork/patchwork/releases/tag/v2.2.6 This release is a PATCH release that focuses on bugfixes. For more details, please see below. Happy patchworking! --- Changes in Patchwork v2.2.5..v2.2.6 --- 97f7adee Release 2.2.6 1d860982 REST: Don't error if a versioned field we would remove is absent d1c6ea11 Post-release version bump ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Patchwork v3.0.4 Available
We're pleased to announce the release of Patchwork v3.0.4. This release is part of the "Grosgrain" release series: https://github.com/getpatchwork/patchwork/releases/tag/v3.0.4 This release is a PATCH release that focuses on bugfixes. For more details, please see below. Happy patchworking! --- Changes in Patchwork v3.0.3..v3.0.4 --- 2d68dafa Release 3.0.4 956f988a REST: Don't error if a versioned field we would remove is absent 6d159b18 migrations: don't go to the db for 0041_python3 migration 0235b2b2 Post-release version bump ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH 6/6] trivial: Stop universally disabling F405 check
We only need this for one file. Simply filter out the things we want to
ignore.
Signed-off-by: Stephen Finucane
---
patchwork/settings/dev.py | 6 --
tox.ini | 3 +--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git patchwork/settings/dev.py patchwork/settings/dev.py
index aa3ee3c5..2b7fc954 100644
--- patchwork/settings/dev.py
+++ patchwork/settings/dev.py
@@ -7,6 +7,8 @@ Design based on:
http://www.revsys.com/blog/2014/nov/21/recommended-django-project-layout/
"""
+import os
+
from .base import * # noqa
try:
@@ -77,7 +79,7 @@ PASSWORD_HASHERS =
['django.contrib.auth.hashers.MD5PasswordHasher']
# django-debug-toolbar
if debug_toolbar:
-INSTALLED_APPS += [
+INSTALLED_APPS += [ # noqa: F405
'debug_toolbar'
]
@@ -86,7 +88,7 @@ if debug_toolbar:
# This should go first in the middleware classes
MIDDLEWARE = [
'debug_toolbar.middleware.DebugToolbarMiddleware',
-] + MIDDLEWARE
+] + MIDDLEWARE # noqa: F405
INTERNAL_IPS = [
'127.0.0.1', '::1',
diff --git tox.ini tox.ini
index 1de53ac3..494e9fb5 100644
--- tox.ini
+++ tox.ini
@@ -47,9 +47,8 @@ commands = flake8 {posargs:patchwork manage.py}
# Some rules are ignored as their use makes the code more difficult to read:
#
# E129 visually indented line with same indent as next logical line
-# F405 'name' may be undefined, or defined from star imports: 'module'
# W504 line break after binary operator
-ignore = E129, F405, W504
+ignore = E129, W504
[testenv:docs]
deps =
--
2.31.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH 3/6] requirements: Bump Django to 3.2.x, django-filter to 21.1
It seems the ORM is now smarter and requires less JOINs that previously
in two tests. In addition, a new setting is required to ensure the type
of our primary field columns doesn't change when Django 4.0 is released.
We drop support for Django 3.1 in the process, though this doesn't have
much of a real-world impact since we still support Django 2.2, an LTS
release.
Signed-off-by: Stephen Finucane
---
patchwork/settings/base.py | 4
patchwork/tests/api/test_event.py| 6 +-
patchwork/tests/api/test_patch.py| 6 +-
.../notes/django-3-2-support-363b32e74bdcf017.yaml | 9 +
requirements-dev.txt | 4 ++--
requirements-prod.txt| 4 ++--
tox.ini | 12 ++--
7 files changed, 33 insertions(+), 12 deletions(-)
create mode 100644 releasenotes/notes/django-3-2-support-363b32e74bdcf017.yaml
diff --git patchwork/settings/base.py patchwork/settings/base.py
index c1bb9b27..ff14d91b 100644
--- patchwork/settings/base.py
+++ patchwork/settings/base.py
@@ -68,6 +68,10 @@ TEMPLATES = [
},
]
+# TODO(stephenfin): Consider changing to BigAutoField when we drop support for
+# Django < 3.2
+DEFAULT_AUTO_FIELD = 'django.db.models.AutoField'
+
DEFAULT_FROM_EMAIL = 'Patchwork '
SERVER_EMAIL = DEFAULT_FROM_EMAIL
diff --git patchwork/tests/api/test_event.py patchwork/tests/api/test_event.py
index 32e99366..62506a5b 100644
--- patchwork/tests/api/test_event.py
+++ patchwork/tests/api/test_event.py
@@ -5,6 +5,7 @@
import unittest
+import django
from django.conf import settings
from django.urls import reverse
@@ -187,7 +188,10 @@ class TestEventAPI(utils.APITestCase):
for _ in range(3):
self._create_events()
-with self.assertNumQueries(28):
+# TODO(stephenfin): Remove when we drop support for Django < 3.2
+num_queries = 28 if django.VERSION < (3, 2) else 27
+
+with self.assertNumQueries(num_queries):
self.client.get(self.api_url())
def test_order_by_date_default(self):
diff --git patchwork/tests/api/test_patch.py patchwork/tests/api/test_patch.py
index 1c616409..36c3e5f9 100644
--- patchwork/tests/api/test_patch.py
+++ patchwork/tests/api/test_patch.py
@@ -7,6 +7,7 @@ import email.parser
from email.utils import make_msgid
import unittest
+import django
from django.conf import settings
from django.urls import NoReverseMatch
from django.urls import reverse
@@ -228,7 +229,10 @@ class TestPatchAPI(utils.APITestCase):
series = create_series()
create_patches(5, series=series)
-with self.assertNumQueries(7):
+# TODO(stephenfin): Remove when we drop support for Django < 3.2
+num_queries = 7 if django.VERSION < (3, 2) else 5
+
+with self.assertNumQueries(num_queries):
self.client.get(self.api_url())
@utils.store_samples('patch-detail')
diff --git releasenotes/notes/django-3-2-support-363b32e74bdcf017.yaml
releasenotes/notes/django-3-2-support-363b32e74bdcf017.yaml
new file mode 100644
index ..039535e9
--- /dev/null
+++ releasenotes/notes/django-3-2-support-363b32e74bdcf017.yaml
@@ -0,0 +1,9 @@
+---
+features:
+ - |
+`Django 3.2 <https://docs.djangoproject.com/en/dev/releases/3.2/>`_ is now
+supported.
+upgrade:
+ - |
+Django 3.1 is no longer supported. It is no longer supported upstream and
+most distributions provide a newer version.
diff --git requirements-dev.txt requirements-dev.txt
index d58dfaa9..437b1e8b 100644
--- requirements-dev.txt
+++ requirements-dev.txt
@@ -1,6 +1,6 @@
-Django~=3.1.0
+Django~=3.2.0
djangorestframework~=3.12.0
-django-filter~=2.4.0
+django-filter~=21.1.0
django-debug-toolbar~=3.2.0
django-dbbackup~=3.3.0
-r requirements-test.txt
diff --git requirements-prod.txt requirements-prod.txt
index 6bac4d6a..fa6bc95f 100644
--- requirements-prod.txt
+++ requirements-prod.txt
@@ -1,5 +1,5 @@
-Django~=3.1.0
+Django~=3.2.0
djangorestframework~=3.12.0
-django-filter~=2.4.0
+django-filter~=21.1.0
psycopg2-binary~=2.8.0
sqlparse~=0.4.0
diff --git tox.ini tox.ini
index 46d72517..8eab01da 100644
--- tox.ini
+++ tox.ini
@@ -1,6 +1,6 @@
[tox]
minversion = 3.2
-envlist = pep8,docs,py{36,37,38,39}-django{22,30,31}
+envlist = pep8,docs,py{36,37,38,39}-django{22,31,32}
skipsdist = true
ignore_basepython_conflict = true
@@ -10,13 +10,13 @@ deps =
-r{toxinidir}/requirements-test.txt
django22: django>=2.2,<2.3
django22: djangorestframework>=3.10,<3.13
-django22: django-filter>=2.1,<3.0
-django30: django>=3.0,<3.1
-django30: djangorestframework>=3.10,<3.13
-django30: django-filter>=2.2,<3.0
+django22: django-filter~=21.1.0
django31: django>=3.1,<3.2
django31: djan
[PATCH 5/6] requirements: Bump psycopg2-binary, openapi-core
Once again, openapi-core has moved stuff around internally. Nothing we can't fix though, as it seems the thing moved is no longer necessary. Signed-off-by: Stephen Finucane --- patchwork/tests/api/validator.py | 6 +- requirements-test.txt| 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git patchwork/tests/api/validator.py patchwork/tests/api/validator.py index 2b1921a3..96912227 100644 --- patchwork/tests/api/validator.py +++ patchwork/tests/api/validator.py @@ -10,8 +10,7 @@ from django.urls import resolve import openapi_core from openapi_core.contrib.django import DjangoOpenAPIRequestFactory from openapi_core.contrib.django import DjangoOpenAPIResponseFactory -from openapi_core.schema.media_types.exceptions import OpenAPIMediaTypeError -from openapi_core.schema.parameters.exceptions import OpenAPIParameterError +from openapi_core.exceptions import OpenAPIParameterError from openapi_core.templating import util from openapi_core.unmarshalling.schemas.formatters import Formatter from openapi_core.validation.request.validators import RequestValidator @@ -113,9 +112,6 @@ def validate_data(path, request, response, validate_request, result = validator.validate(request) try: result.raise_for_errors() -except OpenAPIMediaTypeError: -if response.status_code != status.HTTP_400_BAD_REQUEST: -raise except OpenAPIParameterError: # TODO(stephenfin): In API v2.0, this should be an error. As things # stand, we silently ignore these issues. diff --git requirements-test.txt requirements-test.txt index c8ce258b..7f03b0fc 100644 --- requirements-test.txt +++ requirements-test.txt @@ -1,6 +1,6 @@ mysqlclient~=2.0.0 -psycopg2-binary~=2.8.0 +psycopg2-binary~=2.9.0 sqlparse~=0.4.0 python-dateutil~=2.8.0 tblib~=1.7.0 -openapi-core~=0.13.4 +openapi-core~=0.14.2 -- 2.31.1 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH 4/6] tox: Use compatible minor releases
As we previously did for the requirements files (commit c90473ea44),
switch to use compatible releases for tox. We previously avoided doing
this on the basis that the existing system gave an idea of supported
package versions, but we weren't actually testing these combinations
since pip will always install the latest available version of each
package. This will apply to anyone using pip to manage their
dependencies, while anyone using distro packages can rely on the distro
having done this testing for them. Given the above, this makes our
effort to track supported ranges moot. So long as we cap newer versions
of e.g. django-filter when they're not compatible with older Django
versions, we should be good.
Signed-off-by: Stephen Finucane
---
tox.ini | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git tox.ini tox.ini
index 8eab01da..1de53ac3 100644
--- tox.ini
+++ tox.ini
@@ -8,14 +8,14 @@ ignore_basepython_conflict = true
basepython = python3
deps =
-r{toxinidir}/requirements-test.txt
-django22: django>=2.2,<2.3
-django22: djangorestframework>=3.10,<3.13
+django22: django~=2.2.0
+django22: djangorestframework~=3.12.0
django22: django-filter~=21.1.0
-django31: django>=3.1,<3.2
-django31: djangorestframework>=3.10,<3.13
+django31: django~=3.1.0
+django31: djangorestframework~=3.12.0
django31: django-filter~=21.1.0
-django32: django>=3.2,<3.3
-django32: djangorestframework>=3.10,<3.13
+django32: django>=3.2.0
+django32: djangorestframework~=3.12.0
django32: django-filter~=21.1.0
setenv =
DJANGO_SETTINGS_MODULE = patchwork.settings.dev
--
2.31.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH 2/6] requirements: Bump doc requirements
We were suggesting we supported positively ancient versions of Sphinx and reno in particular. Bump these. Signed-off-by: Stephen Finucane --- docs/requirements.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git docs/requirements.txt docs/requirements.txt index e2641c8f..f17cb789 100644 --- docs/requirements.txt +++ docs/requirements.txt @@ -1,5 +1,5 @@ -sphinx>=2.0 -reno>=2.2 -sphinx_rtd_theme~=0.5.0 -jinja2~=2.11.2 +sphinx>=4.2.0 +reno>=3.4.0 +sphinx-rtd-theme~=1.0 +jinja2~=3.0 sphinxcontrib-openapi~=0.7.0 -- 2.31.1 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH 1/6] Revert "docs: prevent build error by rolling back Sphinx version"
This reverts commit 65547c8701004f1a2a9ed9d16f1a372f4bd856e4. There is a new release of sphinxcontrib-httpdomain [1] that fixes the issues with Sphinx 4.1.x and later [2] [1] https://github.com/sphinx-contrib/httpdomain/releases/tag/1.8.0 [2] https://github.com/sphinx-contrib/httpdomain/pull/54 --- docs/requirements.txt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git docs/requirements.txt docs/requirements.txt index b60bca53..e2641c8f 100644 --- docs/requirements.txt +++ docs/requirements.txt @@ -1,6 +1,4 @@ -# sphinx 4.1.x breaks sphinxcontrib-httpdomain which sphinxcontrib-openapi depends on -# see https://github.com/sphinx-contrib/httpdomain/pull/51 -sphinx>=2.0,<4.1 +sphinx>=2.0 reno>=2.2 sphinx_rtd_theme~=0.5.0 jinja2~=2.11.2 -- 2.31.1 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] models: Add Series.is_open
On Thu, 2021-09-23 at 12:43 +0100, Stephen Finucane wrote: > On Thu, 2021-09-23 at 12:40 +0100, Stephen Finucane wrote: > > On Sun, 2021-09-05 at 11:34 +1000, Daniel Axtens wrote: > > > Stephen Finucane writes: > > > > > > > Start making series more useful by tracking whether the state is open, > > > > meaning one or more patches are in a state with action_required=True, or > > > > not. This means that the 'git-pw series list' command will only list > > > > series that are actually actionable. > > > > > > > > Signed-off-by: Stephen Finucane > > > > --- > > > > Open questions: > > > > - Is there a more efficient way to do the migration than I've done? > > > > - Should we expose an 'is_open' boolean attribute of a 'state' (open, > > > > closed) attribute for the series views? > > > > - Is there a race condition possible in how I've implemented the series > > > > state checks? > > > > > > AIUI, series.is_open is a derived property (or I suppose in db terms a > > > 'view') of series.patches. > > > > > > Is the cost of computing it 'live' so bad that we need to cache it in > > > the db? (a 'materialised view'[1]) > > > > > > Is there any chance we could just always compute on demand? I'm trying > > > to think of all the ways we expose series: > > > > > > - on the patch list page, we already also fetch all the patches > > > > > > - on the API /series/ page, we also fetch all the patches even for just > > >the list view. > > > > > > so it seems to me we probably shouldn't suffer a performance hit for > > > doing it live. > > > > The biggest impact I see is filtering. This is the main thing I want because > > presently 'git pw series list' is pretty much useless without this. I don't > > doing this dynamically would work because attempting to fetch, for example, > > the > > first 100 "closed" series will require loading every series and every > > patch. You > > couldn't just load the first 100 since they could be entirely/mostly "open", > > meaning you'd need to load another 100, and another, and another, until you > > get > > 100 closed. This will need to be stored _somewhere_ fwict. > > > > > [1] > > > https://www.datacamp.com/community/tutorials/materialized-views-postgresql > > > and noting that materialised views are not supported natively in mysql - > > > but you've basically done https://linuxhint.com/materialized-views-mysql > > > with the refresh logic in the application layer rather than a stored > > > procedure. > > > > > > > diff --git patchwork/models.py patchwork/models.py > > > > index 58e4c51e..7441510b 100644 > > > > --- patchwork/models.py > > > > +++ patchwork/models.py > > > > @@ -14,6 +14,7 @@ from django.conf import settings > > > > from django.contrib.auth.models import User > > > > from django.core.exceptions import ValidationError > > > > from django.db import models > > > > +from django.db import transaction > > > > from django.urls import reverse > > > > from django.utils.functional import cached_property > > > > from django.core.validators import validate_unicode_slug > > > > @@ -494,6 +495,19 @@ class Patch(SubmissionMixin): > > > > for tag in tags: > > > > self._set_tag(tag, counter[tag]) > > > > > > > > +def refresh_series_state(self): > > > > +if not self.series: > > > > +return > > > > + > > > > +# If any of the patches in the series is in an "action > > > > required" state, > > > > +# then the series is still open > > > > +# TODO(stephenfin): Can this still race? > > > > +with transaction.atomic(): > > > > +self.series.is_open = self.series.patches.filter( > > > > +state__action_required=True > > > > +).exists() > > > > +self.series.save() > > > > + > > > > > > AIUI based some experimentation and my experience dealing with our > > > series race conditions in 2018, this can race and transactions won't > > > save you her
Re: [PATCH] models: Add Series.is_open
On Thu, 2021-09-23 at 12:40 +0100, Stephen Finucane wrote: > On Sun, 2021-09-05 at 11:34 +1000, Daniel Axtens wrote: > > Stephen Finucane writes: > > > > > Start making series more useful by tracking whether the state is open, > > > meaning one or more patches are in a state with action_required=True, or > > > not. This means that the 'git-pw series list' command will only list > > > series that are actually actionable. > > > > > > Signed-off-by: Stephen Finucane > > > --- > > > Open questions: > > > - Is there a more efficient way to do the migration than I've done? > > > - Should we expose an 'is_open' boolean attribute of a 'state' (open, > > > closed) attribute for the series views? > > > - Is there a race condition possible in how I've implemented the series > > > state checks? > > > > AIUI, series.is_open is a derived property (or I suppose in db terms a > > 'view') of series.patches. > > > > Is the cost of computing it 'live' so bad that we need to cache it in > > the db? (a 'materialised view'[1]) > > > > Is there any chance we could just always compute on demand? I'm trying > > to think of all the ways we expose series: > > > > - on the patch list page, we already also fetch all the patches > > > > - on the API /series/ page, we also fetch all the patches even for just > >the list view. > > > > so it seems to me we probably shouldn't suffer a performance hit for > > doing it live. > > The biggest impact I see is filtering. This is the main thing I want because > presently 'git pw series list' is pretty much useless without this. I don't > doing this dynamically would work because attempting to fetch, for example, > the > first 100 "closed" series will require loading every series and every patch. > You > couldn't just load the first 100 since they could be entirely/mostly "open", > meaning you'd need to load another 100, and another, and another, until you > get > 100 closed. This will need to be stored _somewhere_ fwict. > > > [1] > > https://www.datacamp.com/community/tutorials/materialized-views-postgresql > > and noting that materialised views are not supported natively in mysql - > > but you've basically done https://linuxhint.com/materialized-views-mysql > > with the refresh logic in the application layer rather than a stored > > procedure. > > > > > diff --git patchwork/models.py patchwork/models.py > > > index 58e4c51e..7441510b 100644 > > > --- patchwork/models.py > > > +++ patchwork/models.py > > > @@ -14,6 +14,7 @@ from django.conf import settings > > > from django.contrib.auth.models import User > > > from django.core.exceptions import ValidationError > > > from django.db import models > > > +from django.db import transaction > > > from django.urls import reverse > > > from django.utils.functional import cached_property > > > from django.core.validators import validate_unicode_slug > > > @@ -494,6 +495,19 @@ class Patch(SubmissionMixin): > > > for tag in tags: > > > self._set_tag(tag, counter[tag]) > > > > > > +def refresh_series_state(self): > > > +if not self.series: > > > +return > > > + > > > +# If any of the patches in the series is in an "action required" > > > state, > > > +# then the series is still open > > > +# TODO(stephenfin): Can this still race? > > > +with transaction.atomic(): > > > +self.series.is_open = self.series.patches.filter( > > > +state__action_required=True > > > +).exists() > > > +self.series.save() > > > + > > > > AIUI based some experimentation and my experience dealing with our > > series race conditions in 2018, this can race and transactions won't > > save you here. The transaction will roll back all the changes if any > > statement fails (e.g. if you had an integrityerror from a foreign key), > > and other users won't see intermediate writes from your > > transaction. What it won't do is preserve the data dependency you're > > creating in the way a traditional mutex would. > > > > Basically, consider a concurrent parsemail and an API client changing a > > patch state an
Re: [PATCH] models: Add Series.is_open
On Sun, 2021-09-05 at 11:34 +1000, Daniel Axtens wrote: > Stephen Finucane writes: > > > Start making series more useful by tracking whether the state is open, > > meaning one or more patches are in a state with action_required=True, or > > not. This means that the 'git-pw series list' command will only list > > series that are actually actionable. > > > > Signed-off-by: Stephen Finucane > > --- > > Open questions: > > - Is there a more efficient way to do the migration than I've done? > > - Should we expose an 'is_open' boolean attribute of a 'state' (open, > > closed) attribute for the series views? > > - Is there a race condition possible in how I've implemented the series > > state checks? > > AIUI, series.is_open is a derived property (or I suppose in db terms a > 'view') of series.patches. > > Is the cost of computing it 'live' so bad that we need to cache it in > the db? (a 'materialised view'[1]) > > Is there any chance we could just always compute on demand? I'm trying > to think of all the ways we expose series: > > - on the patch list page, we already also fetch all the patches > > - on the API /series/ page, we also fetch all the patches even for just >the list view. > > so it seems to me we probably shouldn't suffer a performance hit for > doing it live. The biggest impact I see is filtering. This is the main thing I want because presently 'git pw series list' is pretty much useless without this. I don't doing this dynamically would work because attempting to fetch, for example, the first 100 "closed" series will require loading every series and every patch. You couldn't just load the first 100 since they could be entirely/mostly "open", meaning you'd need to load another 100, and another, and another, until you get 100 closed. This will need to be stored _somewhere_ fwict. > [1] https://www.datacamp.com/community/tutorials/materialized-views-postgresql > and noting that materialised views are not supported natively in mysql - > but you've basically done https://linuxhint.com/materialized-views-mysql > with the refresh logic in the application layer rather than a stored > procedure. > > > diff --git patchwork/models.py patchwork/models.py > > index 58e4c51e..7441510b 100644 > > --- patchwork/models.py > > +++ patchwork/models.py > > @@ -14,6 +14,7 @@ from django.conf import settings > > from django.contrib.auth.models import User > > from django.core.exceptions import ValidationError > > from django.db import models > > +from django.db import transaction > > from django.urls import reverse > > from django.utils.functional import cached_property > > from django.core.validators import validate_unicode_slug > > @@ -494,6 +495,19 @@ class Patch(SubmissionMixin): > > for tag in tags: > > self._set_tag(tag, counter[tag]) > > > > +def refresh_series_state(self): > > +if not self.series: > > +return > > + > > +# If any of the patches in the series is in an "action required" > > state, > > +# then the series is still open > > +# TODO(stephenfin): Can this still race? > > +with transaction.atomic(): > > +self.series.is_open = self.series.patches.filter( > > +state__action_required=True > > +).exists() > > +self.series.save() > > + > > AIUI based some experimentation and my experience dealing with our > series race conditions in 2018, this can race and transactions won't > save you here. The transaction will roll back all the changes if any > statement fails (e.g. if you had an integrityerror from a foreign key), > and other users won't see intermediate writes from your > transaction. What it won't do is preserve the data dependency you're > creating in the way a traditional mutex would. > > Basically, consider a concurrent parsemail and an API client changing a > patch state and refreshing series state on the same series: > > > | parsemail| API client| > > | | series.patch[2].save()| > | | BEGIN TRANSACTION | > | | is_open = SELECT(...) = False | > | series.patch[3].save() | | > | BEGIN TRANSACTION| |
[PATCH] models: Add Series.is_open
Start making series more useful by tracking whether the state is open,
meaning one or more patches are in a state with action_required=True, or
not. This means that the 'git-pw series list' command will only list
series that are actually actionable.
Signed-off-by: Stephen Finucane
---
Open questions:
- Is there a more efficient way to do the migration than I've done?
- Should we expose an 'is_open' boolean attribute of a 'state' (open,
closed) attribute for the series views?
- Is there a race condition possible in how I've implemented the series
state checks?
---
docs/api/schemas/latest/patchwork.yaml | 8 +++
docs/api/schemas/patchwork.j2 | 12
docs/api/schemas/v1.3/patchwork.yaml| 8 +++
patchwork/api/embedded.py | 7 +-
patchwork/api/filters.py| 7 +-
patchwork/api/series.py | 9 ++-
patchwork/migrations/0046_series_is_open.py | 58 +
patchwork/models.py | 20 ++
patchwork/tests/api/test_series.py | 52 +++
patchwork/tests/test_models.py | 72 +
10 files changed, 247 insertions(+), 6 deletions(-)
create mode 100644 patchwork/migrations/0046_series_is_open.py
create mode 100644 patchwork/tests/test_models.py
diff --git docs/api/schemas/latest/patchwork.yaml
docs/api/schemas/latest/patchwork.yaml
index e3bff990..cb8fcdfa 100644
--- docs/api/schemas/latest/patchwork.yaml
+++ docs/api/schemas/latest/patchwork.yaml
@@ -2260,6 +2260,10 @@ components:
description: >
Version of series as indicated by the subject prefix(es).
type: integer
+is_open:
+ title: Status
+ type: boolean
+ readOnly: true
total:
title: Total
description: >
@@ -2610,6 +2614,10 @@ components:
Version of series as indicated by the subject prefix(es).
type: integer
readOnly: true
+is_open:
+ title: Status
+ type: boolean
+ readOnly: true
mbox:
title: Mbox
type: string
diff --git docs/api/schemas/patchwork.j2 docs/api/schemas/patchwork.j2
index 3b4ad2f6..13610b84 100644
--- docs/api/schemas/patchwork.j2
+++ docs/api/schemas/patchwork.j2
@@ -2354,6 +2354,12 @@ components:
description: >
Version of series as indicated by the subject prefix(es).
type: integer
+{% if version >= (1, 3) %}
+is_open:
+ title: Status
+ type: boolean
+ readOnly: true
+{% endif %}
total:
title: Total
description: >
@@ -2718,6 +2724,12 @@ components:
Version of series as indicated by the subject prefix(es).
type: integer
readOnly: true
+{% if version >= (1, 3) %}
+is_open:
+ title: Status
+ type: boolean
+ readOnly: true
+{% endif %}
mbox:
title: Mbox
type: string
diff --git docs/api/schemas/v1.3/patchwork.yaml
docs/api/schemas/v1.3/patchwork.yaml
index 6cbba646..3b38799c 100644
--- docs/api/schemas/v1.3/patchwork.yaml
+++ docs/api/schemas/v1.3/patchwork.yaml
@@ -2260,6 +2260,10 @@ components:
description: >
Version of series as indicated by the subject prefix(es).
type: integer
+is_open:
+ title: Status
+ type: boolean
+ readOnly: true
total:
title: Total
description: >
@@ -2610,6 +2614,10 @@ components:
Version of series as indicated by the subject prefix(es).
type: integer
readOnly: true
+is_open:
+ title: Status
+ type: boolean
+ readOnly: true
mbox:
title: Mbox
type: string
diff --git patchwork/api/embedded.py patchwork/api/embedded.py
index 78316979..38a00ac4 100644
--- patchwork/api/embedded.py
+++ patchwork/api/embedded.py
@@ -181,11 +181,14 @@ class SeriesSerializer(SerializedRelatedField):
class Meta:
model = models.Series
-fields = ('id', 'url', 'web_url', 'date', 'name', 'version',
- 'mbox')
+fields = (
+'id', 'url', 'web_url', 'date', 'name', 'version', 'is_open',
+'mbox',
+)
read_only_fields = fields
versioned_fields = {
'1.1': ('web_url', ),
+'1.3': ('is_open', ),
}
extra_kwargs = {
'url': {'view_name': 'api-series-detail'},
diff --git patchwork/api/filters.py patchwor
[PATCH] README: Add Discord badge
Add a Discord badge that can be used to connect to the server. Link taken from daxtens' email to the list about the server [1]. [1] https://lists.ozlabs.org/pipermail/patchwork/2021-September/007226.html Signed-off-by: Stephen Finucane Cc: Daniel Axtens --- README.rst | 4 1 file changed, 4 insertions(+) diff --git README.rst README.rst index 7af45ce7..8d73ad78 100644 --- README.rst +++ README.rst @@ -18,6 +18,10 @@ Patchwork :target: http://patchwork.readthedocs.io/en/latest/?badge=latest :alt: Documentation Status +.. image:: https://img.shields.io/discord/591914197219016707.svg?label=&logo=discord&logoColor=ff&color=7389D8&labelColor=6A7EC2 + :target: https://discord.gg/hGWjXVTAbB + :alt: Discord + **Patchwork** is a patch tracking system for community-based projects. It is intended to make the patch management process easier for both the project's contributors and maintainers, leaving time for the more important (and more -- 2.31.1 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[RFC PATCH v2 10/19] templates: Convert user profile view
This is our first "full page", in that we have a navbar. A large number
of TODOs are left here as we're simply changing templates rather than
updating views, but those gaps will be closed in a future change.
Signed-off-by: Stephen Finucane
---
patchwork/templates/patchwork/login.html | 5 +-
patchwork/templates/patchwork/profile.html| 432 --
templates/base2.html | 82
.../registration/password_reset_confirm.html | 4 +
.../registration/password_reset_done.html | 4 +
.../registration/password_reset_form.html | 4 +
6 files changed, 395 insertions(+), 136 deletions(-)
diff --git patchwork/templates/patchwork/login.html
patchwork/templates/patchwork/login.html
index ef609f1f..b8ab462c 100644
--- patchwork/templates/patchwork/login.html
+++ patchwork/templates/patchwork/login.html
@@ -2,8 +2,7 @@
{% block title %}Sign in to Patchwork{% endblock %}
-{% block headers %}
-{% endblock %}
+{% block navigation %}{% endblock %}
{% block body %}
@@ -75,3 +74,5 @@ for (var i = 0; i < btns.length; i++) {
}
{% endblock %}
+
+{% block footer %}{% endblock %}
diff --git patchwork/templates/patchwork/profile.html
patchwork/templates/patchwork/profile.html
index 552dde47..7a0b54fe 100644
--- patchwork/templates/patchwork/profile.html
+++ patchwork/templates/patchwork/profile.html
@@ -1,173 +1,337 @@
-{% extends "base.html" %}
+{% extends "base2.html" %}
{% block title %}{{ user.username }}{% endblock %}
-{% block heading %}Your Profile{% endblock %}
{% block body %}
-Your Profile
+
+
+
+
+
+ Overview
+
+
+ Projects
+ Bundles
+ Todo List
+
+
+ Settings
+
+
+ Profile
+ Linked emails
+ Profile settings
+ Security
+
+
+
+
+
+
+Overview
+
+
+
+
+ #
+ Projects
+
{% if user.profile.maintainer_projects.count %}
-
- Maintainer of
+
+ Maintainer of
{% for project in user.profile.maintainer_projects.all %}
- {{
project.linkname }}{% if not forloop.last %},{% endif %}
+{{
project.linkname }}{% if not forloop.last %},{% endif %}
{% endfor %}.
-
+
{% endif %}
-
{% if user.profile.contributor_projects.count %}
-
- Contributor to
+
+ Contributor to
{% for project in user.profile.contributor_projects.all %}
- {{
project.linkname }}{% if not forloop.last %},{% endif %}
+ {{
project.linkname }}{% if not forloop.last %},{% endif %}
{% endfor %}.
-
+
{% endif %}
+
-
-
-Todo
-
- Your todo list contains patches that
- have been delegated to you.
+
+
+ #
+ Bundles
+
+{% if bundles %}
+You have the following bundle{{ bundles|length|pluralize }}:
+
+{% for bundle in bundles %}
+ {{ bundle.name
}}
+{% endfor %}
+
+
+ Visit the bundles page to
manage your bundles.
+
+{% else %}
+You have no bundles.
+{% endif %}
+
+
+
+
+ #
+ Todo List
+
+
+ Your todo list contains patches
that
+ have been delegated to you.
+
+
{% if user.profile.n_todo_patches %}
- Your have {{ user.profile.n_todo_patches }}
- patch{{ user.profile.n_todo_patches|pluralize:"es" }} in your todo list.
+ Your have {{ user.profile.n_todo_patches }}
+ patch{{ user.profile.n_todo_patches|pluralize:"es" }} in your todo
list.
{% else %}
- You have no patches in your todo list at present.
+ You have no patches in your todo list at present.
{% endif %}
-
-
+
+
-
-Linked email addresses
-
- The following email addresses are associated with this Patchwork account.
- Adding alternative addresses allows Patchwork to group contributions that
- you have made under different addresses.
-
-
- The "notify?" column allows you to opt-in or opt-out of automated
- Patchwork notification emails. Setting it to "no" will disable automated
- notifications for that address.
-
-
- Adding a new email address will send a confirmation email to that
address.
-
-
-
-email
-action
-notify?
-
+
+
+Settings
+
+
+{# TODO: Add view to enable this #}
+
+
+ #
+ Profile
+
+
+ {% csrf_token %}
+
+
+ Username
+
+
+
+
+
+
+
+ First name
+
+
+
[RFC PATCH v2 19/19] templates: Convert registration template
This is the last of our non submission pages. Next up, the main patch
and cover letter list/detail pages.
Signed-off-by: Stephen Finucane
---
patchwork/forms.py| 7 +-
.../templates/patchwork/registration.html | 211 +-
patchwork/views/user.py | 15 +-
3 files changed, 120 insertions(+), 113 deletions(-)
diff --git patchwork/forms.py patchwork/forms.py
index a975db18..b1b68179 100644
--- patchwork/forms.py
+++ patchwork/forms.py
@@ -32,6 +32,7 @@ class RegistrationForm(forms.Form):
User.objects.get(username__iexact=value)
except User.DoesNotExist:
return self.cleaned_data['username']
+
raise forms.ValidationError(
'This username is already taken. Please choose another.'
)
@@ -39,12 +40,12 @@ class RegistrationForm(forms.Form):
def clean_email(self):
value = self.cleaned_data['email']
try:
-user = User.objects.get(email__iexact=value)
+User.objects.get(email__iexact=value)
except User.DoesNotExist:
return self.cleaned_data['email']
+
raise forms.ValidationError(
-'This email address is already in use '
-'for the account "%s".\n' % user.username
+'This email address is already in use for another account.'
)
def clean(self):
diff --git patchwork/templates/patchwork/registration.html
patchwork/templates/patchwork/registration.html
index 8e2a3511..c9a2a94e 100644
--- patchwork/templates/patchwork/registration.html
+++ patchwork/templates/patchwork/registration.html
@@ -1,113 +1,112 @@
-{% extends "base.html" %}
+{% extends "base2.html" %}
{% block title %}Registration{% endblock %}
-{% block heading %}Registration{% endblock %}
+
+{% block navigation %}{% endblock %}
{% block body %}
-{% if confirmation and not error %}
-
- Registration successful!
-
-
- A confirmation email has been sent to {{ confirmation.email }}.
- You'll need to visit the link provided in that email to confirm your
- registration.
-
-{% else %}
-By creating a Patchwork account, you can:
-
- create "bundles" of patches
- update the state of your own patches
-
-
- {% csrf_token %}
-
-
- register
-
-{% if error %}
-
- {{ error }}
-
-{% endif %}
-
- {{ form.first_name.label_tag }}
-
-{% if form.first_name.errors %}
-{{ form.first_name.errors }}
-{% endif %}
-{{ form.first_name }}
-{% if form.first_name.help_text %}
-{{ form.first_name.help_text }}
-{% endif %}
-
-
-
- {{ form.last_name.label_tag }}
-
-{% if form.last_name.errors %}
-{{ form.last_name.errors }}
-{% endif %}
-{{ form.last_name }}
-{% if form.last_name.help_text %}
-{{ form.last_name.help_text }}
-{% endif %}
-
-
-
-
-
-Your name is used to identify you on the site
-
-
-
- {{ form.email.label_tag }}
-
-{% if form.email.errors %}
-{{ form.email.errors }}
+
+
+
+
+
+
+Create your Patchwork account
+
+{% if form.non_field_errors %}
+
+
+{{ form.non_field_errors }}
+
{% endif %}
-{{ form.email }}
-{% if form.email.help_text %}
-{{ form.email.help_text }}
-{% endif %}
-
-
-
-
-
-Patchwork will send a confirmation email to this address
-
-
-
- {{ form.username.label_tag }}
-
-{% if form.username.errors %}
-{{ form.username.errors }}
-{% endif %}
-{{ form.username }}
-{% if form.username.help_text %}
-{{ form.username.help_text }}
-{% endif %}
-
-
-
- {{ form.password.label_tag }}
-
-{% if form.password.errors %}
-{{ form.password.errors }}
-{% endif %}
-{{ form.password }}
-{% if form.password.help_text %}
-{{ form.password.help_text }}
-{% endif %}
-
-
-
-
-
-
-
-
-
+{% for message in messages %}
+{% if message.tags == 'success' %}
+
+{% elif message.tags == 'warning' %}
+
+{% elif message.tags == 'error' %}
+
+{% else %}
+
{% endif %}
+
+{{ message }}
+
+{% endfor %}
+
+{% csrf_token %}
+
+ Username
+
+
+
+
+
+
+{% for error in form.username.errors %}
+ {{ error }}
+{% endfor %}
+
+
+ Email
+
+
+
+
+
+
[RFC PATCH v2 11/19] templates: Enhance profile view further
Fill in the gaps intentionally missed previously by amalgamating most
user-specific views into the user profile view.
Signed-off-by: Stephen Finucane
---
patchwork/forms.py| 252 ++---
patchwork/templates/patchwork/profile.html| 86 +-
.../patchwork/user-link-confirm.html | 15 -
patchwork/templates/patchwork/user-link.html | 28 --
patchwork/tests/views/test_user.py| 52 ++--
patchwork/urls.py | 14 +-
patchwork/views/mail.py | 6 +-
patchwork/views/user.py | 263 +-
8 files changed, 513 insertions(+), 203 deletions(-)
delete mode 100644 patchwork/templates/patchwork/user-link-confirm.html
delete mode 100644 patchwork/templates/patchwork/user-link.html
diff --git patchwork/forms.py patchwork/forms.py
index 24322c78..5f8dff96 100644
--- patchwork/forms.py
+++ patchwork/forms.py
@@ -4,10 +4,12 @@
# SPDX-License-Identifier: GPL-2.0-or-later
from django.contrib.auth.models import User
+from django.core import exceptions
from django import forms
from django.db.models import Q
from django.db.utils import ProgrammingError
+from patchwork import models
from patchwork.models import Bundle
from patchwork.models import Patch
from patchwork.models import State
@@ -15,13 +17,14 @@ from patchwork.models import UserProfile
class RegistrationForm(forms.Form):
+
first_name = forms.CharField(max_length=30, required=False)
last_name = forms.CharField(max_length=30, required=False)
-username = forms.RegexField(regex=r'^\w+$', max_length=30,
-label=u'Username')
-email = forms.EmailField(max_length=100, label=u'Email address')
-password = forms.CharField(widget=forms.PasswordInput(),
- label='Password')
+username = forms.RegexField(
+regex=r'^\w+$', max_length=30, label='Username'
+)
+email = forms.EmailField(max_length=100, label='Email address')
+password = forms.CharField(widget=forms.PasswordInput(), label='Password')
def clean_username(self):
value = self.cleaned_data['username']
@@ -29,8 +32,9 @@ class RegistrationForm(forms.Form):
User.objects.get(username__iexact=value)
except User.DoesNotExist:
return self.cleaned_data['username']
-raise forms.ValidationError('This username is already taken. '
-'Please choose another.')
+raise forms.ValidationError(
+'This username is already taken. Please choose another.'
+)
def clean_email(self):
value = self.cleaned_data['email']
@@ -38,21 +42,24 @@ class RegistrationForm(forms.Form):
user = User.objects.get(email__iexact=value)
except User.DoesNotExist:
return self.cleaned_data['email']
-raise forms.ValidationError('This email address is already in use '
-'for the account "%s".\n' % user.username)
+raise forms.ValidationError(
+'This email address is already in use '
+'for the account "%s".\n' % user.username
+)
def clean(self):
return self.cleaned_data
-class EmailForm(forms.Form):
-email = forms.EmailField(max_length=200)
-
-
class BundleForm(forms.ModelForm):
+
name = forms.RegexField(
-regex=r'^[^/]+$', min_length=1, max_length=50, label=u'Name',
-error_messages={'invalid': 'Bundle names can\'t contain slashes'})
+regex=r'^[^/]+$',
+min_length=1,
+max_length=50,
+label='Name',
+error_messages={'invalid': 'Bundle names can\'t contain slashes'},
+)
class Meta:
model = Bundle
@@ -61,37 +68,180 @@ class BundleForm(forms.ModelForm):
class CreateBundleForm(BundleForm):
-def __init__(self, *args, **kwargs):
-super(CreateBundleForm, self).__init__(*args, **kwargs)
-
-class Meta:
-model = Bundle
-fields = ['name']
-
def clean_name(self):
name = self.cleaned_data['name']
-count = Bundle.objects.filter(owner=self.instance.owner,
- name=name).count()
+count = Bundle.objects.filter(
+owner=self.instance.owner, name=name
+).count()
if count > 0:
-raise forms.ValidationError('A bundle called %s already exists'
-% name)
+raise forms.ValidationError(
+'A bundle called %s already exists'
[RFC PATCH v2 18/19] templates: Convert mail settings pages
This one is rather tricky. We do a major overhaul of URLs and general
flow of confirmations, relying heavily on the messages framework to
avoid the need to have separate pages. Previously, configuring opt-in or
opt-out of email involved the following:
GET /mail/
POST /mail/
[some validation]
POST /mail/optout/ # or optin
If the last step threw an error, we'd stay on '/mail/optout/' or
'/mail/optin/' with a displayed error and tell the user to correct the
mistake. Now, we simply do this:
GET /mail/
POST /mail/
[some validation]
GET /mail//
POST /mail//
Error messages are propagated via the messages framework with all
non-field validation errors resulting in a redirect to the homepage.
Signed-off-by: Stephen Finucane
---
.../templates/patchwork/confirm-error.html| 23 ---
.../templates/patchwork/mail-configure.html | 70
.../templates/patchwork/mail-settings.html| 106 +++
patchwork/templates/patchwork/mail.html | 37
.../templates/patchwork/optin-request.html| 53 --
patchwork/templates/patchwork/optin.html | 21 ---
.../templates/patchwork/optout-request.html | 56 --
patchwork/templates/patchwork/optout.html | 25 ---
.../patchwork/registration-confirm.html | 14 --
patchwork/urls.py | 3 +-
patchwork/views/mail.py | 168 +++---
patchwork/views/notification.py | 35 ++--
patchwork/views/user.py | 56 +++---
13 files changed, 304 insertions(+), 363 deletions(-)
delete mode 100644 patchwork/templates/patchwork/confirm-error.html
create mode 100644 patchwork/templates/patchwork/mail-configure.html
delete mode 100644 patchwork/templates/patchwork/mail.html
delete mode 100644 patchwork/templates/patchwork/optin-request.html
delete mode 100644 patchwork/templates/patchwork/optin.html
delete mode 100644 patchwork/templates/patchwork/optout-request.html
delete mode 100644 patchwork/templates/patchwork/optout.html
delete mode 100644 patchwork/templates/patchwork/registration-confirm.html
diff --git patchwork/templates/patchwork/confirm-error.html
patchwork/templates/patchwork/confirm-error.html
deleted file mode 100644
index b1ce42ee..
--- patchwork/templates/patchwork/confirm-error.html
+++ /dev/null
@@ -1,23 +0,0 @@
-{% extends "base.html" %}
-
-{% block title %}Confirmation{% endblock %}
-{% block heading %}Confirmation{% endblock %}
-
-
-{% block body %}
-
-{% if error == 'inactive' %}
-
- This confirmation has already been processed; you've probably visited this
- page before.
-
-{% endif %}
-
-{% if error == 'expired' %}
-
- The confirmation has expired. If you'd still like to perform the
- {{ conf.get_type_display }} process, you'll need to resubmit the request.
-
-{% endif %}
-
-{% endblock %}
diff --git patchwork/templates/patchwork/mail-configure.html
patchwork/templates/patchwork/mail-configure.html
new file mode 100644
index ..c0e07154
--- /dev/null
+++ patchwork/templates/patchwork/mail-configure.html
@@ -0,0 +1,70 @@
+{% extends "base2.html" %}
+
+{% block title %}Mail settings{% endblock %}
+
+{% block body %}
+{% for message in messages %}
+{% if message.tags == 'success' %}
+
+{% elif message.tags == 'warning' %}
+
+{% elif message.tags == 'error' %}
+
+{% else %}
+
+{% endif %}
+ {{ message }}
+
+
+{% endfor %}
+
+
+
+
+ Mail settings for {{ email }}
+
+
+
+
+
+ You can configure your notification settings for Patchwork.
+ Use this if you wish to opt out of all email from Patchwork,
+ or if have previously opted out of Patchwork mail but now wish to
+ receive notifications from Patchwork.
+
+
+ If you opt out of email, Patchwork may still email you if you do certain
+ actions yourself (such as create a new Patchwork account) but will not
+ send you unsolicited email.
+
+
+ When you submit a request, an email will be sent to your address with
+ a link to click to finalise the request.
+ Patchwork does this to prevent someone modifying your mail settings
+ without your consent.
+
+
+{% if is_optout %}
+
+ Patchwork may not send automated notifications to this
address.
+
+{% else %}
+
+ Patchwork may send automated notifications to this
address.
+
+{% endif %}
+
+
+ {% csrf_token %}
+
+
+
+
+
+
+
+
+
+
+
+{% endblock %}
diff --git patchwork/templates/patchwork/mail-settings.html
patchwork/templates/patchwork/mail-settings.html
index 58f567ac..858140ae 100644
--- patchwork/templates/patchwork/mail-settings.html
+++ patchwork/templates/patchwork/mail-settings.html
@@ -1,37 +1,83 @@
-{% extends "base.html" %}
+{% extends &qu
[RFC PATCH v2 16/19] templates: Convert project view
Signed-off-by: Stephen Finucane
---
patchwork/forms.py | 83 +
patchwork/templates/patchwork/project.html | 407 +
patchwork/views/project.py | 81 +++-
3 files changed, 500 insertions(+), 71 deletions(-)
diff --git patchwork/forms.py patchwork/forms.py
index 5f8dff96..a975db18 100644
--- patchwork/forms.py
+++ patchwork/forms.py
@@ -244,6 +244,89 @@ class UserProfileForm(forms.ModelForm):
labels = {'show_ids': 'Show Patch IDs:'}
+class AddProjectMaintainerForm(forms.Form):
+
+name = 'add-maintainer'
+
+username = forms.RegexField(
+regex=r'^\w+$', max_length=30, label='Username'
+)
+
+def __init__(self, project, *args, **kwargs):
+self.project = project
+super().__init__(*args, **kwargs)
+
+def clean_username(self):
+value = self.cleaned_data['username']
+
+try:
+user = User.objects.get(username__iexact=value)
+except User.DoesNotExist:
+raise forms.ValidationError(
+'That username is not valid. Please choose another.'
+)
+
+if self.project in user.profile.maintainer_projects.all():
+raise forms.ValidationError(
+'That user is already a maintainer of this project.'
+)
+
+return value
+
+
+class RemoveProjectMaintainerForm(forms.Form):
+
+name = 'remove-maintainer'
+
+username = forms.RegexField(
+regex=r'^\w+$', max_length=30, label='Username'
+)
+
+def __init__(self, project, *args, **kwargs):
+self.project = project
+super().__init__(*args, **kwargs)
+
+def clean_username(self):
+value = self.cleaned_data['username']
+
+try:
+user = User.objects.get(username__iexact=value)
+except User.DoesNotExist:
+raise forms.ValidationError(
+'That username is not valid. Please choose another.'
+)
+
+maintainers = User.objects.filter(
+profile__maintainer_projects=self.project,
+).select_related('profile')
+
+if user not in maintainers:
+raise forms.ValidationError(
+'That user is not a maintainer of this project.'
+)
+
+# TODO(stephenfin): Should we prevent users removing themselves?
+
+if maintainers.count() <= 1:
+raise forms.ValidationError(
+'You cannot remove the only maintainer of the project.'
+)
+
+return value
+
+
+class ProjectSettingsForm(forms.ModelForm):
+
+name = 'project-settings'
+
+class Meta:
+model = models.Project
+fields = [
+'name', 'web_url', 'scm_url', 'webscm_url', 'list_archive_url',
+'list_archive_url_format', 'commit_url_format',
+]
+
+
def _get_delegate_qs(project, instance=None):
if instance and not project:
project = instance.project
diff --git patchwork/templates/patchwork/project.html
patchwork/templates/patchwork/project.html
index cad372f7..1b25bbe6 100644
--- patchwork/templates/patchwork/project.html
+++ patchwork/templates/patchwork/project.html
@@ -1,79 +1,348 @@
-{% extends "base.html" %}
+{% extends "base2.html" %}
{% block title %}{{ project.name }}{% endblock %}
-{% block info_active %}active{% endblock %}
{% block body %}
-About {{ project.name }}
-
-
-
-Name
-{{ project.name }}
-
-
-List address
-{{ project.listemail }}
-
-{% if project.list_archive_url %}
-
-List archive
-{{ project.list_archive_url
}}
-
+{% for message in messages %}
+{% if message.tags == 'success' %}
+
+{% elif message.tags == 'warning' %}
+
+{% elif message.tags == 'error' %}
+
+{% else %}
+
{% endif %}
-
-Maintainer{{ maintainers|length|pluralize }}
-
- {% for maintainer in maintainers %}
-{{ maintainer.profile.name }}
- <mailto:{{ maintainer.email }}">{{ maintainer.email
}}>
-
- {% endfor %}
-
-
-
-Patches
-{{ n_patches }} (+ {{ n_archived_patches }} archived)
-
-{% if project.web_url %}
-
-Website
-{{ project.web_url }}
-
+ {{ message }}
+
+
+{% endfor %}
+
+
+
+
+ About {{ project.name }}
+
+
+ {{ project.listemail }}
+
+
+
+
+
+
+
+ #
+ Overview
+
+
+
+
+
+
+{{ n_patches }}
+
+ Patches
+
+
+
+
+
+
+
+
+
+
+ Website
+
+
+
+
+
+
+
[RFC PATCH v2 13/19] templates: Convert bundles view
Signed-off-by: Stephen Finucane
---
patchwork/templates/patchwork/bundles.html | 137 -
1 file changed, 81 insertions(+), 56 deletions(-)
diff --git patchwork/templates/patchwork/bundles.html
patchwork/templates/patchwork/bundles.html
index cc2ebf90..c351a3cf 100644
--- patchwork/templates/patchwork/bundles.html
+++ patchwork/templates/patchwork/bundles.html
@@ -1,66 +1,91 @@
-{% extends "base.html" %}
-
-{% load static %}
+{% extends "base2.html" %}
{% block title %}Bundles{% endblock %}
-{% block bundle_active %}active{% endblock %}
{% block body %}
-Bundles
+
+
+Bundles
+
+
+
+
+ Bundles are groups of related patches.
+ You can create bundles by selecting patches from a project,
+ then using the 'create bundle' form to give your bundle a name.
+ Each bundle can be public or private;
+ public bundles are given a persistent URL,
+ based on your username and the name of the bundle.
+ Private bundles are only visible to you.
+
+
+
{% if bundles %}
-
-
-Bundle
-Project
-Public
-Patches
-Download
-Delete
-
+
+
+
+ Bundle
+ Project
+ Patches
+ Public
+ Actions
+
{% for bundle in bundles %}
-
-{{ bundle.name }}
-
-
-{{ bundle.project.linkname }}
-
-
-
- {% if bundle.public %}
-
- {% else %}
-
- {% endif %}
-
-{{ bundle.patches.count }}
-
-
-
-{% csrf_token %}
-{{ bundle.delete_form.as_p }}
-
-
-
-
-
-
-
-{% endfor %}
-
+
+ {% csrf_token %}
+
+
+
+
+
+{{ bundle.name }}
+
+
+
+ {{ bundle.project.name }}
+
+
+ {{ bundle.patches.count }}
+
+{% if bundle.public %}
+
+
+
+{% else %}
+
+
+
{% endif %}
-
-
- Bundles are groups of related patches. You can create bundles by
- selecting patches from a project, then using the 'create bundle' form
- to give your bundle a name. Each bundle can be public or private; public
- bundles are given a persistent URL, based you your username and the name
- of the bundle. Private bundles are only visible to you.
-
-
-{% if not bundles %}
-You have no bundles.
+
+
+
+
+ Download
+
+
+
+ Delete
+
+
+
+{% endfor %}
+
+
+{% else %}
+
+
+
+
+
+
+
+
+ You have no bundles.
+
+
+
{% endif %}
+
+
{% endblock %}
--
2.31.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[RFC PATCH v2 14/19] templates: Convert projects view
Signed-off-by: Stephen Finucane
---
patchwork/templates/patchwork/projects.html | 83 +
1 file changed, 50 insertions(+), 33 deletions(-)
diff --git patchwork/templates/patchwork/projects.html
patchwork/templates/patchwork/projects.html
index 16b1bc5d..d645fdbc 100644
--- patchwork/templates/patchwork/projects.html
+++ patchwork/templates/patchwork/projects.html
@@ -1,38 +1,55 @@
-{% extends "base.html" %}
+{% extends "base2.html" %}
+
+{% block title %}Projects{% endblock %}
-{% block title %}Project List{% endblock %}
{% block body %}
-
-{% if projects %}
-{% for p in projects %}
-{% cycle '' '' '' %}
-
-
-
-{{ p.name }}
-
-
-
- View patches
-
+
+
+Projects
+
+
+
+{% for project in projects %}
+
+
+
+ {{ project.name }}
+
+
+ ({{ project.listemail }})
+
+
+
+
+
+
+About
+
+
+
+
+
+
+
+View patches
+
+
+
+
+{% empty %}
+
+
+
+
+
-
-{% if p.web_url %}
- {{ p.web_url }}
-{% endif %}
-{% if p.webscm_url %}
- {{ p.webscm_url }}
-{% endif %}
-
-
-
-{% if forloop.last %}
-
-{% else %}
-{% cycle '' '' '' %}
-{% endif %}
+
+
+ Patchwork doesn't have any projects to display.
+
+
+
{% endfor %}
-{% else %}
-Patchwork doesn't have any projects to display!
-{% endif %}
+
+
{% endblock %}
--
2.31.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[RFC PATCH v2 12/19] view: Simplify user opt-in, opt-out
If we already have a user account associated with an email address, why
make the user jump through hoops when opting in or out of emails. It's
silly.
Signed-off-by: Stephen Finucane
---
patchwork/views/user.py | 78 +
1 file changed, 16 insertions(+), 62 deletions(-)
diff --git patchwork/views/user.py patchwork/views/user.py
index d1a1180e..440aa38a 100644
--- patchwork/views/user.py
+++ patchwork/views/user.py
@@ -103,50 +103,6 @@ def register_confirm(request, conf):
return render(request, 'patchwork/registration-confirm.html')
-def _opt_in(request, email):
-conf = EmailConfirmation(type='optin', email=email)
-conf.save()
-
-context = {'confirmation': conf}
-subject = render_to_string('patchwork/mails/optin-request-subject.txt')
-message = render_to_string(
-'patchwork/mails/optin-request.txt', context, request=request)
-
-try:
-send_mail(subject, message, settings.DEFAULT_FROM_EMAIL, [email])
-except smtplib.SMTPException:
-messages.error(
-request,
-'An error occurred while submitting this request. '
-'Please contact an administrator.'
-)
-return False
-
-return True
-
-
-def _opt_out(request, email):
-conf = EmailConfirmation(type='optout', email=email)
-conf.save()
-
-context = {'confirmation': conf}
-subject = render_to_string('patchwork/mails/optout-request-subject.txt')
-message = render_to_string(
-'patchwork/mails/optout-request.txt', context, request=request)
-
-try:
-send_mail(subject, message, settings.DEFAULT_FROM_EMAIL, [email])
-except smtplib.SMTPException:
-messages.error(
-request,
-'An error occurred while submitting this request. '
-'Please contact an administrator.'
-)
-return False
-
-return True
-
-
def _send_confirmation_email(request, email):
conf = EmailConfirmation(type='userperson', user=request.user, email=email)
conf.save()
@@ -228,30 +184,28 @@ def profile(request):
user_email_optin_form = forms.UserEmailOptinForm(
user=request.user, data=request.POST)
if user_email_optin_form.is_valid():
-if _opt_in(
-request, user_email_optin_form.cleaned_data['email'],
-):
-messages.success(
-request,
-'Requested opt-in to email from Patchwork. '
-'Check your email for confirmation.',
-)
-return HttpResponseRedirect(reverse('user-profile'))
+EmailOptout.objects.filter(
+email=user_email_optin_form.cleaned_data['email'],
+).delete()
+messages.success(
+request,
+'Opt-in into email from Patchwork.'
+)
+return HttpResponseRedirect(reverse('user-profile'))
messages.error(request, 'Error opting into email.')
elif form_name == forms.UserEmailOptoutForm.name:
user_email_optout_form = forms.UserEmailOptoutForm(
user=request.user, data=request.POST)
if user_email_optout_form.is_valid():
-if _opt_out(
-request, user_email_optout_form.cleaned_data['email'],
-):
-messages.success(
-request,
-'Requested opt-out from email from Patchwork. '
-'Check your email for confirmation.',
-)
-return HttpResponseRedirect(reverse('user-profile'))
+EmailOptout(
+email=user_email_optout_form.cleaned_data['email'],
+).save()
+messages.success(
+request,
+'Opted-out from email from Patchwork.'
+)
+return HttpResponseRedirect(reverse('user-profile'))
messages.error(request, 'Error opting out of email.')
elif form_name == UserForm.name:
--
2.31.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[RFC PATCH v2 15/19] WIP: templates: Convert todo-list view
Signed-off-by: Stephen Finucane
---
patchwork/templates/patchwork/todo-list.html | 43
1 file changed, 35 insertions(+), 8 deletions(-)
diff --git patchwork/templates/patchwork/todo-list.html
patchwork/templates/patchwork/todo-list.html
index 6c656b10..2bfddaee 100644
--- patchwork/templates/patchwork/todo-list.html
+++ patchwork/templates/patchwork/todo-list.html
@@ -1,17 +1,44 @@
-{% extends "base.html" %}
+{% extends "base2.html" %}
{% load person %}
-{% block title %}{{ user }}'s todo list{% endblock %}
+{% block title %}Todo list{% endblock %}
{% block body %}
-TODO
+
+
+Todo list
+
-
- A Patchwork todo-list contains patches that are assigned to you, are in an
- "action required" state ({{ action_required_states|join:", " }}), and are
- not archived.
-
+
+
+ Your Patchwork todo list contains patches that are assigned to you,
+ are in an "action required" state ({{ action_required_states|join:", "
}}),
+ and are not archived.
+
+
+
+
+{% if patches %}
+{% for patch in patches %}
+{% endfor %}
+{% else %}
+
+
+
+
+
+
+
+
+ You have no pending reviews.
+
+
+
+{% endif %}
+
+
+
{% include "patchwork/partials/patch-list.html" %}
{% endblock %}
--
2.31.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[RFC PATCH v2 17/19] templates: Convert about page
Another one bites the dust.
Signed-off-by: Stephen Finucane
---
patchwork/templates/patchwork/about.html | 147 ++-
1 file changed, 91 insertions(+), 56 deletions(-)
diff --git patchwork/templates/patchwork/about.html
patchwork/templates/patchwork/about.html
index 43f3110b..cdd8cdc5 100644
--- patchwork/templates/patchwork/about.html
+++ patchwork/templates/patchwork/about.html
@@ -1,70 +1,105 @@
-{% extends "base.html" %}
+{% extends "base2.html" %}
{% block title %}About{% endblock %}
-{% block heading %}About Patchwork{% endblock %}
{% block body %}
-
- About Patchwork
+
+
+
+ About Patchwork
+
- Patchwork is free software, and is available from the http://jk.ozlabs.org/projects/patchwork/";>Patchwork website.
- Documentation is available on http://patchwork.readthedocs.io/";>Read the Docs.
+
+ Patchwork is a patch tracking system for community-based projects.
+ It is intended to make the patch management process easier for both the
+ project's contributors and maintainers, leaving time for the more
+ important (and more interesting) stuff.
+
- Patchwork is built on the https://djangoproject.com/";>Django
- web framework using https://getbootstrap.com/";>Bootstrap.
+
+ Patchwork is free software, and is available from the
+ https://github.com/getpatchwork/patchwork/";>Patchwork
website.
+ Documentation is available on
+ http://patchwork.readthedocs.io/";>Read the Docs.
+
+
-
-
- Version
+
+
+ #
+ Status
+
+
+
+
+
+
+Version
+
+
+{{ version }}
+
+
+
+
+
+
+
+
+
+REST API
+
+
+{% if enabled_apis.rest %}
+enabled
+{% else %}
+disabled
+{% endif %}
+
+
+
-
-
-{{ version }}
-
-
-
- {% if admins %}
-
-
- Administrators
+
+
+
+
+XML-RPC API
+
+
+{% if enabled_apis.xmlrpc %}
+enabled
+{% else %}
+disabled
+{% endif %}
+
+
+
-
- {% for admin in admins %}
-
-mailto:{{ admin.1 }}">{{ admin.0 }}
-
- {% endfor %}
-
-
- {% endif %}
-
-
- API Status
+
+
+
+
+ #
+ Administrators
+
+{% for admin in admins %}
+
+
+
+
+{{ admin.0 }}
+
+
+{{ admin.1 }}
+
+
+
-
-
-REST
-
-{% if enabled_apis.rest %}
-enabled
-{% else %}
-disabled
-{% endif %}
-
-
-XML-RPC
-
-{% if enabled_apis.xmlrpc %}
-enabled
-{% else %}
-disabled
-{% endif %}
-
-
-
+{% empty %}
+This instance has no administrators.
+{% endfor %}
+
{% endblock %}
--
2.31.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[RFC PATCH v2 08/19] trivial: Run views through black
Do this before conducting major surgery on these views.
Signed-off-by: Stephen Finucane
---
patchwork/views/__init__.py | 107
patchwork/views/api.py | 13 +++--
patchwork/views/bundle.py | 71 +++-
patchwork/views/cover.py| 42 --
patchwork/views/mail.py | 24 +---
patchwork/views/patch.py| 90 +++---
patchwork/views/project.py | 7 ++-
patchwork/views/pwclient.py | 5 +-
patchwork/views/series.py | 3 +-
patchwork/views/user.py | 96 +++-
patchwork/views/utils.py| 27 +
patchwork/views/xmlrpc.py | 63 ++---
12 files changed, 350 insertions(+), 198 deletions(-)
diff --git patchwork/views/__init__.py patchwork/views/__init__.py
index 3efe90cd..c3199ffd 100644
--- patchwork/views/__init__.py
+++ patchwork/views/__init__.py
@@ -124,8 +124,7 @@ def set_bundle(request, project, action, data, patches,
context):
if Bundle.objects.filter(owner=user, name=bundle_name).count() > 0:
return ['You already have a bundle called "%s"' % bundle_name]
-bundle = Bundle(owner=user, project=project,
-name=bundle_name)
+bundle = Bundle(owner=user, project=project, name=bundle_name)
bundle.save()
messages.success(request, "Bundle %s created" % bundle.name)
elif action == 'add':
@@ -138,15 +137,22 @@ def set_bundle(request, project, action, data, patches,
context):
for patch in patches:
if action in ['create', 'add']:
-bundlepatch_count = BundlePatch.objects.filter(bundle=bundle,
- patch=patch).count()
+bundlepatch_count = BundlePatch.objects.filter(
+bundle=bundle, patch=patch
+).count()
if bundlepatch_count == 0:
bundle.append_patch(patch)
-messages.success(request, "Patch '%s' added to bundle %s" %
- (patch.name, bundle.name))
+messages.success(
+request,
+"Patch '%s' added to bundle %s"
+% (patch.name, bundle.name),
+)
else:
-messages.warning(request, "Patch '%s' already in bundle %s" %
- (patch.name, bundle.name))
+messages.warning(
+request,
+"Patch '%s' already in bundle %s"
+% (patch.name, bundle.name),
+)
elif action == 'remove':
try:
bp = BundlePatch.objects.get(bundle=bundle, patch=patch)
@@ -156,16 +162,24 @@ def set_bundle(request, project, action, data, patches,
context):
else:
messages.success(
request,
-"Patch '%s' removed from bundle %s\n" % (patch.name,
- bundle.name))
+"Patch '%s' removed from bundle %s\n"
+% (patch.name, bundle.name),
+)
bundle.save()
return []
-def generic_list(request, project, view, view_args=None, filter_settings=None,
- patches=None, editable_order=False):
+def generic_list(
+request,
+project,
+view,
+view_args=None,
+filter_settings=None,
+patches=None,
+editable_order=False,
+):
if not filter_settings:
filter_settings = []
@@ -198,13 +212,16 @@ def generic_list(request, project, view, view_args=None,
filter_settings=None,
data = request.POST
order = Order(data.get('order'), editable=editable_order)
-context.update({
-'order': order,
-'list_view': {
-'view': view,
-'view_params': view_args or {},
-'params': params
-}})
+context.update(
+{
+'order': order,
+'list_view': {
+'view': view,
+'view_params': view_args or {},
+'params': params,
+},
+}
+)
# form processing
@@ -240,8 +257,9 @@ def generic_list(request, project, view, view_args=None,
filter_settings=None,
errors = set_bundle(request, project, action, data, ps, context)
elif properties_form and action == properties_form.action:
-errors = process_multiplepatch_form(request, properties_form,
-action, ps, context)
+error
[RFC PATCH v2 09/19] templates: Convert login, password reset views
The only things off note here are that we're removing the
'password_reset_complete.html' template in favour of redirecting to the
login page, and that we're opting to generate the forms ourselves rather
than relying on forms generated by Django which are too difficult to
style. This might actually help us in the future if/when we move to a
AJAX-driven UI.
Signed-off-by: Stephen Finucane
---
patchwork/templates/patchwork/login.html | 96 +++-
patchwork/urls.py | 4 +-
templates/base2.html | 15 +++
.../registration/password_reset_complete.html | 8 --
.../registration/password_reset_confirm.html | 104 +++---
.../registration/password_reset_done.html | 35 --
.../registration/password_reset_form.html | 75 +++--
7 files changed, 218 insertions(+), 119 deletions(-)
create mode 100644 templates/base2.html
delete mode 100644 templates/registration/password_reset_complete.html
diff --git patchwork/templates/patchwork/login.html
patchwork/templates/patchwork/login.html
index 86111342..ef609f1f 100644
--- patchwork/templates/patchwork/login.html
+++ patchwork/templates/patchwork/login.html
@@ -1,37 +1,77 @@
-{% extends "base.html" %}
+{% extends "base2.html" %}
{% block title %}Sign in to Patchwork{% endblock %}
-{% block heading %}Sign in to Patchwork{% endblock %}
{% block headers %}
-
-$(function() {
-$('#id_username').focus()
-});
-
{% endblock %}
{% block body %}
-
-{% csrf_token %}
-
-
- login
-
-{% if error %}
-
- {{ error }}
-
+
+
+
+
+
+
+Sign in to Patchwork
+
+{% if form.non_field_errors %}
+
+
+{{ form.non_field_errors }}
+
{% endif %}
-{{ form }}
-
-
-
-
-
-Forgot password?
-
-
-
-
+
+{% csrf_token %}
+
+ Username
+
+
+
+
+
+
+{% for error in form.username.errors %}
+ {{ error }}
+{% endfor %}
+
+
+ Password
+
+
+
+
+
+
+{% for error in form.password.errors %}
+ {{ error }}
+{% endfor %}
+
+
+
+Login
+
+
+
+
+Sign Up
+ •
+Forgot Password
+
+
+
+
+
+
+
+
+var btns = document.getElementsByClassName('delete')
+
+for (var i = 0; i < btns.length; i++) {
+ console.log('got button');
+ btns[i].addEventListener('click', function(e) {
+console.log('hello');
+this.parentNode.remove(); //this refers to the current target element
+ }, false);
+}
+
{% endblock %}
diff --git patchwork/urls.py patchwork/urls.py
index 5ddf2dbd..a7dfc3d3 100644
--- patchwork/urls.py
+++ patchwork/urls.py
@@ -153,7 +153,9 @@ urlpatterns = [
),
path(
'user/password-reset///',
-auth_views.PasswordResetConfirmView.as_view(),
+auth_views.PasswordResetConfirmView.as_view(
+success_url=reverse_lazy('auth_login'),
+),
name='password_reset_confirm',
),
path(
diff --git templates/base2.html templates/base2.html
new file mode 100644
index ..ac6b43bc
--- /dev/null
+++ templates/base2.html
@@ -0,0 +1,15 @@
+{% load static %}
+
+
+
+
+
+{% block title %}Patchwork{% endblock %} - Patchwork
+
+
+{% block headers %}{% endblock %}
+
+
+{% block body %}{% endblock %}
+
+
diff --git templates/registration/password_reset_complete.html
templates/registration/password_reset_complete.html
deleted file mode 100644
index 8678ee89..
--- templates/registration/password_reset_complete.html
+++ /dev/null
@@ -1,8 +0,0 @@
-{% extends "base.html" %}
-
-{% block title %}Password reset completed{% endblock %}
-{% block heading %}Password reset completed{% endblock %}
-
-{% block body %}
-Your password has been set. You may go ahead and log in now.
-{% endblock %}
diff --git templates/registration/password_reset_confirm.html
templates/registration/password_reset_confirm.html
index 4ab2357f..1c91eb1b 100644
--- templates/registration/password_reset_confirm.html
+++ templates/registration/password_reset_confirm.html
@@ -1,49 +1,77 @@
-{% extends "base.html" %}
+{% extends "base2.html" %}
{% block title %}Password reset confirmation{% endblock %}
{% block heading %}Password reset confirmation{% endblock %}
{% block body %}
+
+
+
+
+
{% if validlink %}
-
- Your username, in case you
[RFC PATCH v2 05/19] settings: Enable django-cors-headers
This is proving very useful as I attempt to add some Vue.js components
to the frontend.
Signed-off-by: Stephen Finucane
---
patchwork/settings/dev.py | 26 +++---
requirements-dev.txt | 1 +
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git patchwork/settings/dev.py patchwork/settings/dev.py
index aa3ee3c5..62ae7004 100644
--- patchwork/settings/dev.py
+++ patchwork/settings/dev.py
@@ -19,6 +19,11 @@ try:
except ImportError:
debug_toolbar = None
+try:
+import corsheaders
+except ImportError:
+corsheaders = None
+
#
# Core settings
# https://docs.djangoproject.com/en/2.2/ref/settings/#core-settings
@@ -81,13 +86,13 @@ if debug_toolbar:
'debug_toolbar'
]
-DEBUG_TOOLBAR_PATCH_SETTINGS = False
-
-# This should go first in the middleware classes
+# This should go as high as possible in the middleware classes
MIDDLEWARE = [
'debug_toolbar.middleware.DebugToolbarMiddleware',
] + MIDDLEWARE
+DEBUG_TOOLBAR_PATCH_SETTINGS = False
+
INTERNAL_IPS = [
'127.0.0.1', '::1',
'172.18.0.1'
@@ -102,6 +107,21 @@ if dbbackup:
DBBACKUP_STORAGE_OPTIONS = {'location': '.backups'}
+# django-cors-headers
+
+if corsheaders:
+INSTALLED_APPS += [
+'corsheaders',
+]
+
+# This should go as high as possible in the middleware classes
+MIDDLEWARE = [
+'corsheaders.middleware.CorsMiddleware',
+] + MIDDLEWARE
+
+CORS_ORIGIN_ALLOW_ALL = True
+CORS_EXPOSE_HEADERS = ['Link']
+
#
# Patchwork settings
#
diff --git requirements-dev.txt requirements-dev.txt
index d58dfaa9..229ed0c2 100644
--- requirements-dev.txt
+++ requirements-dev.txt
@@ -3,4 +3,5 @@ djangorestframework~=3.12.0
django-filter~=2.4.0
django-debug-toolbar~=3.2.0
django-dbbackup~=3.3.0
+django-cors-headers~=3.2.0
-r requirements-test.txt
--
2.31.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[RFC PATCH v2 06/19] WIP: REST: Change permissions for '/people', '/users'
This needs to be versioned. It's the correct thing to do though. Signed-off-by: Stephen Finucane --- patchwork/api/person.py | 4 ++-- patchwork/api/user.py | 5 - 2 files changed, 6 insertions(+), 3 deletions(-) diff --git patchwork/api/person.py patchwork/api/person.py index c806c0dd..59d74a56 100644 --- patchwork/api/person.py +++ patchwork/api/person.py @@ -6,7 +6,7 @@ from rest_framework.serializers import HyperlinkedModelSerializer from rest_framework.generics import ListAPIView from rest_framework.generics import RetrieveAPIView -from rest_framework.permissions import IsAuthenticated +from rest_framework.permissions import IsAuthenticatedOrReadOnly from patchwork.api.embedded import UserSerializer from patchwork.models import Person @@ -27,7 +27,7 @@ class PersonSerializer(HyperlinkedModelSerializer): class PersonMixin(object): -permission_classes = (IsAuthenticated,) +permission_classes = (IsAuthenticatedOrReadOnly,) serializer_class = PersonSerializer def get_queryset(self): diff --git patchwork/api/user.py patchwork/api/user.py index 4ea2322e..d629a7aa 100644 --- patchwork/api/user.py +++ patchwork/api/user.py @@ -84,7 +84,10 @@ class UserDetailSerializer(UserListSerializer): class UserMixin(object): queryset = User.objects.all() -permission_classes = (permissions.IsAuthenticated, IsOwnerOrReadOnly) +permission_classes = ( +permissions.IsAuthenticatedOrReadOnly, +IsOwnerOrReadOnly, +) class UserList(UserMixin, ListAPIView): -- 2.31.1 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[RFC PATCH v2 03/19] templatetags: Add 'site_admin' tag
This lets us avoid complex templating.
Signed-off-by: Stephen Finucane
---
.../templates/patchwork/optin-request.html| 8 ++---
.../templates/patchwork/optout-request.html | 8 ++---
patchwork/templatetags/admins.py | 29 +++
3 files changed, 35 insertions(+), 10 deletions(-)
create mode 100644 patchwork/templatetags/admins.py
diff --git patchwork/templates/patchwork/optin-request.html
patchwork/templates/patchwork/optin-request.html
index 3384c462..36744c26 100644
--- patchwork/templates/patchwork/optin-request.html
+++ patchwork/templates/patchwork/optin-request.html
@@ -1,5 +1,7 @@
{% extends "base.html" %}
+{% load admins %}
+
{% block title %}Opt-in{% endblock %}
{% block heading %}Opt-in{% endblock %}
@@ -40,11 +42,7 @@
{% if error and admins %}
- If you are having trouble opting in, please email
-{% for admin in admins %}
-{% if admins|length > 1 and forloop.last %} or {% endif %}
-{{ admin.0 }} <mailto:{{ admin.1 }}">{{ admin.1 }}>{% if
admins|length > 2 and not forloop.last %}, {% endif %}
-{% endfor %}
+ If you are having trouble opting in, please email {% site_admins %}.
{% endif %}
{% endif %}
diff --git patchwork/templates/patchwork/optout-request.html
patchwork/templates/patchwork/optout-request.html
index 7396fd36..a89f72bb 100644
--- patchwork/templates/patchwork/optout-request.html
+++ patchwork/templates/patchwork/optout-request.html
@@ -1,5 +1,7 @@
{% extends "base.html" %}
+{% load admins %}
+
{% block title %}Opt-out{% endblock %}
{% block heading %}Opt-out{% endblock %}
@@ -42,11 +44,7 @@
{% if error and admins %}
- If you are having trouble opting out, please email
-{% for admin in admins %}
-{% if admins|length > 1 and forloop.last %} or {% endif %}
-{{ admin.0 }} <mailto:{{ admin.1 }}">{{ admin.1 }}>{% if
admins|length > 2 and not forloop.last %}, {% endif %}
-{% endfor %}
+ If you are having trouble opting out, please email {% site_admins %}.
{% endif %}
{% endif %}
diff --git patchwork/templatetags/admins.py patchwork/templatetags/admins.py
new file mode 100644
index ..3809655c
--- /dev/null
+++ patchwork/templatetags/admins.py
@@ -0,0 +1,29 @@
+# Patchwork - automated patch tracking system
+# Copyright (C) 2020 Stephen Finucane
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+from django.conf import settings
+from django import template
+from django.utils.safestring import mark_safe
+
+
+register = template.Library()
+
+
+@register.simple_tag()
+def site_admins():
+admins = [
+f'{admin[0]} <mailto:{admin[1]}";>{admin[1]}>'
+for admin in settings.ADMINS
+]
+
+if not admins:
+return ''
+
+if len(admins) == 1:
+return mark_safe(admins[0])
+
+return mark_safe(
+', '.join(admins[:-2]) + admins[-2] + ' or ' + admins[-1]
+)
--
2.31.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[RFC PATCH v2 01/19] templates: Use standard indentation
Use a standard two space indentation across files, rewrapping some stuff
as we go along.
Signed-off-by: Stephen Finucane
---
patchwork/templates/patchwork/about.html | 6 +-
patchwork/templates/patchwork/bundle.html | 37 +-
patchwork/templates/patchwork/bundles.html| 84 +--
.../templates/patchwork/confirm-error.html| 12 +-
patchwork/templates/patchwork/list.html | 2 +-
patchwork/templates/patchwork/login.html | 40 +-
.../templates/patchwork/mail-settings.html| 47 +-
patchwork/templates/patchwork/mail.html | 45 +-
.../templates/patchwork/optin-request.html| 50 +-
patchwork/templates/patchwork/optin.html | 21 +-
.../templates/patchwork/optout-request.html | 52 +-
patchwork/templates/patchwork/optout.html | 28 +-
.../patchwork/partials/download-buttons.html | 34 +-
.../templates/patchwork/partials/filters.html | 83 +--
.../patchwork/partials/pagination.html| 42 +-
.../patchwork/partials/patch-list.html| 479 +-
patchwork/templates/patchwork/profile.html| 271 +-
patchwork/templates/patchwork/project.html| 98 ++--
patchwork/templates/patchwork/projects.html | 16 +-
.../patchwork/registration-confirm.html | 9 +-
.../templates/patchwork/registration.html | 160 +++---
patchwork/templates/patchwork/submission.html | 400 +++
patchwork/templates/patchwork/todo-list.html | 8 +-
patchwork/templates/patchwork/todo-lists.html | 31 +-
.../patchwork/user-link-confirm.html | 10 +-
patchwork/templates/patchwork/user-link.html | 32 +-
templates/404.html| 2 -
templates/base.html | 252 -
.../registration/password_change_done.html| 2 -
.../registration/password_change_form.html| 52 +-
.../registration/password_reset_complete.html | 3 +-
.../registration/password_reset_confirm.html | 62 ++-
.../registration/password_reset_done.html | 14 +-
.../registration/password_reset_email.html| 2 +-
.../registration/password_reset_form.html | 52 +-
35 files changed, 1268 insertions(+), 1270 deletions(-)
diff --git patchwork/templates/patchwork/about.html
patchwork/templates/patchwork/about.html
index 210e9513..43f3110b 100644
--- patchwork/templates/patchwork/about.html
+++ patchwork/templates/patchwork/about.html
@@ -48,8 +48,7 @@
REST
-
+
{% if enabled_apis.rest %}
enabled
{% else %}
@@ -58,8 +57,7 @@
XML-RPC
-
+
{% if enabled_apis.xmlrpc %}
enabled
{% else %}
diff --git patchwork/templates/patchwork/bundle.html
patchwork/templates/patchwork/bundle.html
index 411c18b5..fc87eac4 100644
--- patchwork/templates/patchwork/bundle.html
+++ patchwork/templates/patchwork/bundle.html
@@ -4,37 +4,34 @@
{% load static %}
{% block headers %}
-
-
+
+
{% endblock %}
-{% block title %}{{project.name}}{% endblock %}
+{% block title %}{{ project.name }}{% endblock %}
{% block body %}
Bundle
-This bundle contains patches for the {{ bundle.project.linkname }}
-project.
+This bundle contains patches for the {{ bundle.project.linkname }}
project.
Download bundle as mbox
{% if bundleform %}
- {% csrf_token %}
-
-
-
-
- Bundle settings
-
-
+ {% csrf_token %}
+
+
+
+ Bundle settings
+
{{ bundleform }}
-
-
-
-
-
-
-
+
+
+
+
+
+
+
diff --git patchwork/templates/patchwork/bundles.html
patchwork/templates/patchwork/bundles.html
index 1bb3b0da..cc2ebf90 100644
--- patchwork/templates/patchwork/bundles.html
+++ patchwork/templates/patchwork/bundles.html
@@ -10,53 +10,55 @@
{% if bundles %}
-
- Bundle
- Project
- Public
- Patches
- Download
- Delete
-
+
+Bundle
+Project
+Public
+Patches
+Download
+Delete
+
{% for bundle in bundles %}
-
- {{ bundle.name }}
-
-
-{{ bundle.project.linkname }}
-
-
-
- {% if bundle.public %}
-
- {% else %}
-
- {% endif %}
-
- {{ bundle.patches.count }}
-
-
-
-{% csrf_token %}
-{{ bundle.delete_form.as_p }}
-
-
-
-
-
+
+{{ bundle.name }}
+
+
+{{ bundle.project.linkname }}
+
+
+
+ {% if bundle.public %}
+
+ {% else %}
+
+ {% endif %}
+
+{{ bundle.patches.count }}
+
+
+
+{% csrf_token %}
+{{ bundle.delete_form.as_p }}
+
+
+
+
+
-
+
{% endfor %}
{% endif %}
-Bundles are groups of related patches. You can create bundles by
-selecting patches from a project, then using the 'create bundle' form
-to give your bundle a name. Each bundle can be public or private; public
-bundles are given a persistent URL, based you your username and the name
-of
[RFC PATCH v2 04/19] REST: Include 'first', 'last' refs in 'Link' header
I've no idea why this wasn't done from day one, but it's a huge
usability win for anyone attempting to do pagination with this header.
Signed-off-by: Stephen Finucane
---
patchwork/api/base.py | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git patchwork/api/base.py patchwork/api/base.py
index d870a511..7baac275 100644
--- patchwork/api/base.py
+++ patchwork/api/base.py
@@ -12,6 +12,7 @@ from rest_framework.pagination import PageNumberPagination
from rest_framework.response import Response
from rest_framework.serializers import HyperlinkedIdentityField
from rest_framework.serializers import HyperlinkedModelSerializer
+from rest_framework.utils.urls import replace_query_param
from patchwork.api import utils
@@ -59,19 +60,33 @@ class LinkHeaderPagination(PageNumberPagination):
max_page_size = settings.MAX_REST_RESULTS_PER_PAGE
page_size_query_param = 'per_page'
+def get_first_link(self):
+url = self.request.build_absolute_uri()
+return replace_query_param(url, self.page_query_param, 1)
+
+def get_last_link(self):
+url = self.request.build_absolute_uri()
+page_number = self.page.paginator.num_pages
+return replace_query_param(url, self.page_query_param, page_number)
+
def get_paginated_response(self, data):
next_url = self.get_next_link()
previous_url = self.get_previous_link()
+first_url = self.get_first_link()
+last_url = self.get_last_link()
+
+links = []
+
+if next_url is not None:
+links.append(f'<{next_url}>; rel="next"')
+
+if previous_url is not None:
+links.append(f'<{previous_url}>; rel="prev"')
+
+links.append(f'<{first_url}>; rel="first"')
+links.append(f'<{last_url}>; rel="last"')
-link = ''
-if next_url is not None and previous_url is not None:
-link = '<{next_url}>; rel="next", <{previous_url}>; rel="prev"'
-elif next_url is not None:
-link = '<{next_url}>; rel="next"'
-elif previous_url is not None:
-link = '<{previous_url}>; rel="prev"'
-link = link.format(next_url=next_url, previous_url=previous_url)
-headers = {'Link': link} if link else {}
+headers = {'Link': ', '.join(links)} if links else {}
return Response(data, headers=headers)
--
2.31.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[RFC PATCH v2 02/19] templatetags: Trivial cleanup
Make this a little nicer to read.
Signed-off-by: Stephen Finucane
---
patchwork/templatetags/listurl.py | 22 -
patchwork/templatetags/patch.py | 24 --
patchwork/templatetags/person.py | 9 +++--
patchwork/templatetags/project.py | 11 +--
patchwork/templatetags/syntax.py | 55 ++-
5 files changed, 75 insertions(+), 46 deletions(-)
diff --git patchwork/templatetags/listurl.py patchwork/templatetags/listurl.py
index 3d5388d2..ad920de2 100644
--- patchwork/templatetags/listurl.py
+++ patchwork/templatetags/listurl.py
@@ -5,8 +5,8 @@
from django.conf import settings
from django import template
-from django.urls import reverse
from django.urls import NoReverseMatch
+from django.urls import reverse
from django.utils.encoding import smart_str
from django.utils.html import escape
@@ -20,7 +20,6 @@ list_params = [c.param for c in FILTERS] + ['order', 'page']
class ListURLNode(template.defaulttags.URLNode):
-
def __init__(self, kwargs):
super(ListURLNode, self).__init__(None, [], {}, False)
self.params = {}
@@ -38,8 +37,9 @@ class ListURLNode(template.defaulttags.URLNode):
except NoReverseMatch:
try:
project_name = settings.SETTINGS_MODULE.split('.')[0]
-path = reverse(project_name + '.' + view_name,
- args=[], kwargs=kwargs)
+path = reverse(
+project_name + '.' + view_name, args=[], kwargs=kwargs
+)
except NoReverseMatch:
raise
@@ -59,8 +59,12 @@ class ListURLNode(template.defaulttags.URLNode):
if not params:
return path
-return path + '?' + '&'.join(
-['%s=%s' % (k, escape(v)) for (k, v) in list(params.items())])
+return '?'.join(
+[
+path,
+'&'.join('%s=%s' % (k, escape(v)) for k, v in params.items()),
+]
+)
@register.tag
@@ -68,7 +72,8 @@ def listurl(parser, token):
bits = token.contents.split(' ', 1)
if not bits:
raise template.TemplateSyntaxError(
-"'%s' takes at least one argument (path to a view)" % bits[0])
+"'%s' takes at least one argument (path to a view)" % bits[0]
+)
kwargs = {}
if len(bits) > 1:
for arg in bits[1].split(','):
@@ -78,5 +83,6 @@ def listurl(parser, token):
kwargs[k] = parser.compile_filter(v)
else:
raise template.TemplateSyntaxError(
-"'%s' requires name=value params" % bits[0])
+"'%s' requires name=value params" % bits[0]
+)
return ListURLNode(kwargs)
diff --git patchwork/templatetags/patch.py patchwork/templatetags/patch.py
index 3837798d..cf22f251 100644
--- patchwork/templatetags/patch.py
+++ patchwork/templatetags/patch.py
@@ -18,6 +18,7 @@ register = template.Library()
def patch_tags(patch):
counts = []
titles = []
+
for tag in [t for t in patch.project.tags if t.show_column]:
count = getattr(patch, tag.attr_name)
titles.append('%d %s' % (count, tag.name))
@@ -25,9 +26,10 @@ def patch_tags(patch):
counts.append("-")
else:
counts.append(str(count))
-return mark_safe('%s' % (
-' / '.join(titles),
-' '.join(counts)))
+
+return mark_safe(
+'%s' % (' / '.join(titles), ' '.join(counts))
+)
@register.filter(name='patch_checks')
@@ -51,14 +53,15 @@ def patch_checks(patch):
count = '-'
check_elements.append(
-'{}'.format(
-color, count))
+f'{count}'
+)
check_elements.reverse()
-return mark_safe('%s' % (
-' / '.join(titles),
-''.join(check_elements)))
+return mark_safe(
+'%s'
+% (' / '.join(titles), ''.join(check_elements))
+)
@register.filter(name='patch_commit_display')
@@ -69,5 +72,6 @@ def patch_commit_display(patch):
if not fmt:
return escape(commit)
-return mark_safe('%s' % (escape(fmt.format(commit)),
- escape(commit)))
+return mark_safe(
+'%s' % (escape(fmt.format(commit)), escape(commit))
+)
diff --git patchwork/templatetags/person.py patchwork/templatetags/person.py
index 61937d94..f49444ae 100644
--- patchwork/templatetags/person.py
+++ patchwork/templatetags/person.p
[RFC PATCH v2 00/19] Integrate Bulma
This is a partial series to migrate the UX to Bulma and rework many of the view flows. Bulma was chosen as it's well maintained and reasonably concise. Tailwind, Bootstrap 4 and Zurb Foundation 6 were also evaluated and could conceivably work, but Tailwind proved incredibly slow to work with (too many options!) and the latter two were significantly beefier without providing significant advantages over Bulma. The series is RFC as it almost certainly conflicts with Raxel's series and I'm not entirely sure all of the ideas included are good ones. I also need additional context in many of the commit messages and unit tests for the code changes. It's also incomplete as I've yet to main the main patch list and submission pages. Depending on feedback though, I can address all of these issues and resolve conflicts with Raxel's work. I'd also be open to people taking the ideas and running with them. We should probably merge the bottom 3-5 patches though. Those seem broadly useful. Changes since v1: - Rebased onto master and resolved merge conflicts Stephen Finucane (19): templates: Use standard indentation templatetags: Trivial cleanup templatetags: Add 'site_admin' tag REST: Include 'first', 'last' refs in 'Link' header settings: Enable django-cors-headers WIP: REST: Change permissions for '/people', '/users' htdocs: Integrate bulma, fontawesome trivial: Run views through black templates: Convert login, password reset views templates: Convert user profile view templates: Enhance profile view further view: Simplify user opt-in, opt-out templates: Convert bundles view templates: Convert projects view WIP: templates: Convert todo-list view templates: Convert project view templates: Convert about page templates: Convert mail settings pages templates: Convert registration template htdocs/css/bulma.css.map |1 + htdocs/css/bulma.min.css |1 + htdocs/css/fontawesome.css| 4619 +++ htdocs/css/fontawesome.min.css|5 + htdocs/webfonts/fa-brands-400.eot | Bin 0 -> 136822 bytes htdocs/webfonts/fa-brands-400.svg | 3717 htdocs/webfonts/fa-brands-400.ttf | Bin 0 -> 136516 bytes htdocs/webfonts/fa-brands-400.woff| Bin 0 -> 92136 bytes htdocs/webfonts/fa-brands-400.woff2 | Bin 0 -> 78472 bytes htdocs/webfonts/fa-regular-400.eot| Bin 0 -> 34350 bytes htdocs/webfonts/fa-regular-400.svg| 801 +++ htdocs/webfonts/fa-regular-400.ttf| Bin 0 -> 34052 bytes htdocs/webfonts/fa-regular-400.woff | Bin 0 -> 16776 bytes htdocs/webfonts/fa-regular-400.woff2 | Bin 0 -> 13588 bytes htdocs/webfonts/fa-solid-900.eot | Bin 0 -> 204814 bytes htdocs/webfonts/fa-solid-900.svg | 5028 + htdocs/webfonts/fa-solid-900.ttf | Bin 0 -> 204528 bytes htdocs/webfonts/fa-solid-900.woff | Bin 0 -> 104280 bytes htdocs/webfonts/fa-solid-900.woff2| Bin 0 -> 80252 bytes patchwork/api/base.py | 33 +- patchwork/api/person.py |4 +- patchwork/api/user.py |5 +- patchwork/forms.py| 338 +- patchwork/settings/dev.py | 26 +- patchwork/templates/patchwork/about.html | 149 +- patchwork/templates/patchwork/bundle.html | 37 +- patchwork/templates/patchwork/bundles.html| 135 +- .../templates/patchwork/confirm-error.html| 19 - patchwork/templates/patchwork/list.html |2 +- patchwork/templates/patchwork/login.html | 105 +- .../templates/patchwork/mail-configure.html | 70 + .../templates/patchwork/mail-settings.html| 103 +- patchwork/templates/patchwork/mail.html | 38 - .../templates/patchwork/optin-request.html| 49 - patchwork/templates/patchwork/optin.html | 18 - .../templates/patchwork/optout-request.html | 50 - patchwork/templates/patchwork/optout.html | 21 - .../patchwork/partials/download-buttons.html | 34 +- .../templates/patchwork/partials/filters.html | 83 +- .../patchwork/partials/pagination.html| 42 +- .../patchwork/partials/patch-list.html| 479 +- patchwork/templates/patchwork/profile.html| 515 +- patchwork/templates/patchwork/project.html| 401 +- patchwork/templates/patchwork/projects.html | 83 +- .../patchwork/registration-confirm.html | 13 - .../templates/patchwork/registration.html | 217 +- patchwork/templates/patchwork/submission.html | 400 +- patchwork/templates/patchwork/todo-list.html | 43 +- patchwork/templates/patchwork/todo-lists.html | 31 +- .../patchwork
Re: [PATCH 1/4] Make addressed/unaddressed workflow opt-in
On Sun, 2021-08-29 at 23:04 +1000, Daniel Axtens wrote: > Stephen Finucane writes: > > > On Fri, 2021-08-27 at 13:50 +1000, Daniel Axtens wrote: > > > Stephen Finucane writes: > > > > > > > The current workflow for the address/unaddressed attribute of comments > > > > sets all comments to unaddressed by default. This is unintuitive, as it > > > > assumes that all comments are actionable items. It also imposes a > > > > massive burden on maintainers, who will need to manually sift through > > > > every single comment received to a list and manually set the > > > > non-actionable items as "addressed". > > > > > > I agree that not every email is an actionable item. > > > > > > I'm not convinced it's a burden on maintainers specifically. The comment > > > can also be marked as addressed by the patch submitter. Also, > > > maintainers (and everyone else) are free to ignore the field (and every > > > other piece of data stored on patchwork). > > > > > > In general I do think 'unaddressed by default' is a good behaviour. But, > > > I agree that we can improve the current behaviour. > > > > > > I think it makes sense to have it as null for every old patch. So if you > > > migrate, old patch comments are neither addressed nor > > > unaddressed. That's something I didn't consider sufficiently earlier on. > > > > > > I think it also makes sense for patches that add 'Acked-by:', > > > 'Reviewed-by:' or 'Tested-by:' to be considered automatically addressed. > > > > > > But I worry that saying that everything is automatically neither means > > > that a patch sumbitter could very easily forget to do that and then we > > > risk losing the value that the feature is supposed to add. > > > > Right, but if as you've said this is a feature intended for submitters > > rather > > than maintainers, then surely we can assume that they will set the flag as > > necessary since they'll ultimately benefit from it? I get that nudges (in > > the > > psychology sense) are a thing but we shouldn't have to "force" people to use > > this feature by turning it on for every single non-code submission they > > make to > > a list and not providing a way to opt out of it. That's not cool and I don't > > think it's all that productive either. Until we have sufficiently advanced > > AI/ML > > to detect actionable comments, simply encouraging submitters to use this > > tool as > > a way to manually track action items (rather than scribbling them in a > > diary or > > whatever) seems more than okay. > > Maybe? Easy to miss an actionable comment if they're not automatically > marked, I'd think. > > Anyway, I feel like we could go back and forth on this a bit, so maybe > we should try to explore and see if there's a bigger set of potential > solutions that might make both of us happier... > > How does this strike you? > > a) all old mail gets the NULL value. Yes, please. > and > > b) Projects get a switch to enable/disable the feature. If you're a > maintainer and you think these fields are more trouble than they're > worth, ask your PW admin to make them disappear. I'm on the fence about this one. As noted in my earlier email, I don't think allowing maintainers to entirely disable this user-centric feature is a good precedent. Also, it doesn't seem entirely necessary given the below. > > and > > b) Users get a switch - maybe with "all automatically unaddressed", > "NULL until manually marked" and "don't show me any of this ever" > options (obviously with better names) So to rephrase, this would be a user preference checkbox to allow users to automatically have any submitted comments marked as unaddressed, yes? If we can have that default to False (i.e. opt-in) then yes, this would work for me. The "don't show me any of this option" could/should probably be implemented as a separate thing though I could live without this if the addressed/unaddressed thing is something a contributor is opting into and is therefore likely to actually tend to. Want me to submit a follow-up patch to add this feature? I think we can merge the series I have as-is (assuming I haven't missed anything) since we've agreed that the field should default to NULL, which means we need those migration and API schema changes. > That way, with basically no extra load on the
Re: [PATCH 3/4] parser: Add 'X-Patchwork-Action-Required' header
On Fri, 2021-08-27 at 13:51 +1000, Daniel Axtens wrote: > Stephen Finucane writes: > > > Allow submitters to indicate that their comment is something that needs > > to be addressed. > > Hmm, do we have any evidence that any of our existing mail headers are > used by anything? I've no idea. A quick scan through an old Patchwork archive mbox I have locally suggests no but I can't speak for other lists. That said, part of the issue here could simply be lack of awareness of the feature as much as anything else. Perhaps we should try to make this feature more prominent in the docs? > Also, I'm not confident I know how to set a header on a comment and I > write my email in emacs, notoriously one of the more configurable > platforms for any given task. Evolution does make it pretty easy [1], as does mutt [1]. We could add these links to the docs also, if it would help? I'll admit I haven't used either myself though since I'm happy enough with 'git-pw' for my day-to-day work. Stephen [1] https://help.gnome.org/users/evolution/stable//mail-composer-custom-header-lines [2] http://www.mutt.org/doc/manual/#ex-my-hdr > > > > > Some minors issues are addressed in the docs while we're here. > > > > Signed-off-by: Stephen Finucane > > --- > > docs/usage/headers.rst | 17 --- > > docs/usage/overview.rst| 7 +++ > > patchwork/parser.py| 10 +++- > > patchwork/tests/test_parser.py | 89 -- > > 4 files changed, 111 insertions(+), 12 deletions(-) > > > > diff --git docs/usage/headers.rst docs/usage/headers.rst > > index 26e97c0a..b2b4b2d6 100644 > > --- docs/usage/headers.rst > > +++ docs/usage/headers.rst > > @@ -5,8 +5,7 @@ Patchwork provides a number of special email headers to > > control how a patch is > > handled when it is received. The examples provided below use > > `git-send-email`, > > but custom headers can also be set when using tools like `mutt`. > > > > -`X-Patchwork-Hint` > > - > > +``X-Patchwork-Hint`` > >Valid values: ignore > > > >When set, this header will ensure the provided email is not parsed > > @@ -16,8 +15,7 @@ but custom headers can also be set when using tools like > > `mutt`. > > > > $ git send-email --add-header="X-Patchwork-Hint: ignore" master > > > > -`X-Patchwork-Delegate` > > - > > +``X-Patchwork-Delegate`` > >Valid values: An email address associated with a Patchwork user > > > >If set and valid, the user corresponding to the provided email address > > will > > @@ -28,8 +26,7 @@ but custom headers can also be set when using tools like > > `mutt`. > > > > $ git send-email --add-header="X-Patchwork-Delegate: > > a...@example.com" master > > > > -`X-Patchwork-State` > > - > > +``X-Patchwork-State`` > >Valid values: Varies between deployments. This can usually be one of > >"Accepted", "Rejected", "RFC" or "Awaiting Upstream", among others. > > > > @@ -39,3 +36,11 @@ but custom headers can also be set when using tools like > > `mutt`. > >.. code-block:: shell > > > > $ git send-email --add-header="X-Patchwork-State: RFC" master > > + > > +``X-Patchwork-Action-Required`` > > + Valid values: > > + > > + When set on a reply to an existing cover letter or patch, this header > > will > > + mark the comment as "unaddressed". The comment can then be addressed > > using > > + the API or web UI. For more details, refer to > > + :ref:`overview-comment-metadata`. > > diff --git docs/usage/overview.rst docs/usage/overview.rst > > index a58acfa5..297569ec 100644 > > --- docs/usage/overview.rst > > +++ docs/usage/overview.rst > > @@ -181,6 +181,8 @@ system to test patches. Checks have a number of fields > > associated with them: > > to Patchwork. > > > > > > +.. _overview-comment-metadata: > > + > > Comment Metadata > > > > > > @@ -198,6 +200,11 @@ the cover letters. Once the submitter has provided the > > required information, > > either the submitter or a maintainer can mark the comment as "addressed". > > This > > provides a more granular way of tracking work items than patch states. > > > > +.. note:: > > + > > + Users can indicate that a comment requires an action using a custom mail > > + header. For mo
Re: [PATCH 1/4] Make addressed/unaddressed workflow opt-in
On Fri, 2021-08-27 at 13:50 +1000, Daniel Axtens wrote:
> Stephen Finucane writes:
>
> > The current workflow for the address/unaddressed attribute of comments
> > sets all comments to unaddressed by default. This is unintuitive, as it
> > assumes that all comments are actionable items. It also imposes a
> > massive burden on maintainers, who will need to manually sift through
> > every single comment received to a list and manually set the
> > non-actionable items as "addressed".
>
> I agree that not every email is an actionable item.
>
> I'm not convinced it's a burden on maintainers specifically. The comment
> can also be marked as addressed by the patch submitter. Also,
> maintainers (and everyone else) are free to ignore the field (and every
> other piece of data stored on patchwork).
>
> In general I do think 'unaddressed by default' is a good behaviour. But,
> I agree that we can improve the current behaviour.
>
> I think it makes sense to have it as null for every old patch. So if you
> migrate, old patch comments are neither addressed nor
> unaddressed. That's something I didn't consider sufficiently earlier on.
>
> I think it also makes sense for patches that add 'Acked-by:',
> 'Reviewed-by:' or 'Tested-by:' to be considered automatically addressed.
>
> But I worry that saying that everything is automatically neither means
> that a patch sumbitter could very easily forget to do that and then we
> risk losing the value that the feature is supposed to add.
Right, but if as you've said this is a feature intended for submitters rather
than maintainers, then surely we can assume that they will set the flag as
necessary since they'll ultimately benefit from it? I get that nudges (in the
psychology sense) are a thing but we shouldn't have to "force" people to use
this feature by turning it on for every single non-code submission they make to
a list and not providing a way to opt out of it. That's not cool and I don't
think it's all that productive either. Until we have sufficiently advanced AI/ML
to detect actionable comments, simply encouraging submitters to use this tool as
a way to manually track action items (rather than scribbling them in a diary or
whatever) seems more than okay.
> Another thing we could consider doing is making it opt-in by
> project. For projects that keep pw very tidy (I'm thinking
> e.g. https://patchwork.kernel.org/project/netdevbpf/list/) then
> the addressed/unaddressed thing might be more useful and less noisy than
> e.g. linuxppc which is a bit less well pruned.
This does sound initially reasonable, but if these things are opt-in and
intended for the submitter then the value of making this configurable on a per-
project basis is significantly lower, right? In fact, it might even be actively
harmful since an opinionated maintainer (let's say you or I) could disable it
for an entire project (patchwork) meaning no submitter (Raxel?) could use this
supposedly submitter-focused feature to track action items even if they wanted
to.
If we were going to do a global'ish config option, I'd probably make it a user
preference like the "show patch IDs" feature, so submitters that wanted to make
use of the feature would see the various flags while maintainer's who didn't
care for it could remain blissfully unaware. That assumes that the feature has
no value whatsoever for maintainers though, which I'm not sure is entirely true?
> But, again, I see the un/addressed field as being for the submitter, not
> the maintainer. The maintainer can't trust it anyway because what the
> submitter considers 'addressed' and the maintainer considers 'addressed'
> could be very different. So really I see this as helping a submitter to
> track that there is nothing waiting on them.
No arguments from me: I'm totally behind the feature as whole and understand the
motivation. I'm saying that submitters should be able to choose to set this
"action required" flag on individual comments as action items pop up, rather
than it being forced onto every single comment they send to the list. There are
a variety of ways they could do this, be it via the web UI, a custom tool run
locally ('pw-mark-actionable '?), a script in your mail client, etc. It
won't be an issue.
> > Change this workflow so that the 'addressed' field defaults to NULL.
> > This means maintainers or users must manually set this to False when
> > they're requesting additional feedback. This is currently possible via
> > the web UI or REST API. A future change will make it possible via a
> > custom mail header.
> >
> > Signed
Re: [PATCH v4 4/9] models: add addressed field
On Mon, 2021-08-23 at 15:12 +1000, Daniel Axtens wrote: > Stephen Finucane writes: > > > On Fri, 2021-08-20 at 04:50 +, Raxel Gutierrez wrote: > > > Currently, there is no state or status associated with comments. In > > > particular, knowing whether a comment on a patch or cover letter is > > > addressed or not is useful for transparency and accountability in the > > > patch review and contribution process. This patch is backend setup for > > > tracking the state of patch and cover comments. > > > > > > Add `addressed` boolean field to patch and cover comments to be able to > > > distinguish between unaddressed and addressed comments in the > > > patch-detail page. > > > > > > Signed-off-by: Raxel Gutierrez > > > Reviewed-by: Daniel Axtens > > > > I'm still not entirely happy with this design. It's functional, but it seems > > overly verbose for what I think we're trying to do here. In particular, the > > idea > > that every single comment is actionable and is unaddressed by default feels > > wrong to me. > > > > Who is this field intended for? Is it for maintainers to track whether a > > question they asked has been answered, or is it for submitter to track > > whether > > they've answered all of a reviewers questions? I suspect it's the former > > and, if > > so, what are your thoughts on defaulting to addressed being unset by default > > (i.e. NULL or no action item). This would mean maintainers would be > > required to > > set 'addressed=False' when needed? We could event allow them to do this via > > an > > email header which would be easily set in mail clients like mutt. If it's > > the > > latter, the same approach would probably still work, I think. WDYT? > > From my point of view, it's mostly aimed at submitters who want a method > to keep track of their own tasks. (I know Emily S was hoping to one day > see unaddressed comments in a user's to-do list, which I think gives you > more insight into the broader use case.) Right, but this should absolutely be opt-in. We shouldn't default to assuming that all users or maintainers want to track work items like this. We *definitely* shouldn't default to all comments, past and present, being classed as actionable things... > > I think maintainers could chose to make it part of their workflow, but > given that the submitter gets to mark a comment as addressed, a > maintainer couldn't really trust that 'addressed' meant 'addressed > satisfactorily'. It's not really a tool to tell maintainers that a patch > is ready. > > IMO the impact is fairly small on people who don't want to use it - they > can just ignore the changes in the comment header bar. Perhaps, but for users that don't plan on using this, this bit of "noise" will suddenly appear on every single patch and cover letter comment, past and present. That doesn't seem reasonable. Even uses that do plan to use this will suffer since every patch the submitter has ever submitted to any list managed by the Patchwork instance would immediately be classed as a needing action (i.e. unresolved) unless and until a maintainer or the submitter went through the tedious process of setting 'addressed' to True for every. single. comment. ever submitter. This would make Emily's idea of adding this to the TODO list a no-go since that TODO list would be already be populated with a huge and unmanageable list of most useless "action items". > The git folks do seem to have a use case in mind for this, and so unless > it's going to really mess up someone else's use case I'm inclined to > accept it and clean up any fallout later. Hopefully it's clear by now what me issues with this series are. I'd rather we'd figured this out on the list before merging, but I appreciate there are time constraints here. In the vein of cleaning up the fallout, I've just submitted a series to make this whole thing opt-in as it should have been from day 0. I'd appreciate eye-on this and would obviously welcome suggestions for other approaches. Cheers, Stephen > > Kind regards, > Daniel > > > > > Aside from this design question, the rest of the changes, particularly > > those to > > the REST API look okay, but I'll need to spend more time on this over the > > weekend. > > > > Cheers, > > Stephen > > > > ___ > > Patchwork mailing list > > Patchwork@lists.ozlabs.org > > https://lists.ozlabs.org/listinfo/patchwork ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH 4/4] tests: Tweak comment API tests
We were missing tests for 'GET /patch/{patch_id}/comment' (list patch
comments) and 'GET /cover/{cover_id}/comment' (list cover comments) when
using API version 1.2. In addition, we had effectively duplicated tests
by including explicit tests for API 1.3. These are unnecessary since we
default to testing against the latest version. Address both issues.
Signed-off-by: Stephen Finucane
---
patchwork/tests/api/test_comment.py | 40 -
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git patchwork/tests/api/test_comment.py
patchwork/tests/api/test_comment.py
index 74acf447..5c035e82 100644
--- patchwork/tests/api/test_comment.py
+++ patchwork/tests/api/test_comment.py
@@ -69,6 +69,17 @@ class TestCoverComments(utils.APITestCase):
self.assertEqual(1, len(resp.data))
self.assertSerialized(comment, resp.data[0])
self.assertIn('list_archive_url', resp.data[0])
+self.assertIn('addressed', resp.data[0])
+
+def test_list_version_1_2(self):
+"""List cover letter comments using API v1.2."""
+create_cover_comment(cover=self.cover)
+
+resp = self.client.get(self.api_url(self.cover, version='1.2'))
+self.assertEqual(status.HTTP_200_OK, resp.status_code)
+self.assertEqual(1, len(resp.data))
+self.assertIn('list_archive_url', resp.data[0])
+self.assertNotIn('addressed', resp.data[0])
def test_list_version_1_1(self):
"""List cover letter comments using API v1.1."""
@@ -110,15 +121,6 @@ class TestCoverComments(utils.APITestCase):
self.assertEqual(status.HTTP_200_OK, resp.status_code)
self.assertSerialized(comment, resp.data)
-def test_detail_version_1_3(self):
-"""Show a cover letter comment using API v1.3."""
-comment = create_cover_comment(cover=self.cover)
-
-resp = self.client.get(
-self.api_url(self.cover, version='1.3', item=comment))
-self.assertEqual(status.HTTP_200_OK, resp.status_code)
-self.assertSerialized(comment, resp.data)
-
def test_detail_version_1_2(self):
"""Show a cover letter comment using API v1.2."""
comment = create_cover_comment(cover=self.cover)
@@ -292,6 +294,17 @@ class TestPatchComments(utils.APITestCase):
self.assertEqual(1, len(resp.data))
self.assertSerialized(comment, resp.data[0])
self.assertIn('list_archive_url', resp.data[0])
+self.assertIn('addressed', resp.data[0])
+
+def test_list_version_1_2(self):
+"""List patch comments using API v1.2."""
+create_patch_comment(patch=self.patch)
+
+resp = self.client.get(self.api_url(self.patch, version='1.2'))
+self.assertEqual(status.HTTP_200_OK, resp.status_code)
+self.assertEqual(1, len(resp.data))
+self.assertIn('list_archive_url', resp.data[0])
+self.assertNotIn('addressed', resp.data[0])
def test_list_version_1_1(self):
"""List patch comments using API v1.1."""
@@ -333,15 +346,6 @@ class TestPatchComments(utils.APITestCase):
self.assertEqual(status.HTTP_200_OK, resp.status_code)
self.assertSerialized(comment, resp.data)
-def test_detail_version_1_3(self):
-"""Show a patch comment using API v1.3."""
-comment = create_patch_comment(patch=self.patch)
-
-resp = self.client.get(
-self.api_url(self.patch, version='1.3', item=comment))
-self.assertEqual(status.HTTP_200_OK, resp.status_code)
-self.assertSerialized(comment, resp.data)
-
def test_detail_version_1_2(self):
"""Show a patch comment using API v1.2."""
comment = create_patch_comment(patch=self.patch)
--
2.31.1
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH 3/4] parser: Add 'X-Patchwork-Action-Required' header
Allow submitters to indicate that their comment is something that needs
to be addressed.
Some minors issues are addressed in the docs while we're here.
Signed-off-by: Stephen Finucane
---
docs/usage/headers.rst | 17 ---
docs/usage/overview.rst| 7 +++
patchwork/parser.py| 10 +++-
patchwork/tests/test_parser.py | 89 --
4 files changed, 111 insertions(+), 12 deletions(-)
diff --git docs/usage/headers.rst docs/usage/headers.rst
index 26e97c0a..b2b4b2d6 100644
--- docs/usage/headers.rst
+++ docs/usage/headers.rst
@@ -5,8 +5,7 @@ Patchwork provides a number of special email headers to control
how a patch is
handled when it is received. The examples provided below use `git-send-email`,
but custom headers can also be set when using tools like `mutt`.
-`X-Patchwork-Hint`
-
+``X-Patchwork-Hint``
Valid values: ignore
When set, this header will ensure the provided email is not parsed
@@ -16,8 +15,7 @@ but custom headers can also be set when using tools like
`mutt`.
$ git send-email --add-header="X-Patchwork-Hint: ignore" master
-`X-Patchwork-Delegate`
-
+``X-Patchwork-Delegate``
Valid values: An email address associated with a Patchwork user
If set and valid, the user corresponding to the provided email address will
@@ -28,8 +26,7 @@ but custom headers can also be set when using tools like
`mutt`.
$ git send-email --add-header="X-Patchwork-Delegate: a...@example.com"
master
-`X-Patchwork-State`
-
+``X-Patchwork-State``
Valid values: Varies between deployments. This can usually be one of
"Accepted", "Rejected", "RFC" or "Awaiting Upstream", among others.
@@ -39,3 +36,11 @@ but custom headers can also be set when using tools like
`mutt`.
.. code-block:: shell
$ git send-email --add-header="X-Patchwork-State: RFC" master
+
+``X-Patchwork-Action-Required``
+ Valid values:
+
+ When set on a reply to an existing cover letter or patch, this header will
+ mark the comment as "unaddressed". The comment can then be addressed using
+ the API or web UI. For more details, refer to
+ :ref:`overview-comment-metadata`.
diff --git docs/usage/overview.rst docs/usage/overview.rst
index a58acfa5..297569ec 100644
--- docs/usage/overview.rst
+++ docs/usage/overview.rst
@@ -181,6 +181,8 @@ system to test patches. Checks have a number of fields
associated with them:
to Patchwork.
+.. _overview-comment-metadata:
+
Comment Metadata
@@ -198,6 +200,11 @@ the cover letters. Once the submitter has provided the
required information,
either the submitter or a maintainer can mark the comment as "addressed". This
provides a more granular way of tracking work items than patch states.
+.. note::
+
+ Users can indicate that a comment requires an action using a custom mail
+ header. For more information, refer to :doc:`/usage/headers`.
+
Collections
---
diff --git patchwork/parser.py patchwork/parser.py
index 61a81246..e6e1a7fb 100644
--- patchwork/parser.py
+++ patchwork/parser.py
@@ -1019,6 +1019,12 @@ def find_delegate_by_header(mail):
return None
+def find_comment_addressed_by_header(mail):
+"""Determine whether a comment is actionable or not."""
+# we dispose of the value - it's irrelevant
+return False if 'X-Patchwork-Action-Required' in mail else None
+
+
def parse_mail(mail, list_id=None):
"""Parse a mail and add to the database.
@@ -1278,6 +1284,7 @@ def parse_mail(mail, list_id=None):
patch = find_patch_for_comment(project, refs)
if patch:
author = get_or_create_author(mail, project)
+addressed = find_comment_addressed_by_header(mail)
with transaction.atomic():
if PatchComment.objects.filter(patch=patch, msgid=msgid):
@@ -1288,7 +1295,8 @@ def parse_mail(mail, list_id=None):
date=date,
headers=headers,
submitter=author,
-content=message)
+content=message,
+addressed=addressed)
logger.debug('Comment saved')
diff --git patchwork/tests/test_parser.py patchwork/tests/test_parser.py
index eaf6599c..d0c5c2d7 100644
--- patchwork/tests/test_parser.py
+++ patchwork/tests/test_parser.py
@@ -18,6 +18,7 @@ from django.db.transaction import atomic
from django.db import connection
from patchwork.models import Cover
+from patchwork.models import CoverComment
from patchwork.models import Patch
from patchwork.models import PatchComment
from patchwork.models import Person
@@ -68,22 +69,42 @@ def read_mail(filename, project=None):
return mail
-def _create_email(msg, msgid=None, sender=None, listid=None, in_reply_to=None):
+def _create_email(
+msg,
+msgid=None,
+subject=None,
+sender=None,
[PATCH 0/4] Make addressed/unaddressed workflow opt-in
We recently added the ability to set a patch or cover letter comment as needing some kind of action. This was implemented via an 'addressed' field for both 'PatchComment' and 'CoverComment'. However, this feature defaulted the value of 'addressed' to False, which is the equivalent of defaulting all all comments to a "needinfo" or "action required" state. This is both noisy and somewhat silly, requiring a massive amount of work by maintainers and other contributors to provide any kind of meaningful state. This series proposes changing this default to NULL, meaning the comment does not have any action items and has not had an action item addressed in the past. This effectively provides a state like the following: +---+ | addressed = NULL | +---+ | | (mark as action required) v +---+ | addressed = False |<--\ +---+ | | | | (mark as unaddressed) | v | +---+ | | addressed = True | | +---+ | | | | (mark as unaddressed) | \-/ Note that we explicitly don't allow setting addressed back to NULL since we would lose history. A future series will add a new Event type to track these changes. Stephen Finucane (4): Make addressed/unaddressed workflow opt-in docs: Document the address/unaddressed comment feature parser: Add 'X-Patchwork-Action-Required' header tests: Tweak comment API tests docs/api/schemas/latest/patchwork.yaml| 2 + docs/api/schemas/patchwork.j2 | 2 + docs/api/schemas/v1.3/patchwork.yaml | 2 + docs/usage/headers.rst| 17 ++-- docs/usage/overview.rst | 35 ++-- htdocs/js/submission.js | 14 ++- patchwork/migrations/0045_addressed_fields.py | 4 +- patchwork/models.py | 4 +- patchwork/parser.py | 10 ++- patchwork/templates/patchwork/submission.html | 20 - patchwork/tests/api/test_comment.py | 40 + patchwork/tests/test_parser.py| 89 +-- 12 files changed, 194 insertions(+), 45 deletions(-) -- 2.31.1 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH 2/4] docs: Document the address/unaddressed comment feature
Signed-off-by: Stephen Finucane --- docs/usage/overview.rst | 28 +++- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git docs/usage/overview.rst docs/usage/overview.rst index 273c792a..a58acfa5 100644 --- docs/usage/overview.rst +++ docs/usage/overview.rst @@ -71,11 +71,11 @@ Cover Letters ~ Cover letters provide a way to offer a "big picture" overview of a series of -patches. When using Git, these mails can be recognised by way of their `0/N` -subject prefix, e.g. `[00/11] A sample series`. Like patches, Patchwork stores -not only the various aspects of the cover letter itself, such as the name and -body of the cover letter, but also various metadata associated with the email -that the cover letter was parsed from. +patches. When using Git, these mails can be recognised by way of their ``0/N`` +subject prefix, e.g. ``[00/11] A sample series``. Like patches, Patchwork +stores not only the various aspects of the cover letter itself, such as the +name and body of the cover letter, but also various metadata associated with +the email that the cover letter was parsed from. Comments @@ -181,6 +181,24 @@ system to test patches. Checks have a number of fields associated with them: to Patchwork. +Comment Metadata + + +Like patches, Patchwork allows users to store various bits of metadata against +comments. + +Action required +~~~ + +.. versionadded:: 3.1.0 + +Patchwork allows users to set an "action required" flag on patch and cover +letter comments. This flag can be set by maintainers or by the users submitting +the cover letters. Once the submitter has provided the required information, +either the submitter or a maintainer can mark the comment as "addressed". This +provides a more granular way of tracking work items than patch states. + + Collections --- -- 2.31.1 ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
[PATCH 1/4] Make addressed/unaddressed workflow opt-in
The current workflow for the address/unaddressed attribute of comments
sets all comments to unaddressed by default. This is unintuitive, as it
assumes that all comments are actionable items. It also imposes a
massive burden on maintainers, who will need to manually sift through
every single comment received to a list and manually set the
non-actionable items as "addressed".
Change this workflow so that the 'addressed' field defaults to NULL.
This means maintainers or users must manually set this to False when
they're requesting additional feedback. This is currently possible via
the web UI or REST API. A future change will make it possible via a
custom mail header.
Signed-off-by: Stephen Finucane
Cc: Raxel Gutierrez
Cc: Daniel Axtens
---
I think it's essential we make this change in order for this patch to be
useful. I also think it's okay to modify the migration in place, since
(a) we don't support deployment from master in production and (b) to the
best of my knowledge, setting a default, non-NULL value on a new column
is an expensive operation on certain databases (MySQL?) while changing
a column value for all rows is *definitely* expensive. The template work
could possibly do with tweaking. Feel free to advise if so.
---
docs/api/schemas/latest/patchwork.yaml| 2 ++
docs/api/schemas/patchwork.j2 | 2 ++
docs/api/schemas/v1.3/patchwork.yaml | 2 ++
htdocs/js/submission.js | 14 +++--
patchwork/migrations/0045_addressed_fields.py | 4 ++--
patchwork/models.py | 4 ++--
patchwork/templates/patchwork/submission.html | 20 +++
7 files changed, 38 insertions(+), 10 deletions(-)
diff --git docs/api/schemas/latest/patchwork.yaml
docs/api/schemas/latest/patchwork.yaml
index e3bff990..2a98c179 100644
--- docs/api/schemas/latest/patchwork.yaml
+++ docs/api/schemas/latest/patchwork.yaml
@@ -1669,12 +1669,14 @@ components:
addressed:
title: Addressed
type: boolean
+ nullable: true
CommentUpdate:
type: object
properties:
addressed:
title: Addressed
type: boolean
+ nullable: true
CoverList:
type: object
properties:
diff --git docs/api/schemas/patchwork.j2 docs/api/schemas/patchwork.j2
index 3b4ad2f6..02aa9f72 100644
--- docs/api/schemas/patchwork.j2
+++ docs/api/schemas/patchwork.j2
@@ -1734,12 +1734,14 @@ components:
addressed:
title: Addressed
type: boolean
+ nullable: true
CommentUpdate:
type: object
properties:
addressed:
title: Addressed
type: boolean
+ nullable: true
{% endif %}
CoverList:
type: object
diff --git docs/api/schemas/v1.3/patchwork.yaml
docs/api/schemas/v1.3/patchwork.yaml
index 6cbba646..0a9046a5 100644
--- docs/api/schemas/v1.3/patchwork.yaml
+++ docs/api/schemas/v1.3/patchwork.yaml
@@ -1669,12 +1669,14 @@ components:
addressed:
title: Addressed
type: boolean
+ nullable: true
CommentUpdate:
type: object
properties:
addressed:
title: Addressed
type: boolean
+ nullable: true
CoverList:
type: object
properties:
diff --git htdocs/js/submission.js htdocs/js/submission.js
index 47cffc82..c93c36ec 100644
--- htdocs/js/submission.js
+++ htdocs/js/submission.js
@@ -29,7 +29,17 @@ $( document ).ready(function() {
};
updateProperty(url, data, updateMessage).then(isSuccess => {
if (isSuccess) {
-
$("div[class^='comment-status-bar-'][data-comment-id='"+commentId+"']").toggleClass("hidden");
+// The API won't accept anything but true or false, so we
+// always hide the -action-required element
+
$("div[class='comment-status-bar-action-required'][data-comment-id='"+commentId+"']").addClass("hidden");
+
+if (event.target.value === "true") {
+
$("div[class^='comment-status-bar-addressed'][data-comment-id='"+commentId+"']").removeClass("hidden");
+
$("div[class^='comment-status-bar-unaddressed'][data-comment-id='"+commentId+"']").addClass("hidden");
+} else if (event.target.value === "false") {
+
$("div[class^='comment-status-bar-addressed'][data-comment-id='"+commentId+"']").addClass("hidden");
+
$("div[class^='comment-status-bar-unaddressed'][data-comment-id='"+commentId+"']").remov
Re: [PATCH] docs: prevent build error by rolling back Sphinx version
On Mon, 2021-08-23 at 16:48 +1000, Daniel Axtens wrote: > We're hitting a sphinxcontrib-httpdomain vs sphinx issue that was > causing the openapi part of doc builds to error out with: > > :1:Problem in http domain: field is supposed to use role 'obj', but > that role is not in the domain. > > See https://github.com/sphinx-contrib/httpdomain/pull/51 > > Until it's fixed, hold back the Sphinx version to < 4.1.0 > > Signed-off-by: Daniel Axtens Unfortunate but necessary. The gate for sphinx-contrib/httpdomain is pretty badly broken right now so it could be a while before that patch lands :-( Reviewed-by: Stephen Finucane ...and applied. Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH v4 4/9] models: add addressed field
On Fri, 2021-08-20 at 04:50 +, Raxel Gutierrez wrote: > Currently, there is no state or status associated with comments. In > particular, knowing whether a comment on a patch or cover letter is > addressed or not is useful for transparency and accountability in the > patch review and contribution process. This patch is backend setup for > tracking the state of patch and cover comments. > > Add `addressed` boolean field to patch and cover comments to be able to > distinguish between unaddressed and addressed comments in the > patch-detail page. > > Signed-off-by: Raxel Gutierrez > Reviewed-by: Daniel Axtens I'm still not entirely happy with this design. It's functional, but it seems overly verbose for what I think we're trying to do here. In particular, the idea that every single comment is actionable and is unaddressed by default feels wrong to me. Who is this field intended for? Is it for maintainers to track whether a question they asked has been answered, or is it for submitter to track whether they've answered all of a reviewers questions? I suspect it's the former and, if so, what are your thoughts on defaulting to addressed being unset by default (i.e. NULL or no action item). This would mean maintainers would be required to set 'addressed=False' when needed? We could event allow them to do this via an email header which would be easily set in mail clients like mutt. If it's the latter, the same approach would probably still work, I think. WDYT? Aside from this design question, the rest of the changes, particularly those to the REST API look okay, but I'll need to spend more time on this over the weekend. Cheers, Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH v4 3/9] templatetags: add utils template filters and tags
On Fri, 2021-08-20 at 04:50 +, Raxel Gutierrez wrote: > Add utils.py file to create template filters and tags that can be used > by most if not all objects in Patchwork. In particular, add a template > filter to get the plural verbose name of a model and add a template tag > that returns whether an object is editable by the current user. These > utilities will be used in an upcoming patch that adds the `addressed` > status label to patch and cover comments. > > Signed-off-by: Raxel Gutierrez Need to see the caller but otherwise LGTM. Reviewed-by: Stephen Finucane ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH v4 2/9] api: change parameter to for cover comments endpoint
On Fri, 2021-08-20 at 04:50 +, Raxel Gutierrez wrote:
> Rename cover lookup parameter `pk` to `cover_id` for the cover comments
> list endpoints to disambiguate from the lookup parameter `comment_id` in
> upcoming patches which introduces the cover comments detail endpoint.
> This doesn't affect the user-facing API.
>
> Signed-off-by: Raxel Gutierrez
LGTM with one comment.
> ---
> patchwork/api/comment.py| 6 +++---
> patchwork/api/cover.py | 2 +-
> patchwork/tests/api/test_comment.py | 4 ++--
> patchwork/urls.py | 2 +-
> 4 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
> index 0c578b44..5d7a77a1 100644
> --- a/patchwork/api/comment.py
> +++ b/patchwork/api/comment.py
> @@ -83,14 +83,14 @@ class CoverCommentList(ListAPIView):
> search_fields = ('subject',)
> ordering_fields = ('id', 'subject', 'date', 'submitter')
> ordering = 'id'
> -lookup_url_kwarg = 'pk'
> +lookup_url_kwarg = 'cover_id'
>
> def get_queryset(self):
> -if not Cover.objects.filter(pk=self.kwargs['pk']).exists():
> +if not Cover.objects.filter(id=self.kwargs['cover_id']).exists():
> raise Http404
We should change these two lines to:
get_object_or_404(Cover, id=self.kwargs['pk'])
and the following import added at the top of the file:
from rest_framework.generics import get_object_or_404
See [1] and [2] for the reason why.
Other than this, LGTM. If you don't need to respin, we'll fix while merging.
Reviewed-by: Stephen Finucane
Stephen
[1] https://lists.ozlabs.org/pipermail/patchwork/2021-August/007140.html
[2] https://lists.ozlabs.org/pipermail/patchwork/2021-August/007142.html
>
> return CoverComment.objects.filter(
> -cover=self.kwargs['pk']
> +cover=self.kwargs['cover_id']
> ).select_related('submitter')
>
>
> diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py
> index b4a3a8f7..c4355f6b 100644
> --- a/patchwork/api/cover.py
> +++ b/patchwork/api/cover.py
> @@ -37,7 +37,7 @@ class CoverListSerializer(BaseHyperlinkedModelSerializer):
>
> def get_comments(self, cover):
> return self.context.get('request').build_absolute_uri(
> -reverse('api-cover-comment-list', kwargs={'pk': cover.id}))
> +reverse('api-cover-comment-list', kwargs={'cover_id': cover.id}))
>
> def to_representation(self, instance):
> # NOTE(stephenfin): This is here to ensure our API looks the same
> even
> diff --git a/patchwork/tests/api/test_comment.py
> b/patchwork/tests/api/test_comment.py
> index 59450d8a..53abf8f0 100644
> --- a/patchwork/tests/api/test_comment.py
> +++ b/patchwork/tests/api/test_comment.py
> @@ -27,7 +27,7 @@ class TestCoverComments(utils.APITestCase):
> kwargs = {}
> if version:
> kwargs['version'] = version
> -kwargs['pk'] = cover.id
> +kwargs['cover_id'] = cover.id
>
> return reverse('api-cover-comment-list', kwargs=kwargs)
>
> @@ -79,7 +79,7 @@ class TestCoverComments(utils.APITestCase):
> def test_list_invalid_cover(self):
> """Ensure we get a 404 for a non-existent cover letter."""
> resp = self.client.get(
> -reverse('api-cover-comment-list', kwargs={'pk': '9'}))
> +reverse('api-cover-comment-list', kwargs={'cover_id': '9'}))
> self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
>
>
> diff --git a/patchwork/urls.py b/patchwork/urls.py
> index 1e6c12ab..0180e76d 100644
> --- a/patchwork/urls.py
> +++ b/patchwork/urls.py
> @@ -337,7 +337,7 @@ if settings.ENABLE_REST_API:
> name='api-patch-comment-list',
> ),
> path(
> -'covers//comments/',
> +'covers//comments/',
> api_comment_views.CoverCommentList.as_view(),
> name='api-cover-comment-list',
> ),
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH v4 1/9] patch-detail: left align message headers
On Fri, 2021-08-20 at 04:50 +, Raxel Gutierrez wrote: > Change both of the message containers in the "Commit Message" and > "Comments" to have their content be left-aligned which improves the > proximity of items which boosts the efficiency of gathering information > and performing actions. Before [1] and after [2] images for reference. > > [1] https://i.imgur.com/ji2ZINL.png > [2] https://i.imgur.com/Dtn8lm9.png > > Signed-off-by: Raxel Gutierrez fwict, this simply moves the date and shortlink URL to be left aligned immediately after the author name. I'm not entirely sure about this as submitter names are not all of the same length. This will mean that these attributes will shift vertically about the screen as you scroll, which would personally annoy me just a little :) With that said, what you've said _sounds_ reasonable and a little more evidence-based than my gut feel, so... Reviewed-by: Stephen Finucane ...but I'd like Daniel to take a look before I apply :D Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] REST: don't 500 if we get a non-int ID in /comment/ views
On Sat, 2021-08-21 at 01:32 +1000, Daniel Axtens wrote: > The Patch and Cover comment views passed unsanitised patch_id/pk > down to the query set filter, which attempts to convert them to > integers, fails, and propagates a ValueError up to us. > > Rather than propagating it to the user as a 500, catch it explicitly > and return a 404. Ideally we'd like to return a 400 and some explanation, > but I can't see an easy way to bend Django or DRF to my will here. > > Signed-off-by: Daniel Axtens > > --- > > Will need some tweaking for 2.2, but conceptually simple. > > Stephen, would be interested if you can come up with a better way > to propagate a meaningful error out to the user! I did indeed find another way to do this. Two ways, in fact. I've submitted a patch to do both, but tl;dr: A. We can use path converters (of type 'int') to ensure these requests never even hit our handlers B. We can use 'rest_framework.generic.get_object_or_404' instead of Django's own implementation, since the former also handles TypeErrors Let me know if this works for you. I can apply it and cut a new release early next week if you can't. Cheers, Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 1/2] REST: Don't error if a versioned field we would remove is absent
On Sat, 2021-08-21 at 00:57 +1000, Daniel Axtens wrote:
> We remove fields that shouldn't be seen on old versions of the API.
> This was done with `pop(field name)`, which will throw an exception
> if the named field is absent from the data. However, sometimes if
> a patch request is via an old API version, we hit this line without
> ever having the field present.
>
> This is odd, but not harmful and we definitely shouldn't 500.
>
> Fixes: d944f17ec059 ("REST: Use versioning for modified responses")
> Signed-off-by: Daniel Axtens
Looks good to me.
Reviewed-by: Stephen Finucane
I squashed the test into this (with a tweak in the name) and applied it.
Thanks!
Stephen
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
[PATCH] urls: Add missing path converters for REST APIs
Almost all of the API endpoints expect numerical resource IDs, with
'/projects' being the sole exception. However, we were not actually
enforcing this anywhere. Instead, we were relying on the custom
'get_object_or_404' implementation used by 'GenericAPIView.retrieve' via
'GenericAPIView.get_object'. Unfortunately we weren't using this
everywhere, most notably in our handler for 'GET /patches/{id}/checks'.
The end result was a HTTP 500 due to a TypeError.
Resolve this by adding the path converters for all REST API paths in
'patchwork.urls', along with tests to prevent regressions going forward.
We also switch to the DRF variant of 'get_object_or_404' in some places
to provide additional protection.
Signed-off-by: Stephen Finucane
Cc: Daniel Axtens
---
patchwork/api/base.py | 3 ++-
patchwork/api/check.py | 7 +++
patchwork/tests/api/test_bundle.py | 11 +++
patchwork/tests/api/test_comment.py | 19 +--
patchwork/tests/api/test_cover.py | 11 +++
patchwork/tests/api/test_patch.py | 11 +++
patchwork/tests/api/test_person.py | 14 ++
patchwork/tests/api/test_project.py | 8
patchwork/tests/api/test_series.py | 11 +++
patchwork/tests/api/test_user.py| 14 ++
patchwork/urls.py | 20 ++--
11 files changed, 112 insertions(+), 17 deletions(-)
diff --git patchwork/api/base.py patchwork/api/base.py
index 6cb703b1..5368c0cb 100644
--- patchwork/api/base.py
+++ patchwork/api/base.py
@@ -59,7 +59,8 @@ class MultipleFieldLookupMixin(object):
queryset = self.filter_queryset(self.get_queryset())
filter_kwargs = {}
for field_name, field in zip(
-self.lookup_fields, self.lookup_url_kwargs):
+self.lookup_fields, self.lookup_url_kwargs,
+):
if self.kwargs[field]:
filter_kwargs[field_name] = self.kwargs[field]
diff --git patchwork/api/check.py patchwork/api/check.py
index a6bf5f8c..5137c1b9 100644
--- patchwork/api/check.py
+++ patchwork/api/check.py
@@ -3,11 +3,10 @@
#
# SPDX-License-Identifier: GPL-2.0-or-later
-from django.http import Http404
from django.http.request import QueryDict
-from django.shortcuts import get_object_or_404
import rest_framework
from rest_framework.exceptions import PermissionDenied
+from rest_framework.generics import get_object_or_404
from rest_framework.generics import ListCreateAPIView
from rest_framework.generics import RetrieveAPIView
from rest_framework.serializers import CurrentUserDefault
@@ -95,8 +94,8 @@ class CheckMixin(object):
def get_queryset(self):
patch_id = self.kwargs['patch_id']
-if not Patch.objects.filter(pk=self.kwargs['patch_id']).exists():
-raise Http404
+# ensure the patch exists
+get_object_or_404(Patch, id=self.kwargs['patch_id'])
return Check.objects.prefetch_related('user').filter(patch=patch_id)
diff --git patchwork/tests/api/test_bundle.py patchwork/tests/api/test_bundle.py
index 79924486..1ada79c3 100644
--- patchwork/tests/api/test_bundle.py
+++ patchwork/tests/api/test_bundle.py
@@ -6,6 +6,7 @@
import unittest
from django.conf import settings
+from django.urls import NoReverseMatch
from django.urls import reverse
from patchwork.models import Bundle
@@ -184,6 +185,16 @@ class TestBundleAPI(utils.APITestCase):
self.assertIn('url', resp.data)
self.assertNotIn('web_url', resp.data)
+def test_detail_non_existent(self):
+"""Ensure we get a 404 for a non-existent bundle."""
+resp = self.client.get(self.api_url('99'))
+self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
+
+def test_detail_invalid(self):
+"""Ensure we get a 404 for an invalid bundle ID."""
+with self.assertRaises(NoReverseMatch):
+self.client.get(self.api_url('foo'))
+
def _test_create_update(self, authenticate=True):
user = create_user()
project = create_project()
diff --git patchwork/tests/api/test_comment.py
patchwork/tests/api/test_comment.py
index 59450d8a..22638d2f 100644
--- patchwork/tests/api/test_comment.py
+++ patchwork/tests/api/test_comment.py
@@ -22,6 +22,7 @@ if settings.ENABLE_REST_API:
@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
class TestCoverComments(utils.APITestCase):
+
@staticmethod
def api_url(cover, version=None):
kwargs = {}
@@ -76,12 +77,19 @@ class TestCoverComments(utils.APITestCase):
with self.assertRaises(NoReverseMatch):
self.client.get(self.api_url(cover, version='1.0'))
-def test_list
Re: [PATCH v3 1/1] static: add rest.js to handle PATCH requests & respective responses
On Thu, 2021-08-19 at 04:20 +, Raxel Gutierrez wrote: > Add `rest.js` file to have a utilities JavaScript module that can be > reused by any Patchwork JS files that make requests to the REST API. The > comments for each function follow the Google JS Style guide [1] which is > something that would be nice to have for better documented frontend code, > especially for JS modules that export functions like rest.js. In > particular, this patch provides the following function: > > - `updateProperty`: make PATCH requests that partially update the >fields of an object given it's REST API endpoint specified by the >caller. Also, the caller can specify the field(s) to modify and the >associated content for update messages in the case of both failed >successful requests that render to the current webpage. The caller >receives whether the request was successful or not. > > The `rest.js` module can be further expanded to support and provide > functions that allow for other requests (e.g. GET, POST, PUT) to the > REST API. > > Also, add functions that handle update & error messages for these PATCH > requests that match the Django messages framework format and form error > styling. These functions are internal to the module and aren't exposed > outside of the `rest.js` file. > > Error and accompanying failed update messages are replaced by successful > update messages and vice versa. Consecutive successful update messages > add to a counter of updated objects. Consecutive error messages stack up. > > Signed-off-by: Raxel Gutierrez > Reviewed-by: Daniel Axtens All my issues are resolved and Daniel's happy Reviewed-by: Stephen Finucane and applied. Thanks! ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 1/2] htdocs: Move all jQuery files from 'lib'
On Wed, 2021-08-11 at 22:46 +0100, Stephen Finucane wrote: > These were previously symlinked, for reasons that I cannot fathom. Just > move them to where they are used. In the future, we will probably want > to manage these with a package manager. > > A README is removed as it mostly duplicated content already found in > 'htdocs/README.rst'. > > Signed-off-by: Stephen Finucane Applied. ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 2/2] htdocs: Fix formatting issues with README
On Wed, 2021-08-11 at 22:46 +0100, Stephen Finucane wrote: > You can't have spaces between a term and definition in a definition > list. > > Signed-off-by: Stephen Finucane Applied. ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
