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@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@redhat.com>
---
api/migrations/0034_auto_20180822_1106.py | 32 ++++++++++
api/models.py | 7 +++
api/search.py | 28 +++++++++
mods/maintainer.py | 72 +++++++++++++++++++++--
static/css/base.css | 2 +-
5 files changed, 136 insertions(+), 5 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 eea924f..545013e 100644
--- a/api/models.py
+++ b/api/models.py
@@ -419,6 +419,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 """
@@ -455,6 +460,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 aa77f22..bcac3ac 100644
--- a/api/search.py
+++ b/api/search.py
@@ -104,6 +104,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 rejected 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
@@ -202,6 +214,12 @@ Search text keyword in the email message. Example:
def _make_filter_result(self, term, **kwargs):
return Q(results__name=term, **kwargs) | Q(results__name__startswith=term+'.', **kwargs)
+ def _make_filter_review(self, username, user, **kwargs):
+ if username == "me":
+ return Q(review__user=user, **kwargs)
+ else:
+ return Q(review__user__username=username, **kwargs)
+
def _make_filter(self, term, user):
if term.startswith("age:"):
cond = term[term.find(":") + 1:]
@@ -240,6 +258,16 @@ Search text keyword in the email message. Example:
return self._make_filter_result(term[8:], results__status=Result.PENDING)
elif term.startswith("running:"):
return self._make_filter_result(term[8:], results__status=Result.RUNNING)
+ return self._make_filter_result(cond[8:], results__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, review__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, review__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..7cb23d2 100644
--- a/mods/maintainer.py
+++ b/mods/maintainer.py
@@ -12,13 +12,27 @@ 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):
+ 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):
+ 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 +49,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 +65,59 @@ 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)
+ if not r or not r[0].accept:
+ 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"})
+ else:
+ message.extra_status.append({
+ "icon": "fa-check",
+ "html": 'The series is marked for merging'
+ })
+
+ if not r or r[0].accept:
+ 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"})
+ else:
+ message.extra_status.append({
+ "icon": "fa-times",
+ "html": 'The series is marked as rejected'
+ })
+
+ print(r)
+ 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
_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
On Thu, 08/23 22:00, Paolo Bonzini wrote:
> 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@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@redhat.com>
> ---
> api/migrations/0034_auto_20180822_1106.py | 32 ++++++++++
> api/models.py | 7 +++
> api/search.py | 28 +++++++++
> mods/maintainer.py | 72 +++++++++++++++++++++--
> static/css/base.css | 2 +-
> 5 files changed, 136 insertions(+), 5 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 eea924f..545013e 100644
> --- a/api/models.py
> +++ b/api/models.py
> @@ -419,6 +419,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 """
>
> @@ -455,6 +460,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 aa77f22..bcac3ac 100644
> --- a/api/search.py
> +++ b/api/search.py
> @@ -104,6 +104,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 rejected 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
> @@ -202,6 +214,12 @@ Search text keyword in the email message. Example:
> def _make_filter_result(self, term, **kwargs):
> return Q(results__name=term, **kwargs) | Q(results__name__startswith=term+'.', **kwargs)
>
> + def _make_filter_review(self, username, user, **kwargs):
> + if username == "me":
> + return Q(review__user=user, **kwargs)
> + else:
> + return Q(review__user__username=username, **kwargs)
> +
> def _make_filter(self, term, user):
> if term.startswith("age:"):
> cond = term[term.find(":") + 1:]
> @@ -240,6 +258,16 @@ Search text keyword in the email message. Example:
> return self._make_filter_result(term[8:], results__status=Result.PENDING)
> elif term.startswith("running:"):
> return self._make_filter_result(term[8:], results__status=Result.RUNNING)
> + return self._make_filter_result(cond[8:], results__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, review__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, review__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..7cb23d2 100644
> --- a/mods/maintainer.py
> +++ b/mods/maintainer.py
> @@ -12,13 +12,27 @@ 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):
> + msg = Message.objects.find_series(message_id)
> + r = Review.objects.filter(user=request.user, message=msg)
Review.objects.get_or_create().
> + 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):
> + 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 +49,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)
Don't these handlers want to check request.user.is_authenticated?
> +
> def www_url_hook(self, urlpatterns):
> urlpatterns.append(url(r"^mark-as-merged/(?P<message_id>.*)/",
> self.www_view_mark_as_merged,
> @@ -42,18 +65,59 @@ 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)
r = r.first() to simplify the if conditions a bit?
> + if not r or not r[0].accept:
> + 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"})
> + else:
> + message.extra_status.append({
> + "icon": "fa-check",
> + "html": 'The series is marked for merging'
> + })
> +
> + if not r or r[0].accept:
> + 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"})
> + else:
> + message.extra_status.append({
> + "icon": "fa-times",
> + "html": 'The series is marked as rejected'
> + })
> +
> + print(r)
Please remember to drop this before pushing. :)
> + 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
>
> _______________________________________________
> Patchew-devel mailing list
> Patchew-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/patchew-devel
Fam
_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
© 2016 - 2025 Red Hat, Inc.