add_unflag_tooltip_to_clarify_btn

[stephaniequintana]

edit complete, "unflag" tooltip added to button

Fixes #10820

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

As designed, this was a simple first-time contribution. I want to ensure that I initiated the PR from the correct branch. I believe it is typically initiated from the changed-branch rather than the main, which seems to be the case here. Any clarity would be greatly appreciated. Thanks in advance.

[gitpod-io]

[codeclimate]

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

View more on Code Climate.

[TildaDares]

Thanks @stephaniequintana. This looks great!

[stephaniequintana]

@TildaDares - Thank you, Tilda. I was under the impression that we didn't need to add a screen shot for these simple fixes, but I'd like for this to pass all of the tests... I will go ahead and add those to the PR.

[TildaDares]

Hi @stephaniequintana, the screenshot isn’t the issue. It has something to do with the code but not yours.

[stephaniequintana]

Ahh, I see. I'll definitely get into the habit of adding screenshots/gifs for better documentation, but it's good to know that isn't the issue here. Thank you, Tilda.

[jywarren]

Hi @stephaniequintana we can re-open this; the error is as follows:

============[Screenshot]: tmp/screenshots/failures_test_unflag_post_in_spam2.png
 FAIL SpamTest#test_unflag_post_in_spam2 (114.97s)
        expected to find css ".noty_body" but there were no matches
        test/system/spam2_test.rb:41:in `block in <class:SpamTest>'

Is there any way this change could have affected this test? Did we miss keeping some CSS class that was needed?

plots2/test/system/spam2_test.rb

Lines 37 to 42 in ba254cf

	test "unflag post in spam2" do
	flag_page = nodes(:about)
	visit "/spam2/flags"
	find("a[href='/moderate/remove_flag_node/#{flag_page.id}'").click()
	assert_selector('.noty_body', text: 'Node unflagged')
	end

If we can't think of a way, it's sometimes possible that CSS/JS tests are a little inconsistent as they are often asynchronous and timing-based. We can reopen the issue to re-trigger the tests... let me do that just in case it resolves itself.

[jywarren]

Yes it happened a 2nd time -- @stephaniequintana any thoughts on how this might be caused by the change?

https://github.com/publiclab/plots2/runs/5741570406?check_suite_focus=true#step:9:99

[stephaniequintana]

@jywarren, I'm just now taking a look at it. To be honest, I did not really analyze the code when I made the changes. I did a simple copy/paste with no changes to the CSS. I am going to study it a bit more, though.

Embarrassingly, I deleted the branch earlier thinking that it was no longer needed. Perhaps I'm still green on how PRs are merged, but it makes sense that my branch should exist at this point. Shall I create another PR?

For convenience, the original issue is here

[stephaniequintana]

@jywarren, I am at a loss. The only change that was made is that we added:
data-toggle="tooltip" data-placement="top" title="Remove all flags" to the existing button.
No classes were added/deleted/changed. For convenience, the original button is:

<a class="btn btn-xs border-curve font-weight-bold btn-warning unflag"
  data-remote="true"href="/moderate/remove_flag_node/<%= flag.id %>">Unflag</a>

A complete stab in the dark: perhaps it didn't trigger b/c there is not a button currently rendered(??) - I assume the test is built around this, though.

Are we certain the test passes on the the main branch?
I did try to run it in GitPod to no avail.
(I most likely need direction on how to correctly run them)

Please let me know how you would like for me to proceed.

[jywarren]

@jywarren, I'm just now taking a look at it. To be honest, I did not really analyze the code when I made the changes. I did a simple copy/paste with no changes to the CSS. I am going to study it a bit more, though.

Embarrassingly, I deleted the branch earlier thinking that it was no longer needed. Perhaps I'm still green on how PRs are merged, but it makes sense that my branch should exist at this point. Shall I create another PR?

For convenience, the original issue is here

No worries at all. You should be able to re-create the branch name and pull down changes from the PR's remote branch, but indeed i see when i follow https://github.com/stephaniequintana/plots2/tree/patch-1 it's gone. You can try opening it in GitPod again, maybe, and then copying the branch to another branch name? It's certainly still somewhere because we're working with it in this PR.

I'm going to take a closer look and see if I can imagine how it could be related. These kinds of things can be really subtle... let's try but we may have to be patient with ourselves to see the issue.

[stephaniequintana]

@jywarren - I already created another branch (but unwittingly named it differently). I'll change the name now so that it'll be there when we're ready for it 👍

edit: I'm going to try to find it first. That'll will be fun and interesting (for me).

last edit: full disclosure: I had deleted not only the branch, but the repo - in dealing with a different issue, rather than squashing multiple commits or rebasing to an earlier commit I panicked and deleted/re-forked the entire repo. I don't believe it has anything to do with the error (I had done this about an hour before you ran the tests it had previously failed), but you should be aware. I do find it curious that the tests are still running.

My new repo with the new branch is set and ready for a PR. Please let me know if/when I need to create one. My apologies for the congestion; I did learn a valuable lesson.

[stephaniequintana]

@jywarren, I took another look and at the error output and have become convinced that the error is due to Codecov. From the system test:

The first related link shows a 404 and the next shows:

The "No token ..." issue has been raised across github and is causing tests to fail sporadically, see here and here. Differing work-arounds are mentioned, but, frustratingly, no real fix is provided.

If Codecov is the culprit, it may be also affecting #10863, #10791, #10585, possibly #10802 (token.rb) and multiple others (where the results have been archived).

Hopefully, I'm not completely off-base. Let me know what you think.

[jywarren]

Unfortunately I won't have an opportunity to work on this in the next few days, i'm so sorry! I wonder if @TildaDares may be able to help out at all. It's a pretty busy moment, so we can also take this slow, no rush. And - if you can open a new PR and link it from here, that's probably for the best, just in case. We can probably pull from it to re-create this branch. Or, you could push that code to this remote branch and re-create the branch? That will re-trigger the tests, but that's not a bad thing.

[stephaniequintana]

@jywarren and @TildaDares - absolutely no worries, you had already mentioned we're going to be patient with this one and I can only imagine how much you all are juggling.

Per above, I create a new PR, #10867, in reference for the deleted branch on this one.

[PeculiarE]

Hiya @stephaniequintana. If I'm not mistaken, you're pointing to the wrong PR. #10867 is @noi5e's PR related to the React commenting system while your own PR relating to this issue is actually #10876. Easy to mix-up, I know 😆.

[TildaDares]

Hi @stephaniequintana, I think you can close this PR now since you've resolved the issue in #10876

[stephaniequintana]

Resolved #10820