Follow tag asynchronously #4587

shubhscoder · 2019-01-10T11:15:53Z

Fixes #4565
GIF for the feature

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 📁
PR is descriptively titled 📑
screenshots/GIFs are attached 📎 in case of UI updation
ask @publiclab/reviewers for help, in a comment below

Thanks!

plotsbot · 2019-01-10T11:27:48Z

	1 Message
📖	@shubhscoder Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.

Generated by 🚫 Danger

shubhscoder · 2019-01-10T17:28:39Z

@gauravano I have implemented the follow feature without redirect. But if I make changes to handle the unfollow part, then the unsubscribe tag at the subscriptions page would be broken, because it uses the same controller function. So I think it would be better that we open a different issue in which we fix both unfollow and unsubscribe together. What do you think?

jywarren

I made some suggestions for modularizing this code and making it more "futureproofed" as well!

jywarren · 2019-01-10T22:44:25Z

app/controllers/subscription_controller.rb

@@ -46,32 +46,23 @@ def add
          begin
            tag.save!
          rescue ActiveRecord::RecordInvalid
-            flash[:error] = tag.errors.full_messages
-            redirect_to "/subscriptions" + "?_=" + Time.now.to_i.to_s
+            render :json => { status: "500", message: "error" }


Hi, this is great but I think we need to preserve the original non-JS version too, as this method is used in other places. Could we do that?

Yes, we can do that, just need to check if request.xhr return true right?

jywarren · 2019-01-10T22:45:47Z

app/views/tag/index.html.erb

@@ -1,4 +1,7 @@
 <div class="col-md-12">
+  <div class="alert alert-success" id="follow-success" style="display: none;position: absolute;">


Here, maybe we need to use Noty to display something using pure JavaScript, so we can consolidate all the JS for this into one place, rather than inserting new HTML... you can read about Noty here:

https://github.com/publiclab/plots2/search?utf8=%E2%9C%93&q=noty&type=

Okay, that looks good I will change that!

jywarren · 2019-01-10T22:49:10Z

app/views/tag/index.html.erb

@@ -80,3 +83,21 @@
 </div>

 <div class="text-center"> <%= will_paginate @tags, :renderer => BootstrapPagination::Rails if @paginated %></div>
+
+<script type="text/javascript">


Here, I'd like to propose that we make this into a separate JavaScript file, and a function which accepts a jQuery selector like:

setupRemoteFollow($('follow-btn-remote'));

What it can do is read the href attribute of each <a class="follow-btn-remote"> tag on the page, and parse it to set up the remote call.

Then we include this JS file on all pages. Then all we have to do to implement the remote call is to give any link the follow-btn-remote class, and it gets hooked up automatically. And if JavaScript fails, it will revert to being a normal link that doesn't require JS.

How does that sound?

Yes sure sounds good, I will do that !

@jywarren I am trying to implement this. We are trying to generalize and use in this file in various remote requests right? But each one would have different requirements, like in this case we need to change the 'follow' button to 'following', etc. In this case how can we implement this? Thank you !

shubhscoder · 2019-01-23T07:32:37Z

@jywarren I have made the required changes. Sorry for the delay. This is the updated GIF

Added link to manage subscriptions Add precondition failed http error code cleanup codeclimate fix Fix typo change remove data-remote Preserve code for non xhr requests Seperate JS and html Codeclimate fix Code Climate fixes Updated tests

jywarren

Just a couple changes, but this is really nice!!!! Thanks @shubhscoder !!!

Once this is complete, perhaps we could get your help with #4759 -- I'd really like to figure this one out! Thanks!!!

👍

jywarren · 2019-02-28T21:25:37Z

config/initializers/assets.rb

@@ -48,5 +48,6 @@
  'user_tags.css.scss',
  'wiki.css',
  'horsey.js',
-  'atwho_autocomplete.js'
+  'atwho_autocomplete.js',
+  'remote_link_handler.js'


Actually do you think we could include this only on that one page, using <%= javascript_include_tag "remote_link_handler" %> (i think that's the syntax, you may need to check on other pages to confirm this)? And perhaps a different name would be best, like async_tag_subscriptions.js?

shubhscoder · 2019-03-01T09:35:51Z

@jywarren I m not sure how codeclimate can be fixed, because the nested ifs and concatenation of literals is kindoff necessary. Do you think there's a way around this?

grvsachdeva · 2019-03-01T13:47:56Z

@shubhscoder we can see about codeclimate fixes after handling the Travis tests errors - https://travis-ci.org/publiclab/plots2/builds/500247993#L3275-L3317

shubhscoder · 2019-03-01T16:27:23Z

@shubhscoder we can see about codeclimate fixes after handling the Travis tests errors - https://travis-ci.org/publiclab/plots2/builds/500247993#L3275-L3317

Yeah, travis errors are because in the production environment async_tag_subscriptions.js is not a precompiled asset. @gauravano do you have any idea of how can we just include the javascript file in a .html.erb with
<%= javascript_include_tag "remote_link_handler" %>
without adding it to pre-compiled list of assets? Thank you!

jywarren · 2019-03-01T18:31:08Z

app/assets/javascripts/async_tag_subscriptions.js

+	    console.log("Hello");
+	    var data_recv = JSON.parse(JSON.stringify(status));
+	    notyNotification('relax', 3000, 'success', 'top', data_recv.message + 'Click <a href="../subscriptions"> here </a> to manage your subscriptions. ');
+	    var html_new = '<a rel="tooltip" title=Following' +' class="btn btn-default btn-sm active" href="'+'/unsubscribe/tag/'+ data_recv.tagname + '"> <i class="fa fa-user-plus" aria-hidden="true"></i>Following' +'</a>';


Here I think CodeClimate is wondering why there are some string concatenations between strings -- could be simpler maybe? Also, i think Following needs quotation marks?

jywarren · 2019-03-01T18:32:19Z

app/views/tag/index.html.erb

@@ -1,3 +1,4 @@
+<%= javascript_include_tag('async_tag_subscriptions.js') %>


Not sure why it doesn't like this - same as here, no?

plots2/app/views/questions/show.html.erb

Line 1 in ff7d91e

<%= javascript_include_tag('comment_expand') %>

OH, you know what? Maybe it wants to precompile it in its own file, to minify it, and since we aren't referencing it in application.js, it's not getting merged with everything else. So you could follow the instructions and:

Add `Rails.application.config.assets.precompile += %w( async_tag_subscriptions.js )` to `config/initializers/assets.rb` and restart your server

Sound right?

jywarren · 2019-03-05T01:25:20Z

Looks great -- ready for merge?

shubhscoder · 2019-03-05T06:25:23Z

@jywarren yes I think we can merge this, thank you so much for helping me along with this!

jywarren · 2019-03-05T16:31:50Z

🎉

shubhscoder · 2019-03-05T16:52:11Z

Thank you so much !

shubhscoder · 2019-03-05T16:52:46Z

@jywarren , I will start with #4759 now. Thank you!

jywarren · 2019-03-05T16:54:55Z

awesome!

…

On Tue, Mar 5, 2019 at 11:53 AM Shubham Sangamnerkar < ***@***.***> wrote: @jywarren <https://github.com/jywarren> , I will start with #4759 <#4759> now. Thank you! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4587 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJyZtmNfKW-vN1Ik1TSvEqkxeOKtiks5vTqDegaJpZM4Z5STe> .

* Added debounce for typeahead search optimization * Update README.md (#4883) The "What makes this project different" section had several long sentences which were difficult to understand. I tried to fix this by breaking the sentences down into smaller more concise sentences. * convert chars to unicode (#4901) * fixes for map module (#4909) * fixes for map module * yarn.lock update * updated yarn.lock (#4911) * updated yarn.lock * tweak * Remove useless variable assignment (#4885) * Simplify username generation * Use status module * Tiny fix * Fix indentation * Made moderate buttons to appear on the same line (#4913) * Made moderate buttons to appear on the same line * Removed break tag. * Clean up (#4902) * Clean up with Rubocop * More cleanup * Excluse views * Small refactor * More clean up * Clean up * Fix conflict * Tiny fix * Follow tag asynchronously (#4587) * Follow tag asynchronously Added link to manage subscriptions Add precondition failed http error code cleanup codeclimate fix Fix typo change remove data-remote Preserve code for non xhr requests Seperate JS and html Codeclimate fix Code Climate fixes Updated tests * Remove pre-compilation of async.js * pre compilation of js assets * cc fix try * Move verification link (#4786) * Move verification link * Change link to text * Remove unnecessary message * popover works, styling is still not showing under elements but is und… (#4906) * popover works, styling is still not showing under elements but is under.btn-default * copied yml file back in * First timer tag script (#4878) * First timer tag script * migration for attaching first-time-poster * change migration bump version to 5.1 * Update schema.rb.example * Consolidating ranges and stats (#4887) * restyle range page to be a partial * remove static stats * merge range and main stats * render range in main stats * fix failing range test * add questions vs answers graph * add go back and further buttons * minor fixes * Change contribution graph making method Change method to create graph based on the range given * fix failing test and code climate issues * remove commented code and indent * remove stats_nav partial * add contribution graph tests * review requests implemetation * move tag graph button (#4921) * move tag graph button * Update _user_controls.html.erb * Added reply by tweet feature (#3175) * Added reply by tweet feature * Updated schedule.rb file * Finalized reply-by-tweet * Corrected schema version * Minor change * Added reply_by_tweet doc * Minor changes * Added twitter gem * Minor changes * Added Environment variables in Docker * Added summery in Doc file * Corrected schema version * Added some documentation * Added some documentation * Migration timestamp changed * Changed migration * Minor change * Minor changes * Added rake to general gem list * Added bundle exec for rake/rails tasks in schedule.rb * Added path env variable * MINOR CHANGE * MINOR CHANGE * MINOR CHANGE * MINOR CHANGE * Added print statement to check print * Minor change * Minor change * Changed whenever config * Minor change * Completed reply by tweet feature * Minor change * Minor change * Added gemfile.lock * Update comment.rb * Added gemfile.lock * Minor changes * Minor changes * Minor changes * Minor changes * Add more relevant search results (#4848) * add helper functions * Modify controller to take extra results * add dict file * add more objects * refactor code and add tests * cc fix * change numbers to account for additions in fixtures * fix tests * remove unused include * code quality fixes * tab fix * changing implementation ideas, removal of unnecessary code * newline fix * cc fix * cc space inside brackets fix * modify query to get rid of redundant words * reduce word to root then tranform * Change file name * chore: update README.md (#4926) Fixes task mentioned in issue #4749 * Bump cytoscape from 3.4.2 to 3.5.0 (#4929) Bumps [cytoscape](https://github.com/cytoscape/cytoscape.js) from 3.4.2 to 3.5.0. - [Release notes](https://github.com/cytoscape/cytoscape.js/releases) - [Commits](cytoscape/cytoscape.js@v3.4.2...v3.5.0) Signed-off-by: dependabot[bot] <support@dependabot.com> * Tiny fix (#4933) * Tiny fix * Rubocop * Update 20190301075323_add_first_tag_poster.rb * updates (#4851) * fix for wiki update (#4942) * fixes, extend to title suggestions * extended to atwho

* Follow tag asynchronously Added link to manage subscriptions Add precondition failed http error code cleanup codeclimate fix Fix typo change remove data-remote Preserve code for non xhr requests Seperate JS and html Codeclimate fix Code Climate fixes Updated tests * Remove pre-compilation of async.js * pre compilation of js assets * cc fix try

* Added debounce for typeahead search optimization * Update README.md (publiclab#4883) The "What makes this project different" section had several long sentences which were difficult to understand. I tried to fix this by breaking the sentences down into smaller more concise sentences. * convert chars to unicode (publiclab#4901) * fixes for map module (publiclab#4909) * fixes for map module * yarn.lock update * updated yarn.lock (publiclab#4911) * updated yarn.lock * tweak * Remove useless variable assignment (publiclab#4885) * Simplify username generation * Use status module * Tiny fix * Fix indentation * Made moderate buttons to appear on the same line (publiclab#4913) * Made moderate buttons to appear on the same line * Removed break tag. * Clean up (publiclab#4902) * Clean up with Rubocop * More cleanup * Excluse views * Small refactor * More clean up * Clean up * Fix conflict * Tiny fix * Follow tag asynchronously (publiclab#4587) * Follow tag asynchronously Added link to manage subscriptions Add precondition failed http error code cleanup codeclimate fix Fix typo change remove data-remote Preserve code for non xhr requests Seperate JS and html Codeclimate fix Code Climate fixes Updated tests * Remove pre-compilation of async.js * pre compilation of js assets * cc fix try * Move verification link (publiclab#4786) * Move verification link * Change link to text * Remove unnecessary message * popover works, styling is still not showing under elements but is und… (publiclab#4906) * popover works, styling is still not showing under elements but is under.btn-default * copied yml file back in * First timer tag script (publiclab#4878) * First timer tag script * migration for attaching first-time-poster * change migration bump version to 5.1 * Update schema.rb.example * Consolidating ranges and stats (publiclab#4887) * restyle range page to be a partial * remove static stats * merge range and main stats * render range in main stats * fix failing range test * add questions vs answers graph * add go back and further buttons * minor fixes * Change contribution graph making method Change method to create graph based on the range given * fix failing test and code climate issues * remove commented code and indent * remove stats_nav partial * add contribution graph tests * review requests implemetation * move tag graph button (publiclab#4921) * move tag graph button * Update _user_controls.html.erb * Added reply by tweet feature (publiclab#3175) * Added reply by tweet feature * Updated schedule.rb file * Finalized reply-by-tweet * Corrected schema version * Minor change * Added reply_by_tweet doc * Minor changes * Added twitter gem * Minor changes * Added Environment variables in Docker * Added summery in Doc file * Corrected schema version * Added some documentation * Added some documentation * Migration timestamp changed * Changed migration * Minor change * Minor changes * Added rake to general gem list * Added bundle exec for rake/rails tasks in schedule.rb * Added path env variable * MINOR CHANGE * MINOR CHANGE * MINOR CHANGE * MINOR CHANGE * Added print statement to check print * Minor change * Minor change * Changed whenever config * Minor change * Completed reply by tweet feature * Minor change * Minor change * Added gemfile.lock * Update comment.rb * Added gemfile.lock * Minor changes * Minor changes * Minor changes * Minor changes * Add more relevant search results (publiclab#4848) * add helper functions * Modify controller to take extra results * add dict file * add more objects * refactor code and add tests * cc fix * change numbers to account for additions in fixtures * fix tests * remove unused include * code quality fixes * tab fix * changing implementation ideas, removal of unnecessary code * newline fix * cc fix * cc space inside brackets fix * modify query to get rid of redundant words * reduce word to root then tranform * Change file name * chore: update README.md (publiclab#4926) Fixes task mentioned in issue publiclab#4749 * Bump cytoscape from 3.4.2 to 3.5.0 (publiclab#4929) Bumps [cytoscape](https://github.com/cytoscape/cytoscape.js) from 3.4.2 to 3.5.0. - [Release notes](https://github.com/cytoscape/cytoscape.js/releases) - [Commits](cytoscape/cytoscape.js@v3.4.2...v3.5.0) Signed-off-by: dependabot[bot] <support@dependabot.com> * Tiny fix (publiclab#4933) * Tiny fix * Rubocop * Update 20190301075323_add_first_tag_poster.rb * updates (publiclab#4851) * fix for wiki update (publiclab#4942) * fixes, extend to title suggestions * extended to atwho

* Follow tag asynchronously Added link to manage subscriptions Add precondition failed http error code cleanup codeclimate fix Fix typo change remove data-remote Preserve code for non xhr requests Seperate JS and html Codeclimate fix Code Climate fixes Updated tests * Remove pre-compilation of async.js * pre compilation of js assets * cc fix try

* Added debounce for typeahead search optimization * Update README.md (publiclab#4883) The "What makes this project different" section had several long sentences which were difficult to understand. I tried to fix this by breaking the sentences down into smaller more concise sentences. * convert chars to unicode (publiclab#4901) * fixes for map module (publiclab#4909) * fixes for map module * yarn.lock update * updated yarn.lock (publiclab#4911) * updated yarn.lock * tweak * Remove useless variable assignment (publiclab#4885) * Simplify username generation * Use status module * Tiny fix * Fix indentation * Made moderate buttons to appear on the same line (publiclab#4913) * Made moderate buttons to appear on the same line * Removed break tag. * Clean up (publiclab#4902) * Clean up with Rubocop * More cleanup * Excluse views * Small refactor * More clean up * Clean up * Fix conflict * Tiny fix * Follow tag asynchronously (publiclab#4587) * Follow tag asynchronously Added link to manage subscriptions Add precondition failed http error code cleanup codeclimate fix Fix typo change remove data-remote Preserve code for non xhr requests Seperate JS and html Codeclimate fix Code Climate fixes Updated tests * Remove pre-compilation of async.js * pre compilation of js assets * cc fix try * Move verification link (publiclab#4786) * Move verification link * Change link to text * Remove unnecessary message * popover works, styling is still not showing under elements but is und… (publiclab#4906) * popover works, styling is still not showing under elements but is under.btn-default * copied yml file back in * First timer tag script (publiclab#4878) * First timer tag script * migration for attaching first-time-poster * change migration bump version to 5.1 * Update schema.rb.example * Consolidating ranges and stats (publiclab#4887) * restyle range page to be a partial * remove static stats * merge range and main stats * render range in main stats * fix failing range test * add questions vs answers graph * add go back and further buttons * minor fixes * Change contribution graph making method Change method to create graph based on the range given * fix failing test and code climate issues * remove commented code and indent * remove stats_nav partial * add contribution graph tests * review requests implemetation * move tag graph button (publiclab#4921) * move tag graph button * Update _user_controls.html.erb * Added reply by tweet feature (publiclab#3175) * Added reply by tweet feature * Updated schedule.rb file * Finalized reply-by-tweet * Corrected schema version * Minor change * Added reply_by_tweet doc * Minor changes * Added twitter gem * Minor changes * Added Environment variables in Docker * Added summery in Doc file * Corrected schema version * Added some documentation * Added some documentation * Migration timestamp changed * Changed migration * Minor change * Minor changes * Added rake to general gem list * Added bundle exec for rake/rails tasks in schedule.rb * Added path env variable * MINOR CHANGE * MINOR CHANGE * MINOR CHANGE * MINOR CHANGE * Added print statement to check print * Minor change * Minor change * Changed whenever config * Minor change * Completed reply by tweet feature * Minor change * Minor change * Added gemfile.lock * Update comment.rb * Added gemfile.lock * Minor changes * Minor changes * Minor changes * Minor changes * Add more relevant search results (publiclab#4848) * add helper functions * Modify controller to take extra results * add dict file * add more objects * refactor code and add tests * cc fix * change numbers to account for additions in fixtures * fix tests * remove unused include * code quality fixes * tab fix * changing implementation ideas, removal of unnecessary code * newline fix * cc fix * cc space inside brackets fix * modify query to get rid of redundant words * reduce word to root then tranform * Change file name * chore: update README.md (publiclab#4926) Fixes task mentioned in issue publiclab#4749 * Bump cytoscape from 3.4.2 to 3.5.0 (publiclab#4929) Bumps [cytoscape](https://github.com/cytoscape/cytoscape.js) from 3.4.2 to 3.5.0. - [Release notes](https://github.com/cytoscape/cytoscape.js/releases) - [Commits](cytoscape/cytoscape.js@v3.4.2...v3.5.0) Signed-off-by: dependabot[bot] <support@dependabot.com> * Tiny fix (publiclab#4933) * Tiny fix * Rubocop * Update 20190301075323_add_first_tag_poster.rb * updates (publiclab#4851) * fix for wiki update (publiclab#4942) * fixes, extend to title suggestions * extended to atwho

shubhscoder changed the title ~~(WIP)Follow tag asynchronously~~ [WIP]Follow tag asynchronously Jan 10, 2019

shubhscoder changed the title ~~[WIP]Follow tag asynchronously~~ Follow tag asynchronously Jan 10, 2019

jywarren requested changes Jan 10, 2019

View reviewed changes

Follow tag asynchronously

10e758b

Added link to manage subscriptions Add precondition failed http error code cleanup codeclimate fix Fix typo change remove data-remote Preserve code for non xhr requests Seperate JS and html Codeclimate fix Code Climate fixes Updated tests

shubhscoder force-pushed the follow_unfollow_tags_asynchronously branch from 93bd48f to 10e758b Compare January 26, 2019 06:51

jywarren requested changes Feb 28, 2019

View reviewed changes

Remove pre-compilation of async.js

de2e0df

jywarren reviewed Mar 1, 2019

View reviewed changes

shubhscoder added 2 commits March 3, 2019 05:41

pre compilation of js assets

463379a

cc fix try

03ff90e

jywarren merged commit 95509de into publiclab:master Mar 5, 2019

shubhscoder mentioned this pull request Mar 11, 2019

Follow unfollow asynchronous requests #4439

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Follow tag asynchronously #4587

Follow tag asynchronously #4587

shubhscoder commented Jan 10, 2019 •

edited

Loading

plotsbot commented Jan 10, 2019 •

edited

Loading

shubhscoder commented Jan 10, 2019

jywarren left a comment

jywarren Jan 10, 2019

shubhscoder Jan 12, 2019

jywarren Jan 10, 2019

shubhscoder Jan 12, 2019

jywarren Jan 10, 2019

shubhscoder Jan 12, 2019

shubhscoder Jan 17, 2019

shubhscoder commented Jan 23, 2019

jywarren left a comment

jywarren Feb 28, 2019

shubhscoder commented Mar 1, 2019

grvsachdeva commented Mar 1, 2019

shubhscoder commented Mar 1, 2019 •

edited

Loading

jywarren Mar 1, 2019

jywarren Mar 1, 2019

jywarren Mar 1, 2019

jywarren commented Mar 5, 2019

shubhscoder commented Mar 5, 2019

jywarren commented Mar 5, 2019

shubhscoder commented Mar 5, 2019

shubhscoder commented Mar 5, 2019

jywarren commented Mar 5, 2019 via email

		@@ -1,4 +1,7 @@
		<div class="col-md-12">
		<div class="alert alert-success" id="follow-success" style="display: none;position: absolute;">

		@@ -1,3 +1,4 @@
		<%= javascript_include_tag('async_tag_subscriptions.js') %>

Follow tag asynchronously #4587

Follow tag asynchronously #4587

Conversation

shubhscoder commented Jan 10, 2019 • edited Loading

plotsbot commented Jan 10, 2019 • edited Loading

shubhscoder commented Jan 10, 2019

jywarren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shubhscoder commented Jan 23, 2019

jywarren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shubhscoder commented Mar 1, 2019

grvsachdeva commented Mar 1, 2019

shubhscoder commented Mar 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jywarren commented Mar 5, 2019

shubhscoder commented Mar 5, 2019

jywarren commented Mar 5, 2019

shubhscoder commented Mar 5, 2019

shubhscoder commented Mar 5, 2019

jywarren commented Mar 5, 2019 via email

shubhscoder commented Jan 10, 2019 •

edited

Loading

plotsbot commented Jan 10, 2019 •

edited

Loading

shubhscoder commented Mar 1, 2019 •

edited

Loading