[PATCH v3 09/10] patch-detail: add label and button for comment addressed status
Daniel Axtens
dja at axtens.net
Tue Aug 17 11:19:23 AEST 2021
Raxel Gutierrez <raxel at google.com> writes:
> Add new label to patch 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 comment. Before [1] and after [2] images for
> reference.
>
> Refactor both the message containers in the "Commit Message" and
> "Comments" to have the same styling through the `patch-message` class so
> that the headers are normalized to be left-aligned. Also, pass context
> about whether the patch is `editable` by the user to the patch-detail
> template.
>
> Use new comment detail REST API endpoint to update the addressed field
> value when "Addressed" or "Unaddressed" buttons are clicked. After, the
> request is made, the appearance of the comment status label and buttons
> are toggled appropriately.
>
> [1] https://imgur.com/NDfcPJy
> [2] https://imgur.com/kIhohED
>
> Signed-off-by: Raxel Gutierrez <raxel at google.com>
> ---
> htdocs/css/style.css | 53 +++++++++++++-
> htdocs/js/submission.js | 15 ++++
> patchwork/templates/patchwork/submission.html | 69 ++++++++++++++-----
> patchwork/views/patch.py | 1 +
> 4 files changed, 118 insertions(+), 20 deletions(-)
>
> diff --git a/htdocs/css/style.css b/htdocs/css/style.css
> index 46a91ee..ba83de4 100644
> --- a/htdocs/css/style.css
> +++ b/htdocs/css/style.css
> @@ -1,3 +1,9 @@
> +:root {
> + --light-color: #F7F7F7;
> + --warning-color: #f0ad4e;
> + --success-color: #5cb85c;
> +}
> +
> h2 {
> font-size: 25px;
> margin: 18px 0 18px 0;
> @@ -21,6 +27,10 @@ pre {
> top: 17em;
> }
>
> +.hidden {
> + visibility: hidden;
> +}
> +
> /* Bootstrap overrides */
>
> .navbar-inverse .navbar-brand > a {
> @@ -277,12 +287,18 @@ table.patch-meta tr th, table.patch-meta tr td {
> color: #f7977a;
> }
>
> -.comment .meta {
> +.patch-message .meta {
> + display: flex;
> + align-items: center;
> background: #f0f0f0;
> padding: 0.3em 0.5em;
> }
>
> -.comment .content {
> +.patch-message .message-date {
> + margin-left: 8px;
> +}
> +
> +.patch-message .content {
> border: 0;
> }
>
> @@ -294,6 +310,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 9676f34..27e4a02 100644
> --- a/htdocs/js/submission.js
> +++ b/htdocs/js/submission.js
> @@ -1,3 +1,5 @@
> +import { updateProperty } from "./rest.js";
> +
> $( document ).ready(function() {
> function toggleDiv(link_id, headers_id, label_show, label_hide) {
> const link = document.getElementById(link_id)
> @@ -14,6 +16,19 @@ $( document ).ready(function() {
> }
> }
>
> + $("button[class^='comment-action']").click((event) => {
> + const patchId = document.getElementById("patch").dataset.patchId;
> + const commentId = event.target.parentElement.dataset.commentId;
> + const url = "/api/patches/" + patchId + "/comments/" + commentId + "/";
> + const data = {'addressed': event.target.value} ;
> + const updateMessage = {
> + 'none': "No comments updated",
> + 'some': "1 comment(s) updated",
> + };
> + updateProperty(url, data, updateMessage);
> + $("div[class^='comment-status-bar-'][data-comment-id='"+commentId+"']").toggleClass("hidden");
> + });
> +
If I hit this button repeatedly and quickly, I eventually get a 500
error from the server and the following on the console:
rest.js:22 PATCH http://localhost:8000/api/patches/17/comments/undefined/ 500 (Internal Server Error)
VM283:1 Uncaught (in promise) SyntaxError: Unexpected token < in JSON at position 0
at JSON.parse (<anonymous>)
at rest.js:28
The django log contains the following revealing snippet (and then we hit
the error from the API patch where we try to look up the linkname field
which doesn't exist):
web_1 | "PATCH /api/patches/17/comments/undefined/ HTTP/1.1" 500 169238
web_1 | Internal Server Error: /api/patches/17/comments/undefined/
web_1 | Traceback (most recent call last):
web_1 | File "/home/patchwork/patchwork/patchwork/api/comment.py", line 101, in get_object
web_1 | obj = queryset.get(id=int(comment_id))
web_1 | ValueError: invalid literal for int() with base 10: 'undefined'
Somehow I'm guessing the data field is getting rewritten and we're
losing the comment ID temporarily? I haven't traced through the JS to
figure out how but hopefully you can debug it from here. FWIW I'm using
Chrome on Ubuntu.
Kind regards,
Daniel
> // 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 66efa0b..1ee3436 100644
> --- a/patchwork/templates/patchwork/submission.html
> +++ b/patchwork/templates/patchwork/submission.html
> @@ -257,14 +257,14 @@
> {% else %}
> <h2>Message</h2>
> {% endif %}
> -<div class="comment">
> -<div class="meta">
> - <span>{{ submission.submitter|personify:project }}</span>
> - <span class="pull-right">{{ submission.date }} UTC</span>
> -</div>
> -<pre class="content">
> -{{ submission|commentsyntax }}
> -</pre>
> +<div class="patch-message">
> + <div class="meta">
> + <span>{{ submission.submitter|personify:project }}</span>
> + <span class="message-date">{{ submission.date }} UTC</span>
> + </div>
> + <pre class="content">
> + {{ submission|commentsyntax }}
> + </pre>
> </div>
>
> {% for item in comments %}
> @@ -273,15 +273,48 @@
> {% endif %}
>
> <a name="{{ item.id }}"></a>
> -<div class="comment">
> -<div class="meta">
> - <span>{{ item.submitter|personify:project }}</span>
> - <span class="pull-right">{{ 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="patch-message">
> + <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 %}
> + <button class="comment-action-unaddressed text-warning" value="false">
> + <span class="glyphicon glyphicon-warning-sign" aria-hidden="true"></span>
> + 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 %}
> + <button class="comment-action-addressed text-success" value="true">
> + <span class="glyphicon glyphicon-ok-circle" aria-hidden="true"></span>
> + Addressed
> + </button>
> + {% endif %}
> + </div>
> + </div>
> + <pre class="content">
> + {{ item|commentsyntax }}
> + </pre>
> </div>
> {% endfor %}
>
> @@ -290,7 +323,7 @@
> {% include "patchwork/partials/download-buttons.html" %}
> <h2>Patch</h2>
> </div>
> -<div id="patch" class="patch">
> +<div id="patch" data-patch-id={{submission.id}} class="patch">
> <pre class="content">
> {{ submission|patchsyntax }}
> </pre>
> diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
> index 3e6874a..ac9cb44 100644
> --- a/patchwork/views/patch.py
> +++ b/patchwork/views/patch.py
> @@ -128,6 +128,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
> --
> 2.33.0.rc1.237.g0d66db33f3-goog
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list