[PATCH 03/10] parsemail: Add series parsing

[Finucane, Stephen]

On 23 Jun 13:23, Andy Doan wrote:
> On 06/13/2016 05:41 AM, Stephen Finucane wrote:
> >It is now possible to parse and store series, so do just that.
> >The parsing at the moment is based on both RFC822 headers and
> >subject lines.
> >
> >Signed-off-by: Stephen Finucane <stephen.finucane at intel.com>
> >---

[snip]

> >-from patchwork.models import (Patch, Project, Person, Comment, State,
> >-                              DelegationRule, Submission, CoverLetter,
> >-                              get_default_initial_patch_state)
> >-from patchwork.parser import parse_patch, patch_get_filenames
> >+from patchwork.models import Comment
> >+from patchwork.models import CoverLetter
> >+from patchwork.models import DelegationRule
> >+from patchwork.models import get_default_initial_patch_state
> >+from patchwork.models import Patch
> >+from patchwork.models import Person
> >+from patchwork.models import Project
> >+from patchwork.models import SeriesRevision
> >+from patchwork.models import SeriesReference
> >+from patchwork.models import State
> >+from patchwork.models import Submission
> >+
> >+from patchwork.parser import parse_patch
> >+from patchwork.parser import patch_get_filenames
> 
> kind of an odd way to import. Why not the more common:
> 
> from patchwork.models import (
>     Comment,
>     CoverLetter,
>     ...,
> )

This is a long play from me. I'd like to move to module level imports
only, and plan to do that once we've done the REST API and series work.
In addition, this is slightly easier to modify, IMO.

> >  LOGGER = logging.getLogger(__name__)
> >
> >@@ -114,6 +126,31 @@ def find_project_by_header(mail):
> >      return project

[snip]

> >@@ -497,9 +560,24 @@ def parse_mail(mail, list_id=None):
> >              filenames = patch_get_filenames(diff)
> >              delegate = auto_delegate(project, filenames)
> >
> >+        # TODO(stephenfin) Eventually this should be moved to a function
> >+        series = find_series(mail)
> >+        if not series and n:  # the series markers indicates a series
> >+            series = SeriesRevision(date=date,
> >+                                    submitter=author,
> >+                                    version=version,
> >+                                    total=n)
> >+            series.save()
> >+
> >+            for ref in refs + [msgid]:  # save references for series
> >+                series_ref = SeriesReference(series=series,
> >+                                             msgid=ref)
> >+                series_ref.save()
> 
> A bit of nitpicking, but you can do this in one line with:
>     SeriesReference.objects.create(series=series, msgid=ref)

Good spot :) I'll fix that in v2.

> >          patch = Patch(
> >              msgid=msgid,
> >              project=project,
> >+            series=series,
> >              name=name,
> >              date=date,
> >              headers=headers,
> >@@ -529,9 +607,23 @@ def parse_mail(mail, list_id=None):
> >          if is_cover_letter:
> >              author.save()
> >
> >+            series = find_series(mail)
> >+            if not series:
> >+                series = SeriesRevision(date=date,
> >+                                        submitter=author,
> >+                                        version=version,
> >+                                        total=n)
> >+                series.save()
> >+
> >+                for ref in refs + [msgid]:  # save references for series
> >+                    series_ref = SeriesReference(series=series,
> >+                                                 msgid=ref)
> >+                    series_ref.save()
> >+
> >              cover_letter = CoverLetter(
> >                  msgid=msgid,
> >                  project=project,
> >+                series=series,
> >                  name=name,
> >                  date=date,
> >                  headers=headers,
> 
> I still get nervous about not having unit tests for this as we
> develop this. Then we could have tests demonstrating which kinds of
> patch-series emails this supports.

Yeah, I merged cover letter support without many, but I'm fixing that
now. Tests will be added in the next revision of this series.
Hopefully my work on reworking the tests should pay off dividends here.