[Patchew-devel] [PATCH v2 10/10] add Review model
Paolo Bonzini
pbonzini at redhat.com
Fri Sep 28 14:45:37 UTC 2018
This model has a simple many-to-many relation with series, that lets
logged-in users mark series as accepted or rejected, and use the status
in a query.
Three new search operators are added: ack (with synonyms accept and
accepted), nack (with synonyms reject and rejected), review (with
synonyms reviewed).
Sample useful queries include the following:
- "-is:merged ack:me" (what should I merge)
- "to:pbonzini at redhat.com -is:merged -reviewed:me" (what should I review)
- "nvme -is:merged -reviewed:fam" (what did Fam overlook :))
Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
---
api/migrations/0034_auto_20180822_1106.py | 32 ++++++++++
api/models.py | 7 +++
api/search.py | 33 +++++++++-
mods/maintainer.py | 75 +++++++++++++++++++++--
static/css/base.css | 2 +-
5 files changed, 143 insertions(+), 6 deletions(-)
create mode 100644 api/migrations/0034_auto_20180822_1106.py
diff --git a/api/migrations/0034_auto_20180822_1106.py b/api/migrations/0034_auto_20180822_1106.py
new file mode 100644
index 0000000..d830d47
--- /dev/null
+++ b/api/migrations/0034_auto_20180822_1106.py
@@ -0,0 +1,32 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.13 on 2018-08-22 11:06
+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', '0033_auto_20180803_0809'),
+ ]
+
+ operations = [
+ migrations.CreateModel(
+ name='Review',
+ fields=[
+ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+ ('accept', models.BooleanField()),
+ ('message', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='api.Message')),
+ ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
+ ],
+ ),
+ migrations.AddField(
+ model_name='message',
+ name='reviews',
+ field=models.ManyToManyField(blank=True, through='api.Review', to=settings.AUTH_USER_MODEL),
+ ),
+ ]
diff --git a/api/models.py b/api/models.py
index 32e8b41..e4737cb 100644
--- a/api/models.py
+++ b/api/models.py
@@ -423,6 +423,11 @@ class MessageManager(models.Manager):
def HeaderFieldModel(**args):
return models.CharField(max_length=4096, **args)
+class Review(models.Model):
+ user = models.ForeignKey(User)
+ message = models.ForeignKey('Message')
+ accept = models.BooleanField()
+
class Message(models.Model):
""" Patch email message """
@@ -459,6 +464,8 @@ 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)
+
objects = MessageManager()
def save_mbox(self, mbox_blob):
diff --git a/api/search.py b/api/search.py
index cf3c145..a218789 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
+from .models import Message, MessageProperty, MessageResult, Result, Review
from functools import reduce
from django.db.models import Q
@@ -122,6 +122,18 @@ Example:
---
+### Search by review state
+
+Syntax:
+
+ - accept:USERNAME or ack:USERNAME - the series was marked as accepted by the user
+ - reject:USERNAME or nack:USERNAME - the series was marked as reject by the user
+ - review:USERNAME - the series was marked as accepted or rejected by the user
+
+USERNAME can be "me" to identify the current user
+
+---
+
### Reverse condition
- Syntax: !TERM
@@ -219,6 +231,16 @@ Search text keyword in the email message. Example:
return Q(is_merged=True)
return None
+ def _make_filter_review(self, username, user, **kwargs):
+ if username == "me":
+ if not user.is_authenticated:
+ # Django hack to return an always false Q object
+ return Q(pk=None)
+ q = Q(user=user, **kwargs)
+ else:
+ q = Q(user__username=username, **kwargs)
+ return self._make_filter_subquery(Review, q)
+
def _make_filter(self, term, user):
if term.startswith("age:"):
cond = term[term.find(":") + 1:]
@@ -260,6 +282,15 @@ Search text keyword in the email message. Example:
return self._make_filter_result(term[8:], status=Result.PENDING)
elif term.startswith("running:"):
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)
+ 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)
+ elif term.startswith("review:") or term.startswith("reviewed:"):
+ username = term[term.find(":") + 1:]
+ return self._make_filter_review(username, user)
elif term.startswith("project:"):
cond = term[term.find(":") + 1:]
self._projects.add(cond)
diff --git a/mods/maintainer.py b/mods/maintainer.py
index e25d8e2..d95ec4c 100644
--- a/mods/maintainer.py
+++ b/mods/maintainer.py
@@ -12,13 +12,31 @@ 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
+from api.models import Message, Review
class MaintainerModule(PatchewModule):
""" Project maintainer related tasks """
name = "maintainer"
+ def _update_review_state(self, request, message_id, accept):
+ if not request.user.is_authenticated:
+ return HttpResponseForbidden()
+ msg = Message.objects.find_series(message_id)
+ r = Review.objects.filter(user=request.user, message=msg)
+ r = r[0] if r else Review(user=request.user, message=msg)
+ r.accept = accept
+ r.save()
+ return HttpResponseRedirect(request.META.get('HTTP_REFERER'))
+
+ def _delete_review(self, request, message_id):
+ if not request.user.is_authenticated:
+ return HttpResponseForbidden()
+ msg = Message.objects.find_series(message_id)
+ r = Review.objects.filter(user=request.user, message=msg)
+ r.delete()
+ return HttpResponseRedirect(request.META.get('HTTP_REFERER'))
+
def _update_merge_state(self, request, message_id, is_merged):
s = Message.objects.find_series(message_id)
if not s:
@@ -35,6 +53,15 @@ class MaintainerModule(PatchewModule):
def www_view_clear_merged(self, request, message_id):
return self._update_merge_state(request, message_id, False)
+ def www_view_mark_accepted(self, request, message_id):
+ return self._update_review_state(request, message_id, True)
+
+ def www_view_mark_rejected(self, request, message_id):
+ return self._update_review_state(request, message_id, False)
+
+ def www_view_clear_reviewed(self, request, message_id):
+ return self._delete_review(request, message_id)
+
def www_url_hook(self, urlpatterns):
urlpatterns.append(url(r"^mark-as-merged/(?P<message_id>.*)/",
self.www_view_mark_as_merged,
@@ -42,18 +69,58 @@ class MaintainerModule(PatchewModule):
urlpatterns.append(url(r"^clear-merged/(?P<message_id>.*)/",
self.www_view_clear_merged,
name="clear-merged"))
+ urlpatterns.append(url(r"^mark-as-accepted/(?P<message_id>.*)/",
+ self.www_view_mark_accepted,
+ name="mark-as-accepted"))
+ urlpatterns.append(url(r"^mark-as-rejected/(?P<message_id>.*)/",
+ self.www_view_mark_rejected,
+ name="mark-as-rejected"))
+ urlpatterns.append(url(r"^clear-reviewed/(?P<message_id>.*)/",
+ self.www_view_clear_reviewed,
+ name="clear-reviewed"))
def prepare_message_hook(self, request, message, detailed):
- if not detailed:
+ if not detailed or not request.user.is_authenticated:
return
- if message.is_series_head and request.user.is_authenticated:
+ if message.is_series_head:
if message.is_merged:
message.extra_ops.append({"url": reverse("clear-merged",
kwargs={"message_id": message.message_id}),
- "icon": "times",
+ "icon": "eraser",
"title": "Clear merged state"})
else:
message.extra_ops.append({"url": reverse("mark-as-merged",
kwargs={"message_id": message.message_id}),
"icon": "check",
"title": "Mark series as merged"})
+
+ r = Review.objects.filter(user=request.user, message=message).first()
+ if r and r.accept:
+ message.extra_status.append({
+ "icon": "fa-check",
+ "html": 'The series is marked for merging'
+ })
+ else:
+ if message.is_series_head:
+ message.extra_ops.append({"url": reverse("mark-as-accepted",
+ kwargs={"message_id": message.message_id}),
+ "icon": "check",
+ "title": "Mark series as accepted"})
+
+ if r and not r.accept:
+ message.extra_status.append({
+ "icon": "fa-times",
+ "html": 'The series is marked as rejected'
+ })
+ else:
+ if message.is_series_head:
+ message.extra_ops.append({"url": reverse("mark-as-rejected",
+ kwargs={"message_id": message.message_id}),
+ "icon": "times",
+ "title": "Mark series as rejected"})
+
+ if r:
+ message.extra_ops.append({"url": reverse("clear-reviewed",
+ kwargs={"message_id": message.message_id}),
+ "icon": "eraser",
+ "title": "Clear review state"})
diff --git a/static/css/base.css b/static/css/base.css
index 7fe45e4..9e04f50 100644
--- a/static/css/base.css
+++ b/static/css/base.css
@@ -137,7 +137,7 @@ h1, h2, h3, .h1, .h2, .h3 {
.status-content > .fa {
color: #337ab7;
}
-.status-content > .fa-warning {
+.status-content > .fa-warning, .status-content > .fa-times {
color: #CC0000;
}
.status-content > .fa-check {
--
2.17.1
More information about the Patchew-devel
mailing list