patch-detail: add label and button for comment addressed status

Add new label to patch and cover comments to show the status of whether
they are addressed or not and add an adjacent button to allow users to
change the status of the comment. Only users that can edit the patch
(i.e. patch author, delegate, project maintainers) as well as comment
authors can change the status of a patch comment. For cover comments,
there are no delegates, so only maintainers and cover/cover comment
authors can edit the status of the cover comment. Before [1] and after
[2] images for reference.

Use new comment detail REST API endpoint to update the addressed field
value when "Addressed" or "Unaddressed" buttons are clicked. After a
successful request is made, the appearance of the comment status label
and buttons are toggled appropriately. For unsuccessful requests (e.g.
network errors prevents reaching the server), the error message is
populated to the page. A future improvement on this behavior is to add
a spinner to the button to provide a feedback that the request is in a
pending state until it's handled.

[1] https://imgur.com/3ZKzgjN
[2] https://imgur.com/hWZrrnM

Signed-off-by: Raxel Gutierrez <raxel@google.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>

diff --git a/htdocs/css/style.css b/htdocs/css/style.css
index a2a2e3c362a4bf0c5131a4eb6c1aef7f65e535d7..9156aa6ee07368cf7707948873ac6100e5b32f9f 100644 (file)
--- a/htdocs/css/style.css
+++ b/htdocs/css/style.css
@@ -1,4 +1,5 @@
 :root {
+    --light-color:rgb(247, 247, 247);
     --success-color:rgb(92, 184, 92);
     --warning-color:rgb(240, 173, 78);
     --danger-color:rgb(217, 83, 79);
@@ -27,6 +28,10 @@ pre {
     top: 17em;
 }
 
+.hidden {
+    visibility: hidden;
+}
+
 /* Bootstrap overrides */
 
 .navbar-inverse .navbar-brand > a {
@@ -315,6 +320,39 @@ table.patch-meta tr th, table.patch-meta tr td {
     font-family: "DejaVu Sans Mono", fixed;
 }
 
+div[class^="comment-status-bar-"] {
+    display: flex;
+    flex-wrap: wrap;
+    align-items: center;
+}
+
+.comment-status-label {
+    margin: 0px 8px;
+}
+
+button[class^=comment-action] {
+    background-color: var(--light-color);
+    border-radius: 4px;
+}
+
+.comment-action-addressed {
+    border-color: var(--success-color);
+}
+
+.comment-action-unaddressed {
+    border-color: var(--warning-color);
+}
+
+.comment-action-addressed:hover {
+    background-color: var(--success-color);
+    color: var(--light-color);
+}
+
+.comment-action-unaddressed:hover {
+    background-color: var(--warning-color);
+    color: var(--light-color);
+}
+
 .quote {
     color: #007f00;
 }

diff --git a/htdocs/js/submission.js b/htdocs/js/submission.js
index 9676f3489b14c862e4c26c5a19bb47c4c155379e..47cffc8279b344eecbdc391bc5676053ed02ca62 100644 (file)
--- a/htdocs/js/submission.js
+++ b/htdocs/js/submission.js
@@ -1,4 +1,7 @@
+import { updateProperty } from "./rest.js";
+
 $( document ).ready(function() {
+    const patchMeta = document.getElementById("patch-meta");
     function toggleDiv(link_id, headers_id, label_show, label_hide) {
         const link = document.getElementById(link_id)
         const headers = document.getElementById(headers_id)
@@ -14,6 +17,23 @@ $( document ).ready(function() {
         }
     }
 
+    $("button[class^='comment-action']").click((event) => {
+        const submissionType = patchMeta.dataset.submissionType;
+        const submissionId = patchMeta.dataset.submissionId;
+        const commentId = event.target.parentElement.dataset.commentId;
+        const url = `/api/${submissionType}/${submissionId}/comments/${commentId}/`;
+        const data = {'addressed': event.target.value} ;
+        const updateMessage = {
+            'error': "No comments updated",
+            'success': "1 comment(s) updated",
+        };
+        updateProperty(url, data, updateMessage).then(isSuccess => {
+            if (isSuccess) {
+                $("div[class^='comment-status-bar-'][data-comment-id='"+commentId+"']").toggleClass("hidden");
+            }
+        })
+    });
+
     // Click listener to show/hide headers
     document.getElementById("toggle-patch-headers").addEventListener("click", function() {
         toggleDiv("toggle-patch-headers", "patch-headers");

diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
index 36b15d0e8f09dcf0c083bc0dd28f7776f2a1d9b1..2238e82e7f5439e5dd10822414dd3f6822f8026d 100644 (file)
--- a/patchwork/templates/patchwork/submission.html
+++ b/patchwork/templates/patchwork/submission.html
@@ -5,6 +5,7 @@
 {% load person %}
 {% load patch %}
 {% load static %}
+{% load utils %}
 
 {% block headers %}
   <script type="module" src="{% static "js/submission.js" %}"></script>
@@ -19,7 +20,12 @@
   <h1>{{ submission.name }}</h1>
 </div>
 
-<table class="patch-meta">
+<table
+  id="patch-meta"
+  class="patch-meta"
+  data-submission-type={{submission|verbose_name_plural|lower}}
+  data-submission-id={{submission.id}}
+>
  <tr>
   <th>Message ID</th>
   {% if submission.list_archive_url %}
@@ -271,18 +277,50 @@
 {% if forloop.first %}
 <h2>Comments</h2>
 {% endif %}
-
+{% is_editable item user as comment_is_editable %}
 <a name="{{ item.id }}"></a>
 <div class="submission-message">
-<div class="meta">
- <span>{{ item.submitter|personify:project }}</span>
- <span class="message-date">{{ item.date }} UTC |
-   <a href="{% url 'comment-redirect' comment_id=item.id %}">#{{ forloop.counter }}</a>
-  </span>
-</div>
-<pre class="content">
-{{ item|commentsyntax }}
-</pre>
+  <div class="meta">
+    {{ item.submitter|personify:project }}
+    <span class="message-date">{{ item.date }} UTC |
+      <a href="{% url 'comment-redirect' comment_id=item.id %}">#{{ forloop.counter }}</a>
+    </span>
+    {% if item.addressed %}
+      <div class="comment-status-bar-addressed" data-comment-id={{item.id}}>
+    {% else %}
+      <div class="comment-status-bar-addressed hidden" data-comment-id={{item.id}}>
+    {% endif %}
+        <div class="comment-status-label text-success mx-3">
+          <span class="glyphicon glyphicon-ok-circle" aria-hidden="true"></span>
+          Addressed
+        </div>
+        {% 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 Unaddressed
+          </button>
+        {% endif %}
+      </div>
+    {% if item.addressed %}
+      <div class="comment-status-bar-unaddressed hidden" data-comment-id={{item.id}}>
+    {% else %}
+      <div class="comment-status-bar-unaddressed" 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>
+          Unaddressed
+        </div>
+        {% if editable or comment_is_editable %}
+          <button class="comment-action-addressed text-success" value="true">
+            <span class="glyphicon glyphicon-ok-circle" aria-hidden="true"></span>
+            Mark Addressed
+          </button>
+        {% endif %}
+      </div>
+  </div>
+  <pre class="content">
+  {{ item|commentsyntax }}
+  </pre>
 </div>
 {% endfor %}
 

diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
index 3e6874ae1e5d31d6dff40d2a34634332827172ee..00b0147ff57c19472332e2cc9b6d0998f455b835 100644 (file)
--- a/patchwork/views/patch.py
+++ b/patchwork/views/patch.py
@@ -109,7 +109,8 @@ def patch_detail(request, project_id, msgid):
 
     comments = patch.comments.all()
     comments = comments.select_related('submitter')
-    comments = comments.only('submitter', 'date', 'id', 'content', 'patch')
+    comments = comments.only('submitter', 'date', 'id', 'content', 'patch',
+                             'addressed')
 
     if patch.related:
         related_same_project = patch.related.patches.only(
@@ -128,6 +129,7 @@ def patch_detail(request, project_id, msgid):
         patch.check_set.all().select_related('user'),
     )
     context['submission'] = patch
+    context['editable'] = editable
     context['patchform'] = form
     context['createbundleform'] = createbundleform
     context['project'] = patch.project