From patchwork Thu Aug 26 17:18:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 1521226 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ozlabs.org (client-ip=2404:9400:2:0:216:3eff:fee1:b9f1; helo=lists.ozlabs.org; envelope-from=patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="key not found in DNS" header.d=that.guru header.i=@that.guru header.a=rsa-sha256 header.s=x header.b=rCX8j6eV; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2404:9400:2:0:216:3eff:fee1:b9f1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4GwV1q3GQLz9sWX for ; Fri, 27 Aug 2021 03:18:55 +1000 (AEST) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4GwV1p60bLz2yfb for ; Fri, 27 Aug 2021 03:18:54 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" header.d=that.guru header.i=@that.guru header.a=rsa-sha256 header.s=x header.b=rCX8j6eV; dkim-atps=neutral X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=that.guru (client-ip=136.175.108.254; helo=mail-108-mta254.mxroute.com; envelope-from=stephen@that.guru; receiver=) Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" header.d=that.guru header.i=@that.guru header.a=rsa-sha256 header.s=x header.b=rCX8j6eV; dkim-atps=neutral Received: from mail-108-mta254.mxroute.com (mail-108-mta254.mxroute.com [136.175.108.254]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4GwV1g3S1Jz2yHP for ; Fri, 27 Aug 2021 03:18:46 +1000 (AEST) Received: from filter004.mxroute.com ([149.28.56.236] filter004.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta254.mxroute.com (ZoneMTA) with ESMTPSA id 17b837889e000074ba.003 for (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256); Thu, 26 Aug 2021 17:18:40 +0000 X-Zone-Loop: 51d7c58d17a31c2e4937ffdcbddae33c35aa6b0e3fb9 X-Originating-IP: [149.28.56.236] DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=x; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=5Vr7vM2u6wzcZPsl77pWW94xqAhBeS2Ftt4W30Gu3lE=; b=rCX8j6eVZn+f9vK/6XEQVW75RP ZM2ACmUCepYmC1WE593MiQkQeZzMkrAyaI3IGpIYsIwdE/pug+dcY9QouJOgYN+nE9zQ7FtDSkk3G tkGieLkjgjEmiS+X4VEiP6GyJMHkgw9TK71pi+219SrHJpjp4BZQ+uWjIQdGiLonbpsrL4HPrBqSg i9nPsyA+W9rYRUe3hf4WmsG+VJLVCPD9Stt8CxwjxthXown6vzdN2eyPKPQfS0ZfHNw4AO6IwOPCh iqie91E9RrWBY0h3cGsn2wA88P4B/yo4K6eak6+Nk/43P9LLM4VYZiwluWlLImP0VBc2NZ1hdObyO 5v0Imd9Q==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH 1/4] Make addressed/unaddressed workflow opt-in Date: Thu, 26 Aug 2021 18:18:28 +0100 Message-Id: <20210826171831.547578-2-stephen@that.guru> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210826171831.547578-1-stephen@that.guru> References: <20210826171831.547578-1-stephen@that.guru> MIME-Version: 1.0 X-AuthUser: stephen@that.guru X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" The current workflow for the address/unaddressed attribute of comments sets all comments to unaddressed by default. This is unintuitive, as it assumes that all comments are actionable items. It also imposes a massive burden on maintainers, who will need to manually sift through every single comment received to a list and manually set the non-actionable items as "addressed". Change this workflow so that the 'addressed' field defaults to NULL. This means maintainers or users must manually set this to False when they're requesting additional feedback. This is currently possible via the web UI or REST API. A future change will make it possible via a custom mail header. Signed-off-by: Stephen Finucane Cc: Raxel Gutierrez Cc: Daniel Axtens --- I think it's essential we make this change in order for this patch to be useful. I also think it's okay to modify the migration in place, since (a) we don't support deployment from master in production and (b) to the best of my knowledge, setting a default, non-NULL value on a new column is an expensive operation on certain databases (MySQL?) while changing a column value for all rows is *definitely* expensive. The template work could possibly do with tweaking. Feel free to advise if so. --- docs/api/schemas/latest/patchwork.yaml | 2 ++ docs/api/schemas/patchwork.j2 | 2 ++ docs/api/schemas/v1.3/patchwork.yaml | 2 ++ htdocs/js/submission.js | 14 +++++++++++-- patchwork/migrations/0045_addressed_fields.py | 4 ++-- patchwork/models.py | 4 ++-- patchwork/templates/patchwork/submission.html | 20 +++++++++++++++---- 7 files changed, 38 insertions(+), 10 deletions(-) diff --git docs/api/schemas/latest/patchwork.yaml docs/api/schemas/latest/patchwork.yaml index e3bff990..2a98c179 100644 --- docs/api/schemas/latest/patchwork.yaml +++ docs/api/schemas/latest/patchwork.yaml @@ -1669,12 +1669,14 @@ components: addressed: title: Addressed type: boolean + nullable: true CommentUpdate: type: object properties: addressed: title: Addressed type: boolean + nullable: true CoverList: type: object properties: diff --git docs/api/schemas/patchwork.j2 docs/api/schemas/patchwork.j2 index 3b4ad2f6..02aa9f72 100644 --- docs/api/schemas/patchwork.j2 +++ docs/api/schemas/patchwork.j2 @@ -1734,12 +1734,14 @@ components: addressed: title: Addressed type: boolean + nullable: true CommentUpdate: type: object properties: addressed: title: Addressed type: boolean + nullable: true {% endif %} CoverList: type: object diff --git docs/api/schemas/v1.3/patchwork.yaml docs/api/schemas/v1.3/patchwork.yaml index 6cbba646..0a9046a5 100644 --- docs/api/schemas/v1.3/patchwork.yaml +++ docs/api/schemas/v1.3/patchwork.yaml @@ -1669,12 +1669,14 @@ components: addressed: title: Addressed type: boolean + nullable: true CommentUpdate: type: object properties: addressed: title: Addressed type: boolean + nullable: true CoverList: type: object properties: diff --git htdocs/js/submission.js htdocs/js/submission.js index 47cffc82..c93c36ec 100644 --- htdocs/js/submission.js +++ htdocs/js/submission.js @@ -29,7 +29,17 @@ $( document ).ready(function() { }; updateProperty(url, data, updateMessage).then(isSuccess => { if (isSuccess) { - $("div[class^='comment-status-bar-'][data-comment-id='"+commentId+"']").toggleClass("hidden"); + // The API won't accept anything but true or false, so we + // always hide the -action-required element + $("div[class='comment-status-bar-action-required'][data-comment-id='"+commentId+"']").addClass("hidden"); + + if (event.target.value === "true") { + $("div[class^='comment-status-bar-addressed'][data-comment-id='"+commentId+"']").removeClass("hidden"); + $("div[class^='comment-status-bar-unaddressed'][data-comment-id='"+commentId+"']").addClass("hidden"); + } else if (event.target.value === "false") { + $("div[class^='comment-status-bar-addressed'][data-comment-id='"+commentId+"']").addClass("hidden"); + $("div[class^='comment-status-bar-unaddressed'][data-comment-id='"+commentId+"']").removeClass("hidden"); + } } }) }); @@ -59,4 +69,4 @@ $( document ).ready(function() { toggleDiv("toggle-related-outside", "related-outside", "show from other projects"); }); } -}); \ No newline at end of file +}); diff --git patchwork/migrations/0045_addressed_fields.py patchwork/migrations/0045_addressed_fields.py index ed3527bc..22887c33 100644 --- patchwork/migrations/0045_addressed_fields.py +++ patchwork/migrations/0045_addressed_fields.py @@ -13,11 +13,11 @@ class Migration(migrations.Migration): migrations.AddField( model_name='covercomment', name='addressed', - field=models.BooleanField(default=False), + field=models.BooleanField(null=True), ), migrations.AddField( model_name='patchcomment', name='addressed', - field=models.BooleanField(default=False), + field=models.BooleanField(null=True), ), ] diff --git patchwork/models.py patchwork/models.py index 58e4c51e..6304b34d 100644 --- patchwork/models.py +++ patchwork/models.py @@ -657,7 +657,7 @@ class CoverComment(EmailMixin, models.Model): related_query_name='comment', on_delete=models.CASCADE, ) - addressed = models.BooleanField(default=False) + addressed = models.BooleanField(null=True) @property def list_archive_url(self): @@ -708,7 +708,7 @@ class PatchComment(EmailMixin, models.Model): related_query_name='comment', on_delete=models.CASCADE, ) - addressed = models.BooleanField(default=False) + addressed = models.BooleanField(null=True) @property def list_archive_url(self): diff --git patchwork/templates/patchwork/submission.html patchwork/templates/patchwork/submission.html index 2238e82e..2814f3d5 100644 --- patchwork/templates/patchwork/submission.html +++ patchwork/templates/patchwork/submission.html @@ -285,7 +285,19 @@ {{ item.date }} UTC | #{{ forloop.counter }} - {% if item.addressed %} + {% if item.addressed == None %} +
+ {% else %} + + {% if item.addressed == True %}
{% else %} - {% if item.addressed %} -