Raxel Gutierrez <ra...@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 <ra...@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@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
