Removed same text on comment reply form #9509

waridrox · 2021-04-14T22:36:33Z

Fixes #9500 and #9508
Removed same text on first comment reply

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

PR is descriptively titled 📑 and links the original issue above 🔗
tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
code is in uniquely-named feature branch and has no merge conflicts 📁
screenshots/GIFs are attached 📎 in case of UI updation
ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

gitpod-io · 2021-04-14T22:36:35Z

codeclimate · 2021-04-14T22:37:24Z

Code Climate has analyzed commit 45a1ba9 and detected 0 issues on this pull request.

View more on Code Climate.

codecov · 2021-04-14T22:43:58Z

Codecov Report

❗ No coverage uploaded for pull request base (main@1902557). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #9509   +/-   ##
=======================================
  Coverage        ?   81.35%           
=======================================
  Files           ?       98           
  Lines           ?     5928           
  Branches        ?        0           
=======================================
  Hits            ?     4823           
  Misses          ?     1105           
  Partials        ?        0

RuthNjeri · 2021-04-15T19:12:55Z

Thanks, @waridrox, is it possible to add a test for this?

waridrox · 2021-04-15T20:58:16Z

Sure, @RuthNjeri I'll add one 😄 but first I wanted to confirm if this problem persists on production env, which may then bring some modifications to the above changes. Thanks :)

RuthNjeri · 2021-04-18T18:39:35Z

Alright, thanks @waridrox. I think you can test it on the stable environment first...there was a way to push directly to stable, @jywarren, is this something @waridrox can do?

jywarren · 2021-04-26T15:50:42Z

Hi, actually you need push rights to publiclab/plots2 unstable branch, and then it publishes at http://unstable.publiclab.org/. @RuthNjeri i believe you have these privileges but if you don't get to it by tomorrow i can do it! However, we're pretty satisfied with either a system test or a manual test in GitPod, if that's easier?

Thanks!

RuthNjeri · 2021-05-01T20:40:42Z

Thanks @jywarren, I pushed your changes to the unstable branch @waridrox, you can test your changes in the link Jeff shared for the stable branch

waridrox · 2021-05-01T20:56:08Z

Thanks a lot @RuthNjeri for the help, yes the issue does solves partially on the first reply but repeats after the second reply. This has also been tested on the official website here:

unstable.mp4

So I think we need to make the changes such that the Reply to a Comment is not generated after the first reply...

RuthNjeri · 2021-05-02T13:34:54Z

@waridrox, the unstable site has the changes for this PR, to understand you better, are you saying that the current changes do not fix the initial problem that was also happening on the main website?

waridrox · 2021-05-02T14:10:55Z

@waridrox, the unstable site has the changes for this PR, to understand you better, are you saying that the current changes do not fix the initial problem that was also happening on the main website?

I'm sorry if I puzzled you 😅, but in an attempt to solve the main issue, i.e.

removing reply text on the first reply to a comment, there was another issue that was observed after solving the above, i.e.
After replying once on a comment, if we reply again, i.e. from the second comment onwards, the contents of the previous comment reply form are copied to a new comment reply form even if reply to a comment button is not clicked.

jywarren · 2021-05-04T20:21:42Z

app/views/comments/_form.html.erb

@@ -92,7 +92,7 @@
        rows="6" 
        cols="40" 
        placeholder="<%= placeholder %>"
-      ><%= location == :edit ? comment.comment : params[:body] %></textarea>
+      ><%= location == :edit ? comment.comment : params[:none] %></textarea>


Hmm, i guess we need a different solution. What was the original intent of this phrase?

location == :edit ? comment.comment : params[:body]

Is it to see if the location is "edit" and if it is, display the existing comment text, but if not, to display the value of the params[:body] variable?

I wonder if it makes the assumption that the params[:body] variable is corresponding to... the submitted content of a comment, and if we should be checking to ensure it's the correct value in some other way? Remember that this may be used both on initial pageload and also in an AJAX response, which complicates this.

It could be useful to monitor the network pane of the Dev Tools to see if the partial template being sent from the server to generate this textarea is bearing the correct content?

gitpod-io · 2021-05-05T16:23:20Z

waridrox · 2021-05-05T16:31:26Z

@jywarren @RuthNjeri, so I tried to investigate the problem further, figuring out where the data gets passed on after making a comment, and I realised that the text input field’s value was not cleared after making a POST AJAX request along with a NotyNotification! 😅. That's why the same text was repeating in the comment reply form...

So, resetting the text input field value resolved the issue:

fixed.mp4

jywarren · 2021-05-05T17:57:44Z

Oh wow awesome!

Hmm, is the system test failure unrelated? Shall we restart it?

waridrox · 2021-05-05T21:31:31Z

Oh wow awesome!

Hmm, is the system test failure unrelated? Shall we restart it?

Will take a look at this soon... Also, I think maybe these tests will have some effect if #9384 got merged due to the waiting time of the debounce function...

waridrox · 2021-05-11T19:52:01Z

So an edge case which I forgot to thought of 😅. Clearing the contents of all text input fields meant that editing the comment was not possible with previous contents of the comment, but with the new changes it does. I'm not able to figure out though why this test is still failing 😕.

Once this is fixed, all tests will pass :)

waridrox · 2021-05-13T22:34:57Z

Ok so finally figured out why the tests were failing, the textarea content value was getting cleared when the reply to individual comments were published. Fixed that and the tests pass now :) @jywarren pls have a look!

final-test.mp4

pydevsg

Good work @waridrox 🎉

RuthNjeri · 2021-05-16T10:05:36Z

Thanks for working on this @waridrox and great job investigating the edge cases. Would you like to write a test for this? I think it will be useful for anyone to understand what is expected of the code written. You can also set it up as another issue so that we merge this one first if you would like.

RuthNjeri

LGTM: just a pending test.

waridrox · 2021-05-16T10:09:51Z

LGTM: just a pending test.

Sure thing! 🚀

jywarren · 2021-05-17T22:31:59Z

Awesome work. Thank you so much!!!!!!

* Removed same text on first comment reply * cleared textarea value * Clear value only for the textarea * Fixed redundant removal of content on individual reply * Added tests to check clear input field on comment replies

Removed same text on first comment reply

45a1ba9

waridrox requested a review from a team as a code owner April 14, 2021 22:36

jywarren reviewed May 4, 2021

View reviewed changes

cleared textarea value

7d5d74f

Clear value only for the textarea

d007532

Fixed redundant removal of content on individual reply

aa20cbc

waridrox requested a review from jywarren May 15, 2021 08:01

pydevsg approved these changes May 15, 2021

View reviewed changes

RuthNjeri approved these changes May 16, 2021

View reviewed changes

Added tests to check clear input field on comment replies

6abf060

jywarren merged commit c4ecf7e into publiclab:main May 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removed same text on comment reply form #9509

Removed same text on comment reply form #9509

waridrox commented Apr 14, 2021 •

edited

Loading

gitpod-io bot commented Apr 14, 2021

codeclimate bot commented Apr 14, 2021

codecov bot commented Apr 14, 2021 •

edited

Loading

RuthNjeri commented Apr 15, 2021

waridrox commented Apr 15, 2021 •

edited

Loading

RuthNjeri commented Apr 18, 2021

jywarren commented Apr 26, 2021

RuthNjeri commented May 1, 2021

waridrox commented May 1, 2021 •

edited

Loading

RuthNjeri commented May 2, 2021

waridrox commented May 2, 2021 •

edited

Loading

jywarren May 4, 2021

gitpod-io bot commented May 5, 2021

waridrox commented May 5, 2021 •

edited

Loading

jywarren commented May 5, 2021

waridrox commented May 5, 2021 •

edited

Loading

waridrox commented May 11, 2021 •

edited

Loading

waridrox commented May 13, 2021 •

edited

Loading

pydevsg left a comment

RuthNjeri commented May 16, 2021

RuthNjeri left a comment

waridrox commented May 16, 2021

jywarren commented May 17, 2021

Removed same text on comment reply form #9509

Removed same text on comment reply form #9509

Conversation

waridrox commented Apr 14, 2021 • edited Loading

gitpod-io bot commented Apr 14, 2021

codeclimate bot commented Apr 14, 2021

codecov bot commented Apr 14, 2021 • edited Loading

Codecov Report

RuthNjeri commented Apr 15, 2021

waridrox commented Apr 15, 2021 • edited Loading

RuthNjeri commented Apr 18, 2021

jywarren commented Apr 26, 2021

RuthNjeri commented May 1, 2021

waridrox commented May 1, 2021 • edited Loading

RuthNjeri commented May 2, 2021

waridrox commented May 2, 2021 • edited Loading

jywarren May 4, 2021

Choose a reason for hiding this comment

gitpod-io bot commented May 5, 2021

waridrox commented May 5, 2021 • edited Loading

jywarren commented May 5, 2021

waridrox commented May 5, 2021 • edited Loading

waridrox commented May 11, 2021 • edited Loading

waridrox commented May 13, 2021 • edited Loading

pydevsg left a comment

Choose a reason for hiding this comment

RuthNjeri commented May 16, 2021

RuthNjeri left a comment

Choose a reason for hiding this comment

waridrox commented May 16, 2021

jywarren commented May 17, 2021

waridrox commented Apr 14, 2021 •

edited

Loading

codecov bot commented Apr 14, 2021 •

edited

Loading

waridrox commented Apr 15, 2021 •

edited

Loading

waridrox commented May 1, 2021 •

edited

Loading

waridrox commented May 2, 2021 •

edited

Loading

waridrox commented May 5, 2021 •

edited

Loading

waridrox commented May 5, 2021 •

edited

Loading

waridrox commented May 11, 2021 •

edited

Loading

waridrox commented May 13, 2021 •

edited

Loading