From 7ebadc368cec12d5b5843785c86d37a25a40e26b Mon Sep 17 00:00:00 2001 From: Veronika Kabatova Date: Thu, 8 Feb 2018 14:30:04 +0100 Subject: [PATCH] Implementation of tagging for cover letters Solves #113 and partially #57 GitHub issues. * Change PatchTag to SubmissionTag to allow tagging of cover letters * Make CoverLetter tags increase tag counts on all patches in the series Signed-off-by: Veronika Kabatova --- patchwork/api/cover.py | 12 ++- patchwork/api/patch.py | 10 ++- patchwork/models.py | 130 +++++++++++++++++++++------------ patchwork/tests/test_parser.py | 12 +-- patchwork/tests/test_tags.py | 6 +- 5 files changed, 112 insertions(+), 58 deletions(-) diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py index 10645048..1c2eef9f 100644 --- a/patchwork/api/cover.py +++ b/patchwork/api/cover.py @@ -29,6 +29,7 @@ from patchwork.api.embedded import ProjectSerializer from patchwork.api.embedded import SeriesSerializer from patchwork.models import CoverLetter +from patchwork.models import SubmissionTag class CoverLetterListSerializer(HyperlinkedModelSerializer): @@ -37,15 +38,24 @@ class CoverLetterListSerializer(HyperlinkedModelSerializer): submitter = PersonSerializer(read_only=True) mbox = SerializerMethodField() series = SeriesSerializer(many=True, read_only=True) + tags = SerializerMethodField() def get_mbox(self, instance): request = self.context.get('request') return request.build_absolute_uri(instance.get_mbox_url()) + def get_tags(self, instance): + tag_ids = [tag.id for tag in instance.project.tags] + return {submissiontag.tag.name: submissiontag.count for + submissiontag in SubmissionTag.objects.filter( + submission_id=instance.id, + tag_id__in=tag_ids + )} + class Meta: model = CoverLetter fields = ('id', 'url', 'project', 'msgid', 'date', 'name', 'submitter', - 'mbox', 'series') + 'mbox', 'series', 'tags') read_only_fields = fields extra_kwargs = { 'url': {'view_name': 'api-cover-detail'}, diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index 115feffa..964808b3 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -35,6 +35,7 @@ from patchwork.api.embedded import UserSerializer from patchwork.models import Patch from patchwork.models import State +from patchwork.models import SubmissionTag from patchwork.parser import clean_subject @@ -92,9 +93,12 @@ def get_mbox(self, instance): return request.build_absolute_uri(instance.get_mbox_url()) def get_tags(self, instance): - # TODO(stephenfin): Make tags performant, possibly by reworking the - # model - return {} + tag_ids = [tag.id for tag in instance.project.tags] + return {submissiontag.tag.name: submissiontag.count for + submissiontag in SubmissionTag.objects.filter( + submission_id=instance.id, + tag_id__in=tag_ids + )} def get_check(self, instance): return instance.combined_check_state diff --git a/patchwork/models.py b/patchwork/models.py index 581dbf7a..9544faf5 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -253,13 +253,13 @@ class Meta: ordering = ['abbrev'] -class PatchTag(models.Model): - patch = models.ForeignKey('Patch', on_delete=models.CASCADE) +class SubmissionTag(models.Model): + submission = models.ForeignKey('Submission', on_delete=models.CASCADE) tag = models.ForeignKey('Tag', on_delete=models.CASCADE) count = models.IntegerField(default=1) class Meta: - unique_together = [('patch', 'tag')] + unique_together = [('submission', 'tag')] def get_default_initial_patch_state(): @@ -289,10 +289,10 @@ def with_tag_counts(self, project=None): for tag in tags: select[tag.attr_name] = ( "coalesce(" - "(SELECT count FROM patchwork_patchtag" - " WHERE patchwork_patchtag.patch_id=" + "(SELECT count FROM patchwork_submissiontag" + " WHERE patchwork_submissiontag.submission_id=" "patchwork_patch.submission_ptr_id" - " AND patchwork_patchtag.tag_id=%s), 0)") + " AND patchwork_submissiontag.tag_id=%s), 0)") select_params.append(tag.id) return qs.extra(select=select, select_params=select_params) @@ -367,9 +367,86 @@ class Submission(FilenameMixin, EmailMixin, models.Model): # submission metadata name = models.CharField(max_length=255) + submission_tags = models.ManyToManyField(Tag, through=SubmissionTag) # patchwork metadata + @staticmethod + def extract_tags(content, tags): + counts = Counter() + + for tag in tags: + regex = re.compile(tag.pattern, re.MULTILINE | re.IGNORECASE) + counts[tag] = len(regex.findall(content)) + + return counts + + def _set_tag(self, tag, count): + if count == 0: + self.submissiontag_set.filter(tag=tag).delete() + return + submissiontag, _ = SubmissionTag.objects.get_or_create(submission=self, + tag=tag) + if submissiontag.count != count: + submissiontag.count = count + submissiontag.save() + + def refresh_tag_counts(self): + tags = self.project.tags + counter = Counter() + + if self.content: + counter += self.extract_tags(self.content, tags) + + for comment in self.comments.all(): + counter = counter + self.extract_tags(comment.content, tags) + + # We need to find the series submission belongs to first + related_series = None + refs = [hdr.split(': ')[1] for hdr in self.headers.split('\n') if + hdr.split(': ')[0] == 'In-Reply-To' or + hdr.split(': ')[0] == 'References'] + [self.msgid] + for ref in refs: + try: + related_series = SeriesReference.objects.get( + msgid=ref, + series__project=self.project + ).series + except SeriesReference.DoesNotExist: + continue + + if related_series: + if hasattr(self, 'coverletter'): + # We tagged cover letter and need to update all patches in + # the series + for patch in related_series.patches.all(): + patch_counter = counter + + if patch.content: + patch_counter += patch.extract_tags(patch.content, + tags) + + for comment in patch.comments.all(): + patch_counter += patch.extract_tags(comment.content, + tags) + for tag in tags: + patch._set_tag(tag, patch_counter[tag]) + else: + # We tagged a patch but need to take into account all tags on + # the cover letter too + cover = related_series.cover_letter + if cover: + counter += cover.extract_tags(cover.content, tags) + for comment in cover.comments.all(): + counter += cover.extract_tags(comment.content, tags) + + for tag in tags: + self._set_tag(tag, counter[tag]) + + def save(self, *args, **kwargs): + super(Submission, self).save(**kwargs) + self.refresh_tag_counts() + def is_editable(self, user): return False @@ -413,7 +490,6 @@ class Patch(SeriesMixin, Submission): diff = models.TextField(null=True, blank=True) commit_ref = models.CharField(max_length=255, null=True, blank=True) pull_url = models.CharField(max_length=255, null=True, blank=True) - tags = models.ManyToManyField(Tag, through=PatchTag) # patchwork metadata @@ -425,38 +501,6 @@ class Patch(SeriesMixin, Submission): objects = PatchManager() - @staticmethod - def extract_tags(content, tags): - counts = Counter() - - for tag in tags: - regex = re.compile(tag.pattern, re.MULTILINE | re.IGNORECASE) - counts[tag] = len(regex.findall(content)) - - return counts - - def _set_tag(self, tag, count): - if count == 0: - self.patchtag_set.filter(tag=tag).delete() - return - patchtag, _ = PatchTag.objects.get_or_create(patch=self, tag=tag) - if patchtag.count != count: - patchtag.count = count - patchtag.save() - - def refresh_tag_counts(self): - tags = self.project.tags - counter = Counter() - - if self.content: - counter += self.extract_tags(self.content, tags) - - for comment in self.comments.all(): - counter = counter + self.extract_tags(comment.content, tags) - - for tag in tags: - self._set_tag(tag, counter[tag]) - def save(self, *args, **kwargs): if not hasattr(self, 'state') or not self.state: self.state = get_default_initial_patch_state() @@ -466,8 +510,6 @@ def save(self, *args, **kwargs): super(Patch, self).save(**kwargs) - self.refresh_tag_counts() - def is_editable(self, user): if not is_authenticated(user): return False @@ -591,13 +633,11 @@ class Comment(EmailMixin, models.Model): def save(self, *args, **kwargs): super(Comment, self).save(*args, **kwargs) - if hasattr(self.submission, 'patch'): - self.submission.patch.refresh_tag_counts() + self.submission.refresh_tag_counts() def delete(self, *args, **kwargs): super(Comment, self).delete(*args, **kwargs) - if hasattr(self.submission, 'patch'): - self.submission.patch.refresh_tag_counts() + self.submission.refresh_tag_counts() class Meta: ordering = ['date'] diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py index c2704a4d..e315ba95 100644 --- a/patchwork/tests/test_parser.py +++ b/patchwork/tests/test_parser.py @@ -783,11 +783,11 @@ def setUp(self): def test_tags(self): self.assertEqual(Patch.objects.count(), 1) patch = Patch.objects.all()[0] - self.assertEqual(patch.patchtag_set.filter( + self.assertEqual(patch.submissiontag_set.filter( tag__name='Acked-by').count(), 0) - self.assertEqual(patch.patchtag_set.get( + self.assertEqual(patch.submissiontag_set.get( tag__name='Reviewed-by').count, 1) - self.assertEqual(patch.patchtag_set.get( + self.assertEqual(patch.submissiontag_set.get( tag__name='Tested-by').count, 1) @@ -811,11 +811,11 @@ def setUp(self): def test_tags(self): self.assertEqual(Patch.objects.count(), 1) patch = Patch.objects.all()[0] - self.assertEqual(patch.patchtag_set.filter( + self.assertEqual(patch.submissiontag_set.filter( tag__name='Acked-by').count(), 0) - self.assertEqual(patch.patchtag_set.get( + self.assertEqual(patch.submissiontag_set.get( tag__name='Reviewed-by').count, 1) - self.assertEqual(patch.patchtag_set.get( + self.assertEqual(patch.submissiontag_set.get( tag__name='Tested-by').count, 1) diff --git a/patchwork/tests/test_tags.py b/patchwork/tests/test_tags.py index 4fd1bf23..cd7f8d06 100644 --- a/patchwork/tests/test_tags.py +++ b/patchwork/tests/test_tags.py @@ -21,7 +21,7 @@ from django.test import TransactionTestCase from patchwork.models import Patch -from patchwork.models import PatchTag +from patchwork.models import SubmissionTag from patchwork.models import Tag from patchwork.tests.utils import create_comment from patchwork.tests.utils import create_patch @@ -97,8 +97,8 @@ def assertTagsEqual(self, patch, acks, reviews, tests): # noqa def count(name): try: - return patch.patchtag_set.get(tag__name=name).count - except PatchTag.DoesNotExist: + return patch.submissiontag_set.get(tag__name=name).count + except SubmissionTag.DoesNotExist: return 0 counts = (