Make addressed/unaddressed workflow opt-in

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 <stephen@that.guru>
Cc: Raxel Gutierrez <raxel@google.com>
Cc: Daniel Axtens <dja@axtens.net>

diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml
index e3bff9907bea857cacdcce387777985ecc013eb9..2a98c179e89e2ff5ae5da738fa1e7afb6aacae0a 100644 (file)
--- a/docs/api/schemas/latest/patchwork.yaml
+++ b/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 a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2
index 3b4ad2f615c7b5e07f9a2e48be19ec54d8bd0d82..02aa9f72ca80e71b6ce94d45f848a0bb9a71597f 100644 (file)
--- a/docs/api/schemas/patchwork.j2
+++ b/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 a/docs/api/schemas/v1.3/patchwork.yaml b/docs/api/schemas/v1.3/patchwork.yaml
index 6cbba646709104bbfe9bdd1e5a6c94cd434bc65c..0a9046a51dc5f39755d6a8fe59c3d5b846ee140f 100644 (file)
--- a/docs/api/schemas/v1.3/patchwork.yaml
+++ b/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 a/htdocs/js/submission.js b/htdocs/js/submission.js
index 47cffc8279b344eecbdc391bc5676053ed02ca62..c93c36ec80bc8dd34202bafc77d1a54bc3bd35de 100644 (file)
--- a/htdocs/js/submission.js
+++ b/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 a/patchwork/migrations/0045_addressed_fields.py b/patchwork/migrations/0045_addressed_fields.py
index ed3527bc178b60795646cc82026d40a1ed6b3bac..22887c337ccfbc848970af8ee5062b72adf61d75 100644 (file)
--- a/patchwork/migrations/0045_addressed_fields.py
+++ b/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 a/patchwork/models.py b/patchwork/models.py
index 58e4c51e971607b5f1ce30c0e9ff210fa1267dd0..6304b34db0a3eb2f234249bd9a3a553ab4ecfc0b 100644 (file)
--- a/patchwork/models.py
+++ b/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 a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
index 2238e82e7f5439e5dd10822414dd3f6822f8026d..2814f3d5426a1b74c6f75cf84d7ac710571950e2 100644 (file)
--- a/patchwork/templates/patchwork/submission.html
+++ b/patchwork/templates/patchwork/submission.html
@@ -285,7 +285,19 @@
     <span class="message-date">{{ item.date }} UTC |
       <a href="{% url 'comment-redirect' comment_id=item.id %}">#{{ forloop.counter }}</a>
     </span>
-    {% if item.addressed %}
+    {% if item.addressed == None %}
+      <div class="comment-status-bar-action-required" data-comment-id={{item.id}}>
+    {% else %}
+      <div class="comment-status-bar-action-required hidden" data-comment-id={{item.id}}>
+    {% endif %}
+        {% if editable or comment_is_editable %}
+          <button class="comment-action-unaddressed text-warning" value="false">
+            <span class="glyphicon glyphicon-warning-sign" aria-hidden="true"></span>
+            Mark Action Required
+          </button>
+        {% endif %}
+      </div>
+    {% if item.addressed == True %}
       <div class="comment-status-bar-addressed" data-comment-id={{item.id}}>
     {% else %}
       <div class="comment-status-bar-addressed hidden" data-comment-id={{item.id}}>
@@ -301,10 +313,10 @@
           </button>
         {% endif %}
       </div>
-    {% if item.addressed %}
-      <div class="comment-status-bar-unaddressed hidden" data-comment-id={{item.id}}>
-    {% else %}
+    {% if item.addressed == False %}
       <div class="comment-status-bar-unaddressed" data-comment-id={{item.id}}>
+    {% else %}
+      <div class="comment-status-bar-unaddressed hidden" data-comment-id={{item.id}}>
     {% endif %}
         <div class="comment-status-label text-warning mx-3">
           <span class="glyphicon glyphicon-warning-sign" aria-hidden="true"></span>