[Patchew-devel] [PATCH v2 03/12] Generalize Review model to Queue
Paolo Bonzini
pbonzini at redhat.com
Wed Nov 28 17:16:43 UTC 2018
On 28/11/18 17:57, Paolo Bonzini wrote:
> On 28/11/18 15:34, Fam Zheng wrote:
>> From: Fam Zheng <famz at redhat.com>
>>
>> The concept of a queue is a per-user model for maintainers. A maintainer
>> can have one ore more queues. The current Review model already has
>> accept and reject stautus, so we generalize that to support a more
>> flexible queue feature.
>>
>> Previously, "mark as accepted" adds a (user, message, accept=True)
>> record in the Review model. Now it adds a (user, message, name='accept')
>> record in the Queue model.
>>
>> Similary, for 'mark as rejected', a (user, message, name='reject')
>> record is added in the Queue model.
>>
>> This makes the two queue names specially purposed on the web interface
>> and search terms, which we'll extend to also support normal queues.
>
> Can you rename this to QueuedSeries? Apart from this nit, I'm totally
> okay with the series!
Meaning - I still am not sure about the presentation of /my-queues, but
I understood why you did that way (I do believe that, given the current
functionality, a per-series view is more useful, on the other hand most
of the time ack:me/nack:me/maint:me would be enough so your per-message
view can be useful too). Probably having _both_ message-list and
series-list views would be the way to go. In any case it doesn't have
to block the functionality and I'd like this to be included in the next
deployment to patchew.org; we can look later at improving it later and
the basic database concepts are already fine like this.
Maybe we'll add a first-class Queue model, in case we need something
more than (user, name); maybe we'll augment Patchew with per-message
tweaking and need a QueuedMessage model[1]. Either way it should be
easy to write a Django migration to populate them.
Paolo
[1] Another idea could be to have a QueuedMessageAction concept (e.g.
modify commit message, drop patch, squash another message id, etc.).
> Paolo
>
>> Signed-off-by: Fam Zheng <famz at redhat.com>
>> ---
>> api/migrations/0042_review_to_queue.py | 19 +++++++++++
>> api/migrations/0043_auto_20181120_0636.py | 40 +++++++++++++++++++++++
>> api/models.py | 12 +++++--
>> api/search.py | 10 +++---
>> mods/maintainer.py | 23 +++++++------
>> 5 files changed, 86 insertions(+), 18 deletions(-)
>> create mode 100644 api/migrations/0042_review_to_queue.py
>> create mode 100644 api/migrations/0043_auto_20181120_0636.py
>>
>> diff --git a/api/migrations/0042_review_to_queue.py b/api/migrations/0042_review_to_queue.py
>> new file mode 100644
>> index 0000000..3101349
>> --- /dev/null
>> +++ b/api/migrations/0042_review_to_queue.py
>> @@ -0,0 +1,19 @@
>> +# -*- coding: utf-8 -*-
>> +# Generated by Django 1.11.16 on 2018-11-20 06:33
>> +from __future__ import unicode_literals
>> +
>> +from django.conf import settings
>> +from django.db import migrations, models
>> +import django.db.models.deletion
>> +
>> +
>> +class Migration(migrations.Migration):
>> +
>> + dependencies = [
>> + migrations.swappable_dependency(settings.AUTH_USER_MODEL),
>> + ('api', '0041_postgres_fts'),
>> + ]
>> +
>> + operations = [
>> + migrations.RenameModel('Review', 'Queue'),
>> + ]
>> diff --git a/api/migrations/0043_auto_20181120_0636.py b/api/migrations/0043_auto_20181120_0636.py
>> new file mode 100644
>> index 0000000..4f83f28
>> --- /dev/null
>> +++ b/api/migrations/0043_auto_20181120_0636.py
>> @@ -0,0 +1,40 @@
>> +# -*- coding: utf-8 -*-
>> +# Generated by Django 1.11.16 on 2018-11-20 06:36
>> +from __future__ import unicode_literals
>> +
>> +from django.conf import settings
>> +from django.db import migrations, models
>> +
>> +
>> +class Migration(migrations.Migration):
>> +
>> + dependencies = [
>> + migrations.swappable_dependency(settings.AUTH_USER_MODEL),
>> + ('api', '0042_review_to_queue'),
>> + ]
>> +
>> + operations = [
>> + migrations.RenameField(
>> + model_name='message',
>> + old_name='reviews',
>> + new_name='queues',
>> + ),
>> + migrations.AddField(
>> + model_name='queue',
>> + name='name',
>> + field=models.CharField(default='accept', help_text='Name of the queue', max_length=1024),
>> + preserve_default=False,
>> + ),
>> + migrations.RemoveField(
>> + model_name='queue',
>> + name='accept',
>> + ),
>> + migrations.AlterUniqueTogether(
>> + name='queue',
>> + unique_together=set([('user', 'message', 'name')]),
>> + ),
>> + migrations.AlterIndexTogether(
>> + name='queue',
>> + index_together=set([('user', 'message')]),
>> + ),
>> + ]
>> diff --git a/api/models.py b/api/models.py
>> index e24098b..c2cd2ac 100644
>> --- a/api/models.py
>> +++ b/api/models.py
>> @@ -436,11 +436,17 @@ def HeaderFieldModel(**args):
>> return models.CharField(max_length=4096, **args)
>>
>>
>> -class Review(models.Model):
>> +class Queue(models.Model):
>> user = models.ForeignKey(User)
>> message = models.ForeignKey('Message')
>> - accept = models.BooleanField()
>> + # Special purposed queues:
>> + # accept: When user marked series as "accepted"
>> + # reject: When user marked series as "rejected"
>> + name = models.CharField(max_length=1024, help_text="Name of the queue")
>>
>> + class Meta:
>> + unique_together = ('user', 'message', 'name')
>> + index_together = [('user', 'message')]
>>
>> class Message(models.Model):
>> """ Patch email message """
>> @@ -479,7 +485,7 @@ class Message(models.Model):
>> # number of patches we've got if is_series_head
>> num_patches = models.IntegerField(null=False, default=-1, blank=True)
>>
>> - reviews = models.ManyToManyField(User, blank=True, through=Review)
>> + queues = models.ManyToManyField(User, blank=True, through=Queue)
>>
>> objects = MessageManager()
>>
>> diff --git a/api/search.py b/api/search.py
>> index 659f65d..3067235 100644
>> --- a/api/search.py
>> +++ b/api/search.py
>> @@ -8,7 +8,7 @@
>> # This work is licensed under the MIT License. Please see the LICENSE file or
>> # http://opensource.org/licenses/MIT.
>>
>> -from .models import Message, MessageProperty, MessageResult, Result, Review
>> +from .models import Message, MessageProperty, MessageResult, Result, Queue
>> from functools import reduce
>>
>> from django.db import connection
>> @@ -265,7 +265,7 @@ Search text keyword in the email message. Example:
>> q = Q(user=user, **kwargs)
>> else:
>> q = Q(user__username=username, **kwargs)
>> - return self._make_filter_subquery(Review, q)
>> + return self._make_filter_subquery(Queue, q)
>>
>> def _make_filter(self, term, user):
>> if term.startswith("age:"):
>> @@ -310,13 +310,13 @@ Search text keyword in the email message. Example:
>> return self._make_filter_result(term[8:], status=Result.RUNNING)
>> elif term.startswith("ack:") or term.startswith("accept:") or term.startswith("accepted:"):
>> username = term[term.find(":") + 1:]
>> - return self._make_filter_review(username, user, accept=True)
>> + return self._make_filter_review(username, user, name="accept")
>> elif term.startswith("nack:") or term.startswith("reject:") or term.startswith("rejected:"):
>> username = term[term.find(":") + 1:]
>> - return self._make_filter_review(username, user, accept=False)
>> + return self._make_filter_review(username, user, name="reject")
>> elif term.startswith("review:") or term.startswith("reviewed:"):
>> username = term[term.find(":") + 1:]
>> - return self._make_filter_review(username, user)
>> + return self._make_filter_review(username, user, name="accept")
>> elif term.startswith("project:"):
>> cond = term[term.find(":") + 1:]
>> self._projects.add(cond)
>> diff --git a/mods/maintainer.py b/mods/maintainer.py
>> index a3874de..7f4f5fe 100644
>> --- a/mods/maintainer.py
>> +++ b/mods/maintainer.py
>> @@ -12,7 +12,7 @@ from django.conf.urls import url
>> from django.http import Http404, HttpResponseRedirect
>> from django.urls import reverse
>> from mod import PatchewModule
>> -from api.models import Message, Review
>> +from api.models import Message, Queue
>>
>> class MaintainerModule(PatchewModule):
>> """ Project maintainer related tasks """
>> @@ -25,8 +25,12 @@ class MaintainerModule(PatchewModule):
>> msg = Message.objects.find_series(message_id)
>> if not msg:
>> raise Http404("Series not found")
>> - Review.objects.update_or_create(user=request.user, message=msg,
>> - defaults = { 'accept': accept })
>> + if accept:
>> + to_create, to_delete = 'accept', 'reject'
>> + else:
>> + to_create, to_delete = 'reject', 'accept'
>> + Queue.objects.filter(user=request.user, message=msg, name=to_delete).delete()
>> + Queue.objects.get_or_create(user=request.user, message=msg, name=to_create)
>> return HttpResponseRedirect(request.META.get('HTTP_REFERER'))
>>
>> def _delete_review(self, request, message_id):
>> @@ -35,7 +39,8 @@ class MaintainerModule(PatchewModule):
>> msg = Message.objects.find_series(message_id)
>> if not msg:
>> raise Http404("Series not found")
>> - r = Review.objects.filter(user=request.user, message=msg)
>> + r = Queue.objects.filter(user=request.user, message=msg,
>> + name__in=['accept', 'reject'])
>> r.delete()
>> return HttpResponseRedirect(request.META.get('HTTP_REFERER'))
>>
>> @@ -96,11 +101,9 @@ class MaintainerModule(PatchewModule):
>> "icon": "check",
>> "title": "Mark series as merged"})
>>
>> - try:
>> - r = Review.objects.get(user=request.user, message=message)
>> - except Review.DoesNotExist:
>> - r = None
>> - if r and r.accept:
>> + r = Queue.objects.filter(user=request.user, message=message,
>> + name__in=['accept', 'reject']).first()
>> + if r and r.name == 'accept':
>> message.extra_status.append({
>> "icon": "fa-check",
>> "html": 'The series is marked for merging'
>> @@ -112,7 +115,7 @@ class MaintainerModule(PatchewModule):
>> "icon": "check",
>> "title": "Mark series as accepted"})
>>
>> - if r and not r.accept:
>> + if r and r.name == 'reject':
>> message.extra_status.append({
>> "icon": "fa-times",
>> "html": 'The series is marked as rejected'
>>
>
More information about the Patchew-devel
mailing list