Refactor JS Function add_tag() and create add_comment() for Javascript API #6422

nstjean · 2019-10-08T17:37:51Z

Fixes #6411 (<=== Add issue number here)

Creating a Pull Request early to get feedback! I'm not sure if I'm doing this right.

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!

nstjean · 2019-10-11T14:27:46Z

@cesswairimu Can you (or any of the mentors!) check my code to make sure I'm doing this correctly? Thank you!!

cesswairimu · 2019-10-11T19:43:02Z

Sure, not sure why travis is failing....taking a look

nstjean · 2019-10-14T14:47:37Z

@jywarren Can you take a look at my code and let me know if this is what you intended? I wasn't sure if you wanted to reference the form at all in the Javascript.

nstjean · 2019-11-02T18:31:44Z

AddComment is done!

I had to rebase in branch before pushing, I don't know if I did it right. Now it's showing me a bunch of codeclimate errors.

I edited notes/_comment.html.erb because the tag nesting was incorrect. I had to change it so that the comment p was tested inside the .comment-class.

jywarren · 2019-11-19T18:26:23Z

Wow, it looks like this may need some rebasing and cleaning up! @nstjean this can be a bit rough, but please save a local copy of your branch with git branch refactor-js-backup so nothing goes wrong.

Would you like to try rebasing again over the latest master branch? I can also help a bit with this!

There's some guidance here, however!

https://publiclab.org/wiki/contributing-to-public-lab-software#Rewinding+the+master+branch

jywarren · 2019-11-19T18:27:36Z

Also, i think this needs parentheses to disambiguate the || logic, just a note!

The rest of the CodeClimate changes can be ignored for now, as they're mostly stylistic!

jywarren · 2019-11-19T18:27:55Z

Thanks for sticking with this one! It's a really awesome one! Just ping me if you get super stuck!

nstjean · 2019-11-19T20:07:21Z

Yes I'll try rebasing using rewinding the master branch!

nstjean · 2019-11-21T17:20:26Z

@jywarren I reset the HEAD, pulled from upstream (publiclab), switched to my branch, rebased. But now when trying to run tests on my local everything is failing, even in Master. Do I have to do something else?

nstjean · 2019-11-21T18:02:27Z

When I try to run my test for my javascript function I get this error, which points to a Chrome version error. But I haven't changed anything other than pulling the new version of Master.

ERROR["test_adding_a_tag_via_javascript", #<Minitest::Reporters::Suite:0x00005587bc60d350 @name="CommentTest">, 1.5917023420042824]
 test_adding_a_tag_via_javascript#CommentTest (1.59s)
Selenium::WebDriver::Error::SessionNotCreatedError:         Selenium::WebDriver::Error::SessionNotCreatedError: session not created: This version of ChromeDriver only supports Chrome version 78
            test/system/comment_test.rb:9:in `block in <class:CommentTest>'

jywarren · 2019-11-21T21:15:04Z

app/assets/javascripts/ajax.js

@@ -0,0 +1,9 @@
+function sendFormSubmissionAjax(dataObj, selector) {


This is looking good! Perhaps this file needs a more descriptive name, however; but is it a generic utility? If so, maybe can add a sentence or two on how to use it?

Thanks!

Yes, I should do that!

jywarren · 2019-11-22T18:05:27Z

This looks good!

Not sure where this error is coming from though:

ERROR["test_find_by_tagname_and_user_id", #<Minitest::Reporters::Suite:0x00005653ad056bb0 @name="NodeTest">, 2.091748925999923]
 test_find_by_tagname_and_user_id#NodeTest (2.09s)
NoMethodError:         NoMethodError: undefined method `title' for nil:NilClass
            test/unit/node_test.rb:529:in `block in <class:NodeTest>'

That's for this test:

plots2/test/unit/node_test.rb

Line 529 in 8c2713f

    
           assert_equal 'Question by a moderated user', Node.find_by_tag_and_author('question:spectrometer', 9, 'questions').first.title

... i guess it seems unrelated? I'll try restarting the unit tests to see what happens..

Thanks for the epic rebase!!!!

jywarren · 2019-11-22T18:05:54Z

test/system/tag_test.rb

@@ -0,0 +1,27 @@
+require 'test_helper'


Wow, really awesome tests!!!!

jywarren · 2019-11-22T20:53:10Z

test/system/comment_test.rb

+    page.evaluate_script("addComment('fantastic four')")
+
+    # check that the tag showed up on the page
+    assert_selector('#comments-list .comment-body p', text: 'fantastic four')


jywarren · 2019-11-22T20:54:33Z

app/assets/javascripts/submit_form_ajax.js

+// Allows data to be submitted from anywhere on the page using Javascript without using the form itself
+function sendFormSubmissionAjax(dataObj, selector) {
+  $.ajax({
+    url: $(selector).attr('action'), // grab the URL from the form


Could we offer a parameter like url which defaults back to this value? That way it could be done without a form!

nstjean · 2019-11-26T20:30:36Z

@jywarren I've been thinking about this. Will it get confusing if sometimes the function is passed a URL and sometimes it is passed the ID of the form? It only needs one or the other. Maybe it should be two different functions? Say function2 takes in a URL as a parameter and submits the form. Then function2 takes in a selector, gets the URL from the form, and then calls function1?

jywarren · 2019-11-26T21:59:19Z

Could we... Refactor the form so it submits through ajax only? I am not that worries about the ambiguity but I would be fine consolidating everything into a single remote call a dropping the direct use of the html form if it's bothering you! Thanks!

…

On Tue, Nov 26, 2019, 3:31 PM Natalie St Jean ***@***.***> wrote: @jywarren <https://github.com/jywarren> I've been thinking about this. Will it get confusing if sometimes the function is passed a URL and sometimes it is passed the ID of the form? It only needs one or the other. Maybe it should be two different functions? Say function2 takes in a URL as a parameter and submits the form. Then function2 takes in a selector, gets the URL from the form, and then calls function1? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6422?email_source=notifications&email_token=AAAF6J5NGAI7YGZC2CI4TJLQVWBPTA5CNFSM4I6U7RQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFHK65A#issuecomment-558804852>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J4YCOYPN6BJHAP6P7DQVWBPTANCNFSM4I6U7RQA> .

nstjean · 2019-11-27T14:17:36Z

Okay! :)

nstjean · 2019-11-27T18:24:35Z

Ok I allowed either a selector or a url to get passed in and it decides what to do from there. :) I also added a test case for a URL being used in both addComment and addTag. Tests work on local.

jywarren · 2019-11-27T20:16:41Z

app/assets/javascripts/comment.js

@@ -51,3 +51,10 @@ function insertTitleSuggestionTemplate() {
  var newText = currentText+template;
  element.val(newText);
 }
+
+// JS API for submitting comments
+function addComment(comment, submitTo) {


Great, and this works appropriately for nested comments (responding to other comments) as well as top-level comments? Just checking!

jywarren · 2019-11-27T20:18:14Z

test/system/tag_test.rb

+
+  end
+
+  test 'adding a tag via javascript with url only' do


Love this. Just one more question - does adding tags to your profile also still work? I think that might've become broken at some point because we (unfortunately) share code between node tags and user tags, but just curious if this touches that at all? Thanks!

nstjean · 2019-11-27T20:36:03Z

Let me check on those possibilities!

jywarren · 2019-11-27T21:05:50Z

Thanks. What's really nice about your code is that we will be moving away from more brittle implementations like the current one, and backing it up with tests, so the hope is to have fewer "gotchas" like this in the future. Thanks a lot!!!

jywarren · 2019-11-27T21:08:14Z

For what it's worth, i just tried adding a profile tag and got some kind of error i think... it submitted via XHR (ajax) to: https://publiclab.org/profile/warren?remote=true&name=providence also which I think isn't right!

jywarren · 2019-11-27T21:09:44Z

So, what we can do is merge this, pending the comment-responses clarification above, and then in a new PR (if you're willing!) try to fix the profile tagging -- preferably by extending the methods you've written, to have a more rational system for handling profile tagging, too!

The nice thing is that we are trying to add profile tags at different places around the site, so having a general purpose JS method to do this will be really great.

…t for it

nstjean · 2019-11-28T00:26:26Z

Ok! I added the ability to add a parent comment ID number to the function call so it will nest it appropriately... and added a test to check.

I can check on the profile tag implementation in another PR!

jywarren · 2019-11-28T00:45:58Z

Awesome ok this is ready to merge then?

…

nstjean · 2019-11-28T00:49:39Z

It's throwing an error on my test test_adding_a_tag_via_javascript_with_url_only - but it works when I run it on local. I'll try to figure out what's going on with it.

nstjean · 2019-11-28T01:49:28Z

It appears I have fixed it! Should be all set!

jywarren · 2019-11-28T01:52:42Z

Super! Now go take a break! 😁🎉🙌🏽

You'll be able to test it out at stable.publiclab.org soon once it builds!

nstjean · 2019-11-28T01:55:45Z

Thanks! 🎉 Enjoy your Thanksgiving break!

…t API (publiclab#6422) * change addTag to AJAX * addTag ajax now works * add system test for addTag js function * fix codeclimate error * add js function AddComment and system test for function * add parentheses to logic statement * add JS function sendFormSubmissionAjax to reduce duplicate code * rename ajax.js to submit_form_ajax.js * remove parentheses in Node.find_by_tag_and_author * allow sendFormSubmissionAjax to take in a selector or a url * allow js addComment and addTest to use URL and add test case for both * fix codeclimate error * add to JS addComment ability to reply to parent comment - and add test for it * attempt to fix tag_test travis error

nstjean mentioned this pull request Oct 15, 2019

Weekly Community Check In 36: Create a first-timer issue #6470

Closed

nstjean mentioned this pull request Oct 28, 2019

Weekly community Check-In #37 - Making new FTO's and Making IS UI stable publiclab/image-sequencer#1280

Closed

nstjean requested a review from a team as a code owner November 2, 2019 18:26

nstjean changed the title ~~[WIP] Refactor JS Function add_tag() and create add_comment() for Javascript API~~ Refactor JS Function add_tag() and create add_comment() for Javascript API Nov 3, 2019

nstjean mentioned this pull request Nov 11, 2019

Weekly community Check-In #38 Planning GCI and Prize Guidelines #6762

Closed

nstjean force-pushed the refactor-js-functions branch from 8ad2ca3 to 0b1e514 Compare November 21, 2019 16:27

nstjean force-pushed the refactor-js-functions branch from 4dfd7a8 to 070df16 Compare November 21, 2019 19:09

jywarren reviewed Nov 21, 2019

View reviewed changes

jywarren reviewed Nov 22, 2019

View reviewed changes

test/system/tag_test.rb

@@ -0,0 +1,27 @@

require 'test_helper'

Copy link

Member

jywarren Nov 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, really awesome tests!!!!

jywarren reviewed Nov 22, 2019

View reviewed changes

nstjean mentioned this pull request Nov 25, 2019

Weekly community Check-In #40 - "Pre GCI Preparation!" #6845

Closed

nstjean added 4 commits November 27, 2019 10:00

change addTag to AJAX

5b51885

addTag ajax now works

b3478f2

add system test for addTag js function

9be74a0

fix codeclimate error

a71e1c6

nstjean added 7 commits November 27, 2019 10:00

add js function AddComment and system test for function

30320ef

add parentheses to logic statement

0395e81

add JS function sendFormSubmissionAjax to reduce duplicate code

bb096aa

rename ajax.js to submit_form_ajax.js

54fff67

remove parentheses in Node.find_by_tag_and_author

8dc0c8c

allow sendFormSubmissionAjax to take in a selector or a url

2d3c642

allow js addComment and addTest to use URL and add test case for both

08962f6

nstjean force-pushed the refactor-js-functions branch from 67c3b0f to 08962f6 Compare November 27, 2019 18:21

fix codeclimate error

49008c6

jywarren reviewed Nov 27, 2019

View reviewed changes

add to JS addComment ability to reply to parent comment - and add tes…

3895a72

…t for it

attempt to fix tag_test travis error

85b0497

jywarren merged commit 2539bcb into publiclab:master Nov 28, 2019

nstjean mentioned this pull request Dec 2, 2019

Fix tagging user profiles and add JS ajax function #6886

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor JS Function add_tag() and create add_comment() for Javascript API #6422

Refactor JS Function add_tag() and create add_comment() for Javascript API #6422

nstjean commented Oct 8, 2019 •

edited

Loading

nstjean commented Oct 11, 2019

cesswairimu commented Oct 11, 2019

nstjean commented Oct 14, 2019

nstjean commented Nov 2, 2019 •

edited

Loading

jywarren commented Nov 19, 2019

jywarren commented Nov 19, 2019

jywarren commented Nov 19, 2019

nstjean commented Nov 19, 2019

nstjean commented Nov 21, 2019 •

edited

Loading

nstjean commented Nov 21, 2019

jywarren Nov 21, 2019

nstjean Nov 21, 2019

jywarren commented Nov 22, 2019

jywarren Nov 22, 2019

jywarren Nov 22, 2019

jywarren Nov 22, 2019

nstjean commented Nov 26, 2019

jywarren commented Nov 26, 2019 via email

nstjean commented Nov 27, 2019

nstjean commented Nov 27, 2019

jywarren Nov 27, 2019

jywarren Nov 27, 2019

nstjean commented Nov 27, 2019

jywarren commented Nov 27, 2019

jywarren commented Nov 27, 2019

jywarren commented Nov 27, 2019

nstjean commented Nov 28, 2019

jywarren commented Nov 28, 2019 via email

nstjean commented Nov 28, 2019

nstjean commented Nov 28, 2019

jywarren commented Nov 28, 2019

nstjean commented Nov 28, 2019

		@@ -0,0 +1,9 @@
		function sendFormSubmissionAjax(dataObj, selector) {

Refactor JS Function add_tag() and create add_comment() for Javascript API #6422

Refactor JS Function add_tag() and create add_comment() for Javascript API #6422

Conversation

nstjean commented Oct 8, 2019 • edited Loading

nstjean commented Oct 11, 2019

cesswairimu commented Oct 11, 2019

nstjean commented Oct 14, 2019

nstjean commented Nov 2, 2019 • edited Loading

jywarren commented Nov 19, 2019

jywarren commented Nov 19, 2019

jywarren commented Nov 19, 2019

nstjean commented Nov 19, 2019

nstjean commented Nov 21, 2019 • edited Loading

nstjean commented Nov 21, 2019

jywarren Nov 21, 2019

Choose a reason for hiding this comment

nstjean Nov 21, 2019

Choose a reason for hiding this comment

jywarren commented Nov 22, 2019

jywarren Nov 22, 2019

Choose a reason for hiding this comment

jywarren Nov 22, 2019

Choose a reason for hiding this comment

jywarren Nov 22, 2019

Choose a reason for hiding this comment

nstjean commented Nov 26, 2019

jywarren commented Nov 26, 2019 via email

nstjean commented Nov 27, 2019

nstjean commented Nov 27, 2019

jywarren Nov 27, 2019

Choose a reason for hiding this comment

jywarren Nov 27, 2019

Choose a reason for hiding this comment

nstjean commented Nov 27, 2019

jywarren commented Nov 27, 2019

jywarren commented Nov 27, 2019

jywarren commented Nov 27, 2019

nstjean commented Nov 28, 2019

jywarren commented Nov 28, 2019 via email

nstjean commented Nov 28, 2019

nstjean commented Nov 28, 2019

jywarren commented Nov 28, 2019

nstjean commented Nov 28, 2019

nstjean commented Oct 8, 2019 •

edited

Loading

nstjean commented Nov 2, 2019 •

edited

Loading

nstjean commented Nov 21, 2019 •

edited

Loading