[PATCH 3/4] parser: allow to handle multiple projects under the same list
Stephen Finucane
stephen at that.guru
Tue May 30 08:50:30 AEST 2017
On Sat, 2017-05-27 at 20:17 +0200, Philippe Pepiot wrote:
> By adding a `subject_prefix` settings to Project. Mail will be
> assigned
> to project if List-Id match and prefix is present.
>
> Signed-off-by: Philippe Pepiot <phil at philpep.org>
Hmm, so I'm not entirely sure about this. On one hand, I understand why
it would be helpful to do this. However, commit '66a88a46' was supposed
to address this. Is there something that that change doesn't have that
you need to expose? I've CCd the authors/reviewers of that change to
get their input.
fwiw, in the longer term I'd like to add a label feature that would
track most of the labels found in subjects (i.e. everything except
'PATCH', 'RFC', 'nn/mm' and 'vNN' tags). This is decidedly a 2.1+ goal,
however.
Stephen
PS: Naturally enough, the rest of this series hinges on this feature so
I'll hold off on reviewing those yet.
> ---
> patchwork/migrations/0020_auto_20170527_1119.py | 37
> ++++++++++++++++++++
> patchwork/models.py | 6 +++-
> patchwork/parser.py | 36 +++++++++++++
> -------
> patchwork/tests/test_parser.py | 45
> +++++++++++++++++++++----
> 4 files changed, 104 insertions(+), 20 deletions(-)
> create mode 100644 patchwork/migrations/0020_auto_20170527_1119.py
>
> diff --git a/patchwork/migrations/0020_auto_20170527_1119.py
> b/patchwork/migrations/0020_auto_20170527_1119.py
> new file mode 100644
> index 0000000..59165e3
> --- /dev/null
> +++ b/patchwork/migrations/0020_auto_20170527_1119.py
> @@ -0,0 +1,37 @@
> +# -*- coding: utf-8 -*-
> +# Generated by Django 1.10.7 on 2017-05-27 11:19
> +from __future__ import unicode_literals
> +
> +from django.db import migrations, models
> +
> +
> +class Migration(migrations.Migration):
> +
> + dependencies = [
> + ('patchwork', '0019_userprofile_show_ids'),
> + ]
> +
> + operations = [
> + migrations.AlterModelOptions(
> + name='patch',
> + options={'base_manager_name': 'objects',
> 'verbose_name_plural': 'Patches'},
> + ),
> + migrations.AlterModelOptions(
> + name='series',
> + options={'ordering': ('date',), 'verbose_name_plural':
> 'Series'},
> + ),
> + migrations.AddField(
> + model_name='project',
> + name='subject_prefix',
> + field=models.CharField(blank=True, help_text=b'An
> optional prefix that mail subject must match. This allow to handle
> multiple project under the same list ID', max_length=255),
> + ),
> + migrations.AlterField(
> + model_name='project',
> + name='listid',
> + field=models.CharField(max_length=255),
> + ),
> + migrations.AlterUniqueTogether(
> + name='project',
> + unique_together=set([('listid', 'subject_prefix')]),
> + ),
> + ]
> diff --git a/patchwork/models.py b/patchwork/models.py
> index ee66ace..148c1f7 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -67,8 +67,11 @@ class Project(models.Model):
>
> linkname = models.CharField(max_length=255, unique=True)
> name = models.CharField(max_length=255, unique=True)
> - listid = models.CharField(max_length=255, unique=True)
> + listid = models.CharField(max_length=255)
> listemail = models.CharField(max_length=200)
> + subject_prefix = models.CharField(max_length=255, blank=True,
> help_text=(
> + 'An optional prefix that mail subject must match. This allow
> to '
> + 'handle multiple project under the same list ID'))
>
> # url metadata
>
> @@ -97,6 +100,7 @@ class Project(models.Model):
>
> class Meta:
> ordering = ['linkname']
> + unique_together = (('listid', 'subject_prefix'))
>
>
> @python_2_unicode_compatible
> diff --git a/patchwork/parser.py b/patchwork/parser.py
> index 6d4640a..e5af55b 100644
> --- a/patchwork/parser.py
> +++ b/patchwork/parser.py
> @@ -136,17 +136,26 @@ def clean_header(header):
> return normalise_space(header_str)
>
>
> -def find_project_by_id(list_id):
> - """Find a `project` object with given `list_id`."""
> - project = None
> - try:
> - project = Project.objects.get(listid=list_id)
> - except Project.DoesNotExist:
> - pass
> - return project
> +def find_project_by_id(list_id, prefixes):
> + """Find a `project` object with given `list_id` and optionally
> matching
> + given `prefixes`.
> +
> + If no prefixes matches, return the project with blank prefix.
> + If multiple prefixes matches, display a warning and return the
> project
> + matching the first prefix.
> + """
> + prefixes = prefixes + ['']
> + projects = Project.objects.filter(listid=list_id,
> + subject_prefix__in=prefixes)
> + projects = sorted(projects, key=lambda p:
> prefixes.index(p.subject_prefix))
> + if not projects:
> + return
> + if len([p for p in projects if p.subject_prefix != '']) > 1:
> + logger.warning('Find multiple projects matching prefixes
> %s', prefixes)
> + return projects[0]
>
>
> -def find_project_by_header(mail):
> +def find_project_by_header(mail, prefixes):
> project = None
> listid_res = [re.compile(r'.*<([^>]+)>.*', re.S),
> re.compile(r'^([\S]+)$', re.S)]
> @@ -164,7 +173,7 @@ def find_project_by_header(mail):
>
> listid = match.group(1)
>
> - project = find_project_by_id(listid)
> + project = find_project_by_id(listid, prefixes)
> if project:
> break
>
> @@ -789,10 +798,12 @@ def parse_mail(mail, list_id=None):
> logger.debug("Ignoring email due to 'ignore' hint")
> return
>
> + subject = mail.get('Subject')
> + prefixes = clean_subject(subject)[1]
> if list_id:
> - project = find_project_by_id(list_id)
> + project = find_project_by_id(list_id, prefixes)
> else:
> - project = find_project_by_header(mail)
> + project = find_project_by_header(mail, prefixes)
>
> if project is None:
> logger.error('Failed to find a project for email')
> @@ -802,7 +813,6 @@ def parse_mail(mail, list_id=None):
>
> msgid = mail.get('Message-Id').strip()
> author = find_author(mail)
> - subject = mail.get('Subject')
> name, prefixes = clean_subject(subject, [project.linkname])
> is_comment = subject_check(subject)
> x, n = parse_series_marker(prefixes)
> diff --git a/patchwork/tests/test_parser.py
> b/patchwork/tests/test_parser.py
> index 63231fa..0430f52 100644
> --- a/patchwork/tests/test_parser.py
> +++ b/patchwork/tests/test_parser.py
> @@ -477,6 +477,39 @@ class
> MultipleProjectPatchCommentTest(MultipleProjectPatchTest):
> Comment.objects.filter(submission=patch).count(), 1)
>
>
> +class MultipleProjectSameListTest(TestCase):
> + """Test that multiple projects can be handled by the same
> mailing-list with
> + `subject_prefix` setting"""
> +
> + def setUp(self):
> + self.p1 = create_project(listid='test.example.com')
> + self.p2 = create_project(listid='test.example.com',
> + subject_prefix='foo')
> + self.p3 = create_project(listid='test.example.com',
> + subject_prefix='bar')
> +
> + @staticmethod
> + def _parse_mail(subject):
> + email = create_email(content=SAMPLE_DIFF, subject=subject)
> + return parse_mail(email)
> +
> + def test_prefix_matching(self):
> + patch = self._parse_mail('[PATCH foo foobar] foo bar')
> + self.assertEqual(patch.project, self.p2)
> + patch = self._parse_mail('[PATCH foobar bar] foo bar')
> + self.assertEqual(patch.project, self.p3)
> +
> + def test_prefix_not_matching(self):
> + patch = self._parse_mail('[PATCH foobar] foo bar')
> + self.assertEqual(patch.project, self.p1)
> +
> + def test_multiple_matching(self):
> + patch = self._parse_mail('[PATCH foo foobar bar] meep')
> + self.assertEqual(patch.project, self.p2)
> + patch = self._parse_mail('[PATCH bar foobar foo] meep')
> + self.assertEqual(patch.project, self.p3)
> +
> +
> class ListIdHeaderTest(TestCase):
> """Test that we parse List-Id headers from mails correctly."""
>
> @@ -485,25 +518,25 @@ class ListIdHeaderTest(TestCase):
>
> def test_no_list_id(self):
> email = MIMEText('')
> - project = find_project_by_header(email)
> + project = find_project_by_header(email, [])
> self.assertEqual(project, None)
>
> def test_blank_list_id(self):
> email = MIMEText('')
> email['List-Id'] = ''
> - project = find_project_by_header(email)
> + project = find_project_by_header(email, [])
> self.assertEqual(project, None)
>
> def test_whitespace_list_id(self):
> email = MIMEText('')
> email['List-Id'] = ' '
> - project = find_project_by_header(email)
> + project = find_project_by_header(email, [])
> self.assertEqual(project, None)
>
> def test_substring_list_id(self):
> email = MIMEText('')
> email['List-Id'] = 'example.com'
> - project = find_project_by_header(email)
> + project = find_project_by_header(email, [])
> self.assertEqual(project, None)
>
> def test_short_list_id(self):
> @@ -511,13 +544,13 @@ class ListIdHeaderTest(TestCase):
> is only the list ID itself (without enclosing angle-
> brackets). """
> email = MIMEText('')
> email['List-Id'] = self.project.listid
> - project = find_project_by_header(email)
> + project = find_project_by_header(email, [])
> self.assertEqual(project, self.project)
>
> def test_long_list_id(self):
> email = MIMEText('')
> email['List-Id'] = 'Test text <%s>' % self.project.listid
> - project = find_project_by_header(email)
> + project = find_project_by_header(email, [])
> self.assertEqual(project, self.project)
>
>
More information about the Patchwork
mailing list