diff options
| -rw-r--r-- | lib/sql/grant-all.mysql.sql | 2 | ||||
| -rw-r--r-- | lib/sql/grant-all.postgres.sql | 4 | ||||
| -rw-r--r-- | patchwork/admin.py | 2 | ||||
| -rw-r--r-- | patchwork/api/cover.py | 19 | ||||
| -rw-r--r-- | patchwork/api/patch.py | 20 | ||||
| -rw-r--r-- | patchwork/migrations/0031_add_patch_series_fields.py | 32 | ||||
| -rw-r--r-- | patchwork/migrations/0032_migrate_data_from_series_patch_to_patch.py | 35 | ||||
| -rw-r--r-- | patchwork/migrations/0033_remove_patch_series_model.py | 58 | ||||
| -rw-r--r-- | patchwork/models.py | 60 | ||||
| -rw-r--r-- | patchwork/parser.py | 6 | ||||
| -rw-r--r-- | patchwork/signals.py | 40 | ||||
| -rw-r--r-- | patchwork/templates/patchwork/partials/download-buttons.html | 17 | ||||
| -rw-r--r-- | patchwork/templates/patchwork/partials/patch-list.html | 12 | ||||
| -rw-r--r-- | patchwork/templates/patchwork/submission.html | 26 | ||||
| -rw-r--r-- | patchwork/tests/test_detail.py | 16 | ||||
| -rw-r--r-- | patchwork/tests/test_events.py | 11 | ||||
| -rw-r--r-- | patchwork/tests/test_series.py | 32 | ||||
| -rw-r--r-- | patchwork/tests/utils.py | 24 | ||||
| -rw-r--r-- | patchwork/views/cover.py | 1 | ||||
| -rw-r--r-- | patchwork/views/patch.py | 3 | ||||
| -rw-r--r-- | patchwork/views/utils.py | 17 |
21 files changed, 263 insertions, 174 deletions
diff --git a/lib/sql/grant-all.mysql.sql b/lib/sql/grant-all.mysql.sql index ff96219..0277077 100644 --- a/lib/sql/grant-all.mysql.sql +++ b/lib/sql/grant-all.mysql.sql @@ -27,7 +27,6 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_patchtag TO 'www-data'@localho GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_person TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_project TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_series TO 'www-data'@localhost; -GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_seriespatch TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_seriesreference TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_state TO 'www-data'@localhost; GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_submission TO 'www-data'@localhost; @@ -42,7 +41,6 @@ GRANT INSERT, SELECT ON patchwork_coverletter TO 'nobody'@localhost; GRANT INSERT, SELECT ON patchwork_patch TO 'nobody'@localhost; GRANT INSERT, SELECT ON patchwork_person TO 'nobody'@localhost; GRANT INSERT, SELECT ON patchwork_series TO 'nobody'@localhost; -GRANT INSERT, SELECT ON patchwork_seriespatch TO 'nobody'@localhost; GRANT INSERT, SELECT ON patchwork_seriesreference TO 'nobody'@localhost; GRANT INSERT, SELECT ON patchwork_submission TO 'nobody'@localhost; GRANT INSERT, SELECT, UPDATE, DELETE ON patchwork_patchtag TO 'nobody'@localhost; diff --git a/lib/sql/grant-all.postgres.sql b/lib/sql/grant-all.postgres.sql index 27f55c9..8f500e9 100644 --- a/lib/sql/grant-all.postgres.sql +++ b/lib/sql/grant-all.postgres.sql @@ -27,7 +27,6 @@ GRANT SELECT, UPDATE, INSERT, DELETE ON patchwork_person, patchwork_project, patchwork_series, - patchwork_seriespatch, patchwork_seriesreference, patchwork_state, patchwork_submission, @@ -56,7 +55,6 @@ GRANT SELECT, UPDATE ON patchwork_person_id_seq, patchwork_project_id_seq, patchwork_series_id_seq, - patchwork_seriespatch_id_seq, patchwork_seriesreference_id_seq, patchwork_state_id_seq, patchwork_tag_id_seq, @@ -70,7 +68,6 @@ GRANT INSERT, SELECT ON patchwork_comment, patchwork_coverletter, patchwork_event - patchwork_seriespatch, patchwork_seriesreference, patchwork_submission, TO "nobody"; @@ -93,7 +90,6 @@ GRANT UPDATE, SELECT ON patchwork_patchtag_id_seq, patchwork_person_id_seq, patchwork_series_id_seq, - patchwork_seriespatch_id_seq, patchwork_seriesreference_id_seq, TO "nobody"; diff --git a/patchwork/admin.py b/patchwork/admin.py index f12b338..9254167 100644 --- a/patchwork/admin.py +++ b/patchwork/admin.py @@ -114,7 +114,7 @@ admin.site.register(Comment, CommentAdmin) class PatchInline(admin.StackedInline): - model = Series.patches.through + model = Patch extra = 0 diff --git a/patchwork/api/cover.py b/patchwork/api/cover.py index 40f8c35..b9e36f2 100644 --- a/patchwork/api/cover.py +++ b/patchwork/api/cover.py @@ -24,7 +24,7 @@ class CoverLetterListSerializer(BaseHyperlinkedModelSerializer): project = ProjectSerializer(read_only=True) submitter = PersonSerializer(read_only=True) mbox = SerializerMethodField() - series = SeriesSerializer(many=True, read_only=True) + series = SeriesSerializer(read_only=True) comments = SerializerMethodField() def get_web_url(self, instance): @@ -39,6 +39,15 @@ class CoverLetterListSerializer(BaseHyperlinkedModelSerializer): return self.context.get('request').build_absolute_uri( reverse('api-cover-comment-list', kwargs={'pk': cover.id})) + def to_representation(self, instance): + # NOTE(stephenfin): This is here to ensure our API looks the same even + # after we changed the series-patch relationship from M:N to 1:N. It + # will be removed in API v2 + data = super(CoverLetterListSerializer, self).to_representation( + instance) + data['series'] = [data['series']] + return data + class Meta: model = CoverLetter fields = ('id', 'url', 'web_url', 'project', 'msgid', 'date', 'name', @@ -89,8 +98,8 @@ class CoverLetterList(ListAPIView): ordering = 'id' def get_queryset(self): - return CoverLetter.objects.all().prefetch_related('series')\ - .select_related('project', 'submitter')\ + return CoverLetter.objects.all()\ + .select_related('project', 'submitter', 'series')\ .defer('content', 'headers') @@ -100,5 +109,5 @@ class CoverLetterDetail(RetrieveAPIView): serializer_class = CoverLetterDetailSerializer def get_queryset(self): - return CoverLetter.objects.all().prefetch_related('series')\ - .select_related('project', 'submitter') + return CoverLetter.objects.all()\ + .select_related('project', 'submitter', 'series') diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index 92423cb..6367dd5 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -70,7 +70,7 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer): submitter = PersonSerializer(read_only=True) delegate = UserSerializer(allow_null=True) mbox = SerializerMethodField() - series = SeriesSerializer(many=True, read_only=True) + series = SeriesSerializer(read_only=True) comments = SerializerMethodField() check = SerializerMethodField() checks = SerializerMethodField() @@ -111,6 +111,14 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer): "'%s'" % (value, self.instance.project)) return value + def to_representation(self, instance): + # NOTE(stephenfin): This is here to ensure our API looks the same even + # after we changed the series-patch relationship from M:N to 1:N. It + # will be removed in API v2 + data = super(PatchListSerializer, self).to_representation(instance) + data['series'] = [data['series']] + return data + class Meta: model = Patch fields = ('id', 'url', 'web_url', 'project', 'msgid', 'date', 'name', @@ -173,8 +181,9 @@ class PatchList(ListAPIView): def get_queryset(self): return Patch.objects.all()\ - .prefetch_related('series', 'check_set')\ - .select_related('project', 'state', 'submitter', 'delegate')\ + .prefetch_related('check_set')\ + .select_related('project', 'state', 'submitter', 'delegate', + 'series')\ .defer('content', 'diff', 'headers') @@ -186,5 +195,6 @@ class PatchDetail(RetrieveUpdateAPIView): def get_queryset(self): return Patch.objects.all()\ - .prefetch_related('series', 'check_set')\ - .select_related('project', 'state', 'submitter', 'delegate') + .prefetch_related('check_set')\ + .select_related('project', 'state', 'submitter', 'delegate', + 'series') diff --git a/patchwork/migrations/0031_add_patch_series_fields.py b/patchwork/migrations/0031_add_patch_series_fields.py new file mode 100644 index 0000000..bfe4385 --- /dev/null +++ b/patchwork/migrations/0031_add_patch_series_fields.py @@ -0,0 +1,32 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +from django.db.models import Count +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('patchwork', '0030_add_submission_covering_index'), + ] + + operations = [ + # Add Patch.series_alt, Patch.number fields. This will store the fields + # currently stored in SeriesPatch + migrations.AddField( + model_name='patch', + name='number', + field=models.PositiveSmallIntegerField(default=None, help_text=b'The number assigned to this patch in the series', null=True), + ), + migrations.AddField( + model_name='patch', + name='series_alt', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='patchwork.Series'), + ), + migrations.AlterUniqueTogether( + name='patch', + unique_together=set([('series_alt', 'number')]), + ), + ] diff --git a/patchwork/migrations/0032_migrate_data_from_series_patch_to_patch.py b/patchwork/migrations/0032_migrate_data_from_series_patch_to_patch.py new file mode 100644 index 0000000..78e8642 --- /dev/null +++ b/patchwork/migrations/0032_migrate_data_from_series_patch_to_patch.py @@ -0,0 +1,35 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +from django.db.models import Count +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('patchwork', '0031_add_patch_series_fields'), + ] + + operations = [ + # Copy SeriesPatch.series, SeriesPatch.number to Patch.series_alt, + # Patch.number. Note that there is no uniqueness check here because no + # code actually allowed us to save multiple series + migrations.RunSQL( + """UPDATE patchwork_patch SET series_alt_id = + (SELECT series_id from patchwork_seriespatch + WHERE patchwork_seriespatch.patch_id = + patchwork_patch.submission_ptr_id); + UPDATE patchwork_patch SET number = + (SELECT number from patchwork_seriespatch + WHERE patchwork_seriespatch.patch_id = + patchwork_patch.submission_ptr_id); + """, + """INSERT INTO patchwork_seriespatch + (patch_id, series_id, number) + SELECT submission_ptr_id, series_alt_id, number + FROM patchwork_patch; + """, + ), + ] diff --git a/patchwork/migrations/0033_remove_patch_series_model.py b/patchwork/migrations/0033_remove_patch_series_model.py new file mode 100644 index 0000000..a9ea2e2 --- /dev/null +++ b/patchwork/migrations/0033_remove_patch_series_model.py @@ -0,0 +1,58 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('patchwork', '0032_migrate_data_from_series_patch_to_patch'), + ] + + operations = [ + # Remove SeriesPatch + migrations.AlterUniqueTogether( + name='seriespatch', + unique_together=set([]), + ), + migrations.RemoveField( + model_name='seriespatch', + name='patch', + ), + migrations.RemoveField( + model_name='seriespatch', + name='series', + ), + migrations.RemoveField( + model_name='series', + name='patches', + ), + migrations.DeleteModel( + name='SeriesPatch', + ), + # Now that SeriesPatch has been removed, we can use the now-unused + # Patch.series field and add a backreference + migrations.RenameField( + model_name='patch', + old_name='series_alt', + new_name='series', + ), + migrations.AlterField( + model_name='patch', + name='series', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='patches', related_query_name='patch', to='patchwork.Series'), + ), + migrations.AlterUniqueTogether( + name='patch', + unique_together=set([('series', 'number')]), + ), + # Migrate CoverLetter to OneToOneField as a cover letter can no longer + # be assigned to multiple series + migrations.AlterField( + model_name='series', + name='cover_letter', + field=models.OneToOneField(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='series', to='patchwork.CoverLetter'), + ), + ] diff --git a/patchwork/models.py b/patchwork/models.py index a043844..a483dc6 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -407,6 +407,15 @@ class Patch(Submission): # patches in a project without needing to do a JOIN. patch_project = models.ForeignKey(Project, on_delete=models.CASCADE) + # series metadata + + series = models.ForeignKey( + 'Series', null=True, blank=True, on_delete=models.CASCADE, + related_name='patches', related_query_name='patch') + number = models.PositiveSmallIntegerField( + default=None, null=True, + help_text='The number assigned to this patch in the series') + objects = PatchManager() @staticmethod @@ -563,6 +572,7 @@ class Patch(Submission): class Meta: verbose_name_plural = 'Patches' base_manager_name = 'objects' + unique_together = [('series', 'number')] indexes = [ # This is a covering index for the /list/ query @@ -606,19 +616,16 @@ class Comment(EmailMixin, models.Model): @python_2_unicode_compatible class Series(FilenameMixin, models.Model): - """An collection of patches.""" + """A collection of patches.""" # parent project = models.ForeignKey(Project, related_name='series', null=True, blank=True, on_delete=models.CASCADE) # content - cover_letter = models.ForeignKey(CoverLetter, - related_name='series', - null=True, blank=True, - on_delete=models.CASCADE) - patches = models.ManyToManyField(Patch, through='SeriesPatch', - related_name='series') + cover_letter = models.OneToOneField(CoverLetter, related_name='series', + null=True, + on_delete=models.CASCADE) # metadata name = models.CharField(max_length=255, blank=True, null=True, @@ -684,9 +691,8 @@ class Series(FilenameMixin, models.Model): self.name = self._format_name(cover) else: try: - name = SeriesPatch.objects.get(series=self, - number=1).patch.name - except SeriesPatch.DoesNotExist: + name = Patch.objects.get(series=self, number=1).name + except Patch.DoesNotExist: name = None if self.name == name: @@ -696,20 +702,16 @@ class Series(FilenameMixin, models.Model): def add_patch(self, patch, number): """Add a patch to the series.""" - # see if the patch is already in this series - if SeriesPatch.objects.filter(series=self, patch=patch).count(): - # TODO(stephenfin): We may wish to raise an exception here in the - # future - return - # both user defined names and cover letter-based names take precedence if not self.name and number == 1: self.name = patch.name # keep the prefixes for patch-based names self.save() - return SeriesPatch.objects.create(series=self, - patch=patch, - number=number) + patch.series = self + patch.number = number + patch.save() + + return patch def get_absolute_url(self): # TODO(stephenfin): We really need a proper series view @@ -728,26 +730,6 @@ class Series(FilenameMixin, models.Model): @python_2_unicode_compatible -class SeriesPatch(models.Model): - """A patch in a series. - - Patches can belong to many series. This allows for things like - auto-completion of partial series. - """ - patch = models.ForeignKey(Patch, on_delete=models.CASCADE) - series = models.ForeignKey(Series, on_delete=models.CASCADE) - number = models.PositiveSmallIntegerField( - help_text='The number assigned to this patch in the series') - - def __str__(self): - return self.patch.name - - class Meta: - unique_together = [('series', 'patch'), ('series', 'number')] - ordering = ['number'] - - -@python_2_unicode_compatible class SeriesReference(models.Model): """A reference found in a series. diff --git a/patchwork/parser.py b/patchwork/parser.py index 2ba1db7..bc9dae2 100644 --- a/patchwork/parser.py +++ b/patchwork/parser.py @@ -27,7 +27,6 @@ from patchwork.models import Person from patchwork.models import Project from patchwork.models import Series from patchwork.models import SeriesReference -from patchwork.models import SeriesPatch from patchwork.models import State from patchwork.models import Submission @@ -1034,8 +1033,7 @@ def parse_mail(mail, list_id=None): # - there is no existing series to assign this patch to, or # - there is an existing series, but it already has a patch with this # number in it - if not series or ( - SeriesPatch.objects.filter(series=series, number=x).count()): + if not series or Patch.objects.filter(series=series, number=x).count(): series = Series(project=project, date=date, submitter=author, @@ -1069,6 +1067,8 @@ def parse_mail(mail, list_id=None): # patch. Don't add unnumbered patches (for example diffs sent # in reply, or just messages with random refs/in-reply-tos) if series and x: + # TODO(stephenfin): Remove 'series' from the conditional as we will + # always have a series series.add_patch(patch, x) return patch diff --git a/patchwork/signals.py b/patchwork/signals.py index b7b8e6f..536b177 100644 --- a/patchwork/signals.py +++ b/patchwork/signals.py @@ -15,7 +15,6 @@ from patchwork.models import Event from patchwork.models import Patch from patchwork.models import PatchChangeNotification from patchwork.models import Series -from patchwork.models import SeriesPatch @receiver(pre_save, sender=Patch) @@ -133,39 +132,46 @@ def create_patch_delegated_event(sender, instance, raw, **kwargs): create_event(instance, orig_patch.delegate, instance.delegate) -@receiver(post_save, sender=SeriesPatch) -def create_patch_completed_event(sender, instance, created, raw, **kwargs): - """Create patch completed event for patches with series.""" +@receiver(pre_save, sender=Patch) +def create_patch_completed_event(sender, instance, raw, **kwargs): - def create_event(patch, series): + def create_event(patch): return Event.objects.create( category=Event.CATEGORY_PATCH_COMPLETED, project=patch.project, patch=patch, - series=series) + series=patch.series) - # don't trigger for items loaded from fixtures or existing items - if raw or not created: + # don't trigger for items loaded from fixtures, new items or items that + # (still) don't have a series + if raw or not instance.pk or not instance.series: + return + + orig_patch = Patch.objects.get(pk=instance.pk) + + # we don't currently allow users to change a series, though this might + # change in the future. However, we handle that here nonetheless + if orig_patch.series == instance.series: return # if dependencies not met, don't raise event. There's also no point raising # events for successors since they'll have the same issue - predecessors = SeriesPatch.objects.filter( + predecessors = Patch.objects.filter( series=instance.series, number__lt=instance.number) if predecessors.count() != instance.number - 1: return - create_event(instance.patch, instance.series) + create_event(instance) # if this satisfies dependencies for successor patch, raise events for # those count = instance.number + 1 - for successor in SeriesPatch.objects.filter( + for successor in Patch.objects.order_by('number').filter( series=instance.series, number__gt=instance.number): if successor.number != count: break - create_event(successor.patch, successor.series) + create_event(successor) count += 1 @@ -204,15 +210,9 @@ def create_series_created_event(sender, instance, created, raw, **kwargs): create_event(instance) -@receiver(post_save, sender=SeriesPatch) +@receiver(post_save, sender=Patch) def create_series_completed_event(sender, instance, created, raw, **kwargs): - # NOTE(stephenfin): We subscribe to the SeriesPatch.post_save signal - # instead of Series.m2m_changed to minimize the amount of times this is - # fired. The m2m_changed signal doesn't support a 'changed' parameter, - # which we could use to quick skip the signal when a patch is merely - # updated instead of added to the series. - # NOTE(stephenfin): It's actually possible for this event to be fired # multiple times for a given series. To trigger this case, you would need # to send an additional patch to already exisiting series. This pattern @@ -229,5 +229,5 @@ def create_series_completed_event(sender, instance, created, raw, **kwargs): if raw or not created: return - if instance.series.received_all: + if instance.series and instance.series.received_all: create_event(instance.series) diff --git a/patchwork/templates/patchwork/partials/download-buttons.html b/patchwork/templates/patchwork/partials/download-buttons.html index 32acf26..21933bd 100644 --- a/patchwork/templates/patchwork/partials/download-buttons.html +++ b/patchwork/templates/patchwork/partials/download-buttons.html @@ -15,22 +15,9 @@ class="btn btn-default" role="button" title="Download cover mbox" >mbox</a> {% endif %} - {% if all_series|length == 1 %} - {% with all_series|first as series %} - <a href="{% url 'series-mbox' series_id=series.id %}" + {% if submission.series %} + <a href="{% url 'series-mbox' series_id=submission.series.id %}" class="btn btn-default" role="button" title="Download patch mbox with dependencies">series</a> - {% endwith %} - {% elif all_series|length > 1 %} - <button type="button" class="btn btn-default dropdown-toggle" - data-toggle="dropdown"> - series <span class="caret"></span> - </button> - <ul class="dropdown-menu" role="menu"> - {% for series in all_series %} - <li><a href="{% url 'series-mbox' series_id=series.id %}" - >{{ series }}</a></li> - {% endfor %} - </ul> {% endif %} </div> diff --git a/patchwork/templates/patchwork/partials/patch-list.html b/patchwork/templates/patchwork/partials/patch-list.html index 2abb9ec..2d090d9 100644 --- a/patchwork/templates/patchwork/partials/patch-list.html +++ b/patchwork/templates/patchwork/partials/patch-list.html @@ -194,13 +194,11 @@ $(document).ready(function() { </a> </td> <td> - {% with patch.series.all.0 as series %} - {% if series %} - <a href="?series={{series.id}}"> - {{ series|truncatechars:100 }} - </a> - {% endif %} - {% endwith %} + {% if patch.series %} + <a href="?series={{patch.series.id}}"> + {{ patch.series|truncatechars:100 }} + </a> + {% endif %} </td> <td class="text-nowrap">{{ patch|patch_tags }}</td> <td class="text-nowrap">{{ patch|patch_checks }}</td> diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html index eb5e358..b1ae542 100644 --- a/patchwork/templates/patchwork/submission.html +++ b/patchwork/templates/patchwork/submission.html @@ -64,25 +64,13 @@ function toggle_div(link_id, headers_id) </div> </td> </tr> -{% if all_series %} +{% if submission.series %} <tr> <th>Series</th> <td> - <div class="patchrelations"> - <ul> - {% for series in all_series %} - <li> - {% if forloop.first %} - {{ series }} - {% else %} - <a href="{% url 'patch-list' project_id=project.linkname %}?series={{ series.id }}"> - {{ series }} - </a> - {% endif %} - </li> - {% endfor %} - </ul> - </div> + <a href="{% url 'patch-list' project_id=project.linkname %}?series={{ submission.series.id }}"> + {{ submission.series }} + </a> </td> </tr> <tr> @@ -92,9 +80,8 @@ function toggle_div(link_id, headers_id) href="javascript:toggle_div('togglepatchrelations', 'patchrelations')" >show</a> <div id="patchrelations" class="patchrelations" style="display:none;"> - {% for series in all_series %} <ul> - {% with series.cover_letter as cover %} + {% with submission.series.cover_letter as cover %} <li> {% if cover %} {% if cover == submission %} @@ -107,7 +94,7 @@ function toggle_div(link_id, headers_id) {% endif %} </li> {% endwith %} - {% for sibling in series.patches.all %} + {% for sibling in submission.series.patches.all %} <li> {% if sibling == submission %} {{ sibling.name|default:"[no subject]"|truncatechars:100 }} @@ -119,7 +106,6 @@ function toggle_div(link_id, headers_id) </li> {% endfor %} </ul> - {% endfor %} </div> </td> </tr> diff --git a/patchwork/tests/test_detail.py b/patchwork/tests/test_detail.py index 9c44779..4ca1c9c 100644 --- a/patchwork/tests/test_detail.py +++ b/patchwork/tests/test_detail.py @@ -9,7 +9,6 @@ from django.urls import reverse from patchwork.tests.utils import create_comment from patchwork.tests.utils import create_cover from patchwork.tests.utils import create_patch -from patchwork.tests.utils import create_series class CoverLetterViewTest(TestCase): @@ -35,21 +34,6 @@ class PatchViewTest(TestCase): response = self.client.get(requested_url) self.assertRedirects(response, redirect_url) - def test_series_dropdown(self): - patch = create_patch() - series = [create_series() for x in range(5)] - - for series_ in series: - series_.add_patch(patch, 1) - - response = self.client.get( - reverse('patch-detail', kwargs={'patch_id': patch.id})) - - for series_ in series: - self.assertContains( - response, - reverse('series-mbox', kwargs={'series_id': series_.id})) - class CommentRedirectTest(TestCase): diff --git a/patchwork/tests/test_events.py b/patchwork/tests/test_events.py index b9952f6..bd4f9be 100644 --- a/patchwork/tests/test_events.py +++ b/patchwork/tests/test_events.py @@ -34,8 +34,8 @@ class PatchCreateTest(_BaseTestCase): """No series, so patch dependencies implicitly exist.""" patch = utils.create_patch() - # This should raise both the CATEGORY_PATCH_CREATED and - # CATEGORY_PATCH_COMPLETED events as there are no specific dependencies + # This should raise the CATEGORY_PATCH_CREATED event only as there is + # no series events = _get_events(patch=patch) self.assertEqual(events.count(), 1) self.assertEqual(events[0].category, Event.CATEGORY_PATCH_CREATED) @@ -57,6 +57,13 @@ class PatchCreateTest(_BaseTestCase): self.assertEventFields(events[0]) self.assertEventFields(events[1]) + # This shouldn't be affected by another update to the patch + series_patch.patch.commit_ref = 'aac76f0b0f8dd657ff07bb' + series_patch.patch.save() + + events = _get_events(patch=series_patch.patch) + self.assertEqual(events.count(), 2) + def test_patch_dependencies_out_of_order(self): series = utils.create_series() series_patch_3 = utils.create_series_patch(series=series, number=3) diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py index ff3412e..f5e6b5b 100644 --- a/patchwork/tests/test_series.py +++ b/patchwork/tests/test_series.py @@ -73,7 +73,15 @@ class _BaseTestCase(TestCase): patches_ = patches[start_idx:end_idx] for patch in patches_: - self.assertEqual(patch.series.first(), series[idx]) + self.assertEqual(patch.series, series[idx]) + + # TODO(stephenfin): Rework this function into two different + # functions - we're clearly not always testing patches here + if isinstance(patch, models.Patch): + self.assertEqual(series[idx].patches.get(id=patch.id), + patch) + else: + self.assertEqual(series[idx].cover_letter, patch) start_idx = end_idx @@ -517,7 +525,7 @@ class SeriesTotalTest(_BaseTestCase): self.assertSerialized(patches, [1]) self.assertSerialized(covers, [1]) - series = patches[0].series.first() + series = patches[0].series self.assertFalse(series.received_all) def test_complete(self): @@ -537,7 +545,7 @@ class SeriesTotalTest(_BaseTestCase): self.assertSerialized(covers, [1]) self.assertSerialized(patches, [2]) - series = patches[0].series.first() + series = patches[0].series self.assertTrue(series.received_all) def test_extra_patches(self): @@ -558,7 +566,7 @@ class SeriesTotalTest(_BaseTestCase): self.assertSerialized(covers, [1]) self.assertSerialized(patches, [3]) - series = patches[0].series.first() + series = patches[0].series self.assertTrue(series.received_all) @@ -639,13 +647,13 @@ class SeriesNameTestCase(TestCase): cover = self._parse_mail(mbox[0]) cover_name = 'A sample series' - self.assertEqual(cover.series.first().name, cover_name) + self.assertEqual(cover.series.name, cover_name) self._parse_mail(mbox[1]) - self.assertEqual(cover.series.first().name, cover_name) + self.assertEqual(cover.series.name, cover_name) self._parse_mail(mbox[2]) - self.assertEqual(cover.series.first().name, cover_name) + self.assertEqual(cover.series.name, cover_name) mbox.close() @@ -663,7 +671,7 @@ class SeriesNameTestCase(TestCase): mbox = self._get_mbox('base-no-cover-letter.mbox') patch = self._parse_mail(mbox[0]) - series = patch.series.first() + series = patch.series self.assertEqual(series.name, patch.name) self._parse_mail(mbox[1]) @@ -687,13 +695,13 @@ class SeriesNameTestCase(TestCase): mbox = self._get_mbox('base-out-of-order.mbox') patch = self._parse_mail(mbox[0]) - self.assertIsNone(patch.series.first().name) + self.assertIsNone(patch.series.name) patch = self._parse_mail(mbox[1]) - self.assertEqual(patch.series.first().name, patch.name) + self.assertEqual(patch.series.name, patch.name) cover = self._parse_mail(mbox[2]) - self.assertEqual(cover.series.first().name, 'A sample series') + self.assertEqual(cover.series.name, 'A sample series') mbox.close() @@ -712,7 +720,7 @@ class SeriesNameTestCase(TestCase): """ mbox = self._get_mbox('base-out-of-order.mbox') - series = self._parse_mail(mbox[0]).series.first() + series = self._parse_mail(mbox[0]).series self.assertIsNone(series.name) series_name = 'My custom series name' diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py index d280fd6..0c3bbff 100644 --- a/patchwork/tests/utils.py +++ b/patchwork/tests/utils.py @@ -19,7 +19,6 @@ from patchwork.models import Patch from patchwork.models import Person from patchwork.models import Project from patchwork.models import Series -from patchwork.models import SeriesPatch from patchwork.models import SeriesReference from patchwork.models import State from patchwork.tests import TEST_PATCH_DIR @@ -228,17 +227,24 @@ def create_series(**kwargs): def create_series_patch(**kwargs): - """Create 'SeriesPatch' object.""" + """Create 'Patch' object and associate with a series.""" + # TODO(stephenfin): Remove this and all callers num = 1 if 'series' not in kwargs else kwargs['series'].patches.count() + 1 + if 'number' in kwargs: + num = kwargs['number'] - values = { - 'series': create_series() if 'series' not in kwargs else None, - 'number': num, - 'patch': create_patch() if 'patch' not in kwargs else None, - } - values.update(**kwargs) + series = create_series() if 'series' not in kwargs else kwargs['series'] + patch = create_patch() if 'patch' not in kwargs else kwargs['patch'] + + series.add_patch(patch, num) + + class SeriesPatch(object): + """Simple wrapper to avoid needing to update all tests at once.""" + def __init__(self, series, patch): + self.series = series + self.patch = patch - return SeriesPatch.objects.create(**values) + return SeriesPatch(series=series, patch=patch) def create_series_reference(**kwargs): diff --git a/patchwork/views/cover.py b/patchwork/views/cover.py index cb8fd3c..08b633a 100644 --- a/patchwork/views/cover.py +++ b/patchwork/views/cover.py @@ -36,7 +36,6 @@ def cover_detail(request, cover_id): comments = comments.only('submitter', 'date', 'id', 'content', 'submission') context['comments'] = comments - context['all_series'] = cover.series.all().order_by('-date') return render(request, 'patchwork/submission.html', context) diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py index 862dc83..277b281 100644 --- a/patchwork/views/patch.py +++ b/patchwork/views/patch.py @@ -105,7 +105,6 @@ def patch_detail(request, patch_id): 'submission') context['comments'] = comments - context['all_series'] = patch.series.all().order_by('-date') context['checks'] = patch.check_set.all().select_related('user') context['submission'] = patch context['patchform'] = form @@ -132,7 +131,7 @@ def patch_mbox(request, patch_id): response = HttpResponse(content_type='text/plain') if series_id: - if not patch.series.count(): + if not patch.series: raise Http404('Patch does not have an associated series. This is ' 'because the patch was processed with an older ' 'version of Patchwork. It is not possible to ' diff --git a/patchwork/views/utils.py b/patchwork/views/utils.py index a2be2c8..3c5d298 100644 --- a/patchwork/views/utils.py +++ b/patchwork/views/utils.py @@ -18,7 +18,6 @@ from django.utils import six from patchwork.models import Comment from patchwork.models import Patch -from patchwork.models import Series if settings.ENABLE_REST_API: from rest_framework.authtoken.models import Token @@ -128,26 +127,22 @@ def series_patch_to_mbox(patch, series_id): Returns: A string for the mbox file. """ - if series_id == '*': - series = patch.series.order_by('-date').first() - else: + if series_id != '*': try: series_id = int(series_id) except ValueError: raise Http404('Expected integer series value or *. Received: %r' % series_id) - try: - series = patch.series.get(id=series_id) - except Series.DoesNotExist: + if patch.series.id != series_id: raise Http404('Patch does not belong to series %d' % series_id) mbox = [] # get the series-ified patch - number = series.seriespatch_set.get(patch=patch).number - for dep in series.seriespatch_set.filter(number__lt=number): - mbox.append(patch_to_mbox(dep.patch)) + for dep in patch.series.patches.filter( + number__lt=patch.number).order_by('number'): + mbox.append(patch_to_mbox(dep)) mbox.append(patch_to_mbox(patch)) @@ -165,7 +160,7 @@ def series_to_mbox(series): """ mbox = [] - for dep in series.seriespatch_set.all(): + for dep in series.patches.all().order_by('number'): mbox.append(patch_to_mbox(dep.patch)) return '\n'.join(mbox) |