From f63c7f92894aec3df5db61d370bb888fb6e775b6 Mon Sep 17 00:00:00 2001
From: Dmitry Maksyoma <ledestin@gmail.com>
Date: Sat, 16 May 2020 00:27:46 +1200
Subject: [PATCH] Fix Runkit tags not being activated when comment is added
 (#6767)

* Fix Runkit tags not being activated when comment is added

* Runkit tag activation was ran once, on page load. I've changed it to
  run on on comment preview and submit.
* It was necessary to add a check to skip already activated Runkit
  tags. The code didn't take into account that a tag could be already
  processed, and would just crash.

* Fix Runkit tags and add tests

* Add test for previewing article with Runkit tag

* "Fix CodeClimate not finding waitForRunkitAndActivateTags()

* Refactor a test for readability

* Make Rubocop happy

* Improve test for previewing article with Runkit tag

* Use one method for determining active Runkit tags

* Defer loading of JS from embed.runkit.com

* Add utility function dynamicallyLoadScript(url)

* Switch to dynamic loading of Runkit

* parsed content code block is also hidden now to avoid displaying <code /> block before runkit iframe loads.

* Fix Runkit test

* Remove Runkit script caching in v2 form

* Use <%== instead of .html_safe in v2 form

* Update app/assets/javascripts/utilities/dynamicallyLoadScript.js

Co-authored-by: Nick Taylor <nick@iamdeveloper.com>

Co-authored-by: Nick Taylor <nick@iamdeveloper.com>
---
 .../initializers/initializeCommentPreview.js  |  7 +-
 .../initializeCommentsPage.js.erb             |  1 +
 .../utilities/dynamicallyLoadScript.js        | 10 +++
 app/javascript/article-form/articleForm.jsx   | 46 +++++------
 app/liquid_tags/runkit_tag.rb                 | 80 ++++++++++++++-----
 app/views/articles/_v2_form.html.erb          |  6 +-
 app/views/articles/show.html.erb              |  4 -
 app/views/liquids/_runkit.html.erb            |  2 +-
 .../files/article_with_runkit_tag.txt         | 12 +++
 .../runkit_liquid_tag_spec.approved.html      |  2 +-
 spec/support/runkit_tag_context.rb            | 18 +++++
 .../articles/user_creates_an_article_spec.rb  | 31 +++++++
 .../comments/user_fills_out_comment_spec.rb   | 39 +++++++++
 13 files changed, 197 insertions(+), 61 deletions(-)
 create mode 100644 app/assets/javascripts/utilities/dynamicallyLoadScript.js
 create mode 100644 spec/fixtures/files/article_with_runkit_tag.txt
 create mode 100644 spec/support/runkit_tag_context.rb

diff --git a/app/assets/javascripts/initializers/initializeCommentPreview.js b/app/assets/javascripts/initializers/initializeCommentPreview.js
index f254a351b..c9332a2ff 100644
--- a/app/assets/javascripts/initializers/initializeCommentPreview.js
+++ b/app/assets/javascripts/initializers/initializeCommentPreview.js
@@ -1,4 +1,4 @@
-'use strict';
+/* global activateRunkitTags */
 
 function getAndShowPreview(markdownPreviewPane, markdownEditor) {
   function successCb(body) {
@@ -6,6 +6,7 @@ function getAndShowPreview(markdownPreviewPane, markdownEditor) {
     markdownEditor.classList.toggle('preview-loading');
     markdownEditor.classList.toggle('preview-toggle');
     markdownPreviewPane.innerHTML = body.processed_html; // eslint-disable-line no-param-reassign
+    activateRunkitTags();
   }
 
   const payload = JSON.stringify({
@@ -15,11 +16,11 @@ function getAndShowPreview(markdownPreviewPane, markdownEditor) {
   });
   getCsrfToken()
     .then(sendFetch('comment-preview', payload))
-    .then(response => {
+    .then((response) => {
       return response.json();
     })
     .then(successCb)
-    .catch(err => {
+    .catch((err) => {
       console.log('error!'); // eslint-disable-line
       console.log(err); // eslint-disable-line no-console
     });
diff --git a/app/assets/javascripts/initializers/initializeCommentsPage.js.erb b/app/assets/javascripts/initializers/initializeCommentsPage.js.erb
index 92d26dbfd..62fde8dde 100644
--- a/app/assets/javascripts/initializers/initializeCommentsPage.js.erb
+++ b/app/assets/javascripts/initializers/initializeCommentsPage.js.erb
@@ -247,6 +247,7 @@ function handleCommentSubmit(event) {
           initializeCommentsPage();
           initializeCommentDate();
           initializeCommentDropdown();
+          activateRunkitTags();
         })
       } else {
         response.json().then(function parseError(errorReponse) {
diff --git a/app/assets/javascripts/utilities/dynamicallyLoadScript.js b/app/assets/javascripts/utilities/dynamicallyLoadScript.js
new file mode 100644
index 000000000..5dcbc5fc2
--- /dev/null
+++ b/app/assets/javascripts/utilities/dynamicallyLoadScript.js
@@ -0,0 +1,10 @@
+
+
+function dynamicallyLoadScript(url) {
+  if (document.querySelector(`script[src='${url}']`)) return;
+
+  const script = document.createElement('script');
+  script.src = url;
+
+  document.head.appendChild(script);
+}
diff --git a/app/javascript/article-form/articleForm.jsx b/app/javascript/article-form/articleForm.jsx
index 45716ae92..91f9fd685 100644
--- a/app/javascript/article-form/articleForm.jsx
+++ b/app/javascript/article-form/articleForm.jsx
@@ -21,6 +21,8 @@ import KeyboardShortcutsHandler from './elements/keyboardShortcutsHandler';
 import Tags from '../shared/components/tags';
 import { OrganizationPicker } from '../organization/OrganizationPicker';
 
+/* global activateRunkitTags */
+
 const SetupImageButton = ({
   className,
   imgSrc,
@@ -53,19 +55,7 @@ export default class ArticleForm extends Component {
   }
 
   static handleRunkitPreview() {
-    const targets = document.getElementsByClassName('runkit-element');
-    for (let i = 0; i < targets.length; i += 1) {
-      if (targets[i].children.length > 0) {
-        const preamble = targets[i].children[0].textContent;
-        const content = targets[i].children[1].textContent;
-        targets[i].innerHTML = '';
-        window.RunKit.createNotebook({
-          element: targets[i],
-          source: content,
-          preamble,
-        });
-      }
-    }
+    activateRunkitTags();
   }
 
   static propTypes = {
@@ -173,7 +163,7 @@ export default class ArticleForm extends Component {
     };
   };
 
-  toggleHelp = e => {
+  toggleHelp = (e) => {
     const { helpShowing } = this.state;
     e.preventDefault();
     window.scrollTo(0, 0);
@@ -182,7 +172,7 @@ export default class ArticleForm extends Component {
     });
   };
 
-  fetchPreview = e => {
+  fetchPreview = (e) => {
     const { previewShowing, bodyMarkdown } = this.state;
     e.preventDefault();
     if (previewShowing) {
@@ -194,7 +184,7 @@ export default class ArticleForm extends Component {
     }
   };
 
-  toggleImageManagement = e => {
+  toggleImageManagement = (e) => {
     const { imageManagementShowing } = this.state;
     e.preventDefault();
     window.scrollTo(0, 0);
@@ -205,7 +195,7 @@ export default class ArticleForm extends Component {
     });
   };
 
-  toggleMoreConfig = e => {
+  toggleMoreConfig = (e) => {
     const { moreConfigShowing } = this.state;
     e.preventDefault();
     this.setState({
@@ -213,7 +203,7 @@ export default class ArticleForm extends Component {
     });
   };
 
-  showPreview = response => {
+  showPreview = (response) => {
     if (response.processed_html) {
       this.setState({
         ...this.setCommonProps({ previewShowing: true }),
@@ -228,25 +218,25 @@ export default class ArticleForm extends Component {
     }
   };
 
-  handleOrgIdChange = e => {
+  handleOrgIdChange = (e) => {
     const organizationId = e.target.selectedOptions[0].value;
     this.setState({ organizationId });
   };
 
-  failedPreview = response => {
+  failedPreview = (response) => {
     // TODO: console.log should not be part of production code. Remove it!
     // eslint-disable-next-line no-console
     console.log(response);
   };
 
-  handleConfigChange = e => {
+  handleConfigChange = (e) => {
     e.preventDefault();
     const newState = {};
     newState[e.target.name] = e.target.value;
     this.setState(newState);
   };
 
-  handleMainImageUrlChange = payload => {
+  handleMainImageUrlChange = (payload) => {
     this.setState({
       mainImage: payload.links[0],
       imageManagementShowing: false,
@@ -259,7 +249,7 @@ export default class ArticleForm extends Component {
     window.removeEventListener('beforeunload', this.localStoreContent);
   };
 
-  onPublish = e => {
+  onPublish = (e) => {
     e.preventDefault();
     this.setState({ submitting: true, published: true });
     const { state } = this;
@@ -267,7 +257,7 @@ export default class ArticleForm extends Component {
     submitArticle(state, this.removeLocalStorage, this.handleArticleError);
   };
 
-  onSaveDraft = e => {
+  onSaveDraft = (e) => {
     e.preventDefault();
     this.setState({ submitting: true, published: false });
     const { state } = this;
@@ -275,15 +265,15 @@ export default class ArticleForm extends Component {
     submitArticle(state, this.removeLocalStorage, this.handleArticleError);
   };
 
-  handleTitleKeyDown = e => {
+  handleTitleKeyDown = (e) => {
     if (e.keyCode === 13) {
       e.preventDefault();
     }
   };
 
-  handleBodyKeyDown = _e => {};
+  handleBodyKeyDown = (_e) => {};
 
-  onClearChanges = e => {
+  onClearChanges = (e) => {
     e.preventDefault();
     // eslint-disable-next-line no-alert
     const revert = window.confirm(
@@ -314,7 +304,7 @@ export default class ArticleForm extends Component {
     });
   };
 
-  handleArticleError = response => {
+  handleArticleError = (response) => {
     window.scrollTo(0, 0);
     this.setState({
       errors: response,
diff --git a/app/liquid_tags/runkit_tag.rb b/app/liquid_tags/runkit_tag.rb
index 29e68b164..3df3ffd57 100644
--- a/app/liquid_tags/runkit_tag.rb
+++ b/app/liquid_tags/runkit_tag.rb
@@ -2,32 +2,68 @@ class RunkitTag < Liquid::Block
   PARTIAL = "liquids/runkit".freeze
 
   SCRIPT = <<~JAVASCRIPT.freeze
-    var checkRunkit = setInterval(function() {
-      try {
-        if (typeof(RunKit) !== 'undefined') {
-          var targets = document.getElementsByClassName("runkit-element");
-          for (var i = 0; i < targets.length; i++) {
-            var wrapperContent = targets[i].textContent;
-            if (/^(\<iframe src)/.test(wrapperContent) === false) {
-              if (targets[i].children.length > 0) {
-                var preamble = targets[i].children[0].textContent;
-                var content = targets[i].children[1].textContent;
-                targets[i].innerHTML = "";
-                var notebook = RunKit.createNotebook({
-                  element: targets[i],
-                  source: content,
-                  preamble: preamble
-                });
-              }
-            }
+    function activateRunkitTags() {
+      if (!areAnyRunkitTagsPresent())
+        return
+
+      var checkRunkit = setInterval(function() {
+        try {
+          dynamicallyLoadRunkitLibrary()
+
+          if (typeof(RunKit) === 'undefined') {
+            return
           }
+
+          replaceTagContentsWithRunkitWidget()
+          clearInterval(checkRunkit);
+        } catch(e) {
+          console.error(e);
           clearInterval(checkRunkit);
         }
-      } catch(e) {
-        console.error(e);
-        clearInterval(checkRunkit);
+      }, 200);
+    }
+
+    function isRunkitTagAlreadyActive(runkitTag) {
+      return runkitTag.querySelector("iframe") !== null;
+    };
+
+    function areAnyRunkitTagsPresent() {
+      var presentRunkitTags = document.getElementsByClassName("runkit-element");
+
+      return presentRunkitTags.length > 0
+    }
+
+    function replaceTagContentsWithRunkitWidget() {
+      var targets = document.getElementsByClassName("runkit-element");
+      for (var i = 0; i < targets.length; i++) {
+        if (isRunkitTagAlreadyActive(targets[i])) {
+          continue;
+        }
+
+        var wrapperContent = targets[i].textContent;
+        if (/^(\<iframe src)/.test(wrapperContent) === false) {
+          if (targets[i].children.length > 0) {
+            var preamble = targets[i].children[0].textContent;
+            var content = targets[i].children[1].textContent;
+            targets[i].innerHTML = "";
+            var notebook = RunKit.createNotebook({
+              element: targets[i],
+              source: content,
+              preamble: preamble
+            });
+          }
+        }
       }
-    }, 200);
+    };
+
+    function dynamicallyLoadRunkitLibrary() {
+      if (typeof(dynamicallyLoadScript) === "undefined")
+        return
+
+      dynamicallyLoadScript("//embed.runkit.com")
+    }
+
+    activateRunkitTags();
   JAVASCRIPT
 
   def initialize(tag_name, markup, tokens)
diff --git a/app/views/articles/_v2_form.html.erb b/app/views/articles/_v2_form.html.erb
index ebb14e0e3..551c8e12a 100644
--- a/app/views/articles/_v2_form.html.erb
+++ b/app/views/articles/_v2_form.html.erb
@@ -1,5 +1,3 @@
-<%= javascript_include_tag "https://embed.runkit.com", defer: true %>
-
 <div class="articleformcontainer">
   <div class="articleform"
        id="article-form"
@@ -51,6 +49,10 @@
   </div>
 </div>
 
+<script async>
+  <%== RunkitTag.script %>
+</script>
+
 <style>
   #footer-container {
     display: none
diff --git a/app/views/articles/show.html.erb b/app/views/articles/show.html.erb
index 0b33b336e..d10e6a0f5 100644
--- a/app/views/articles/show.html.erb
+++ b/app/views/articles/show.html.erb
@@ -35,10 +35,6 @@
       <%= @article_json_ld.to_json.html_safe %>
     </script>
   <% end %>
-
-  <% if @article.processed_html.include? "runkit-element" %>
-    <%= javascript_include_tag "https://embed.runkit.com" %>
-  <% end %>
 <% end %>
 
 <% if internal_navigation? %>
diff --git a/app/views/liquids/_runkit.html.erb b/app/views/liquids/_runkit.html.erb
index 7c439a04b..a17af16b7 100644
--- a/app/views/liquids/_runkit.html.erb
+++ b/app/views/liquids/_runkit.html.erb
@@ -2,7 +2,7 @@
   <code style="display: none">
     <%= preamble %>
   </code>
-  <code>
+  <code style="display: none;">
     <%= parsed_content %>
   </code>
 </div>
diff --git a/spec/fixtures/files/article_with_runkit_tag.txt b/spec/fixtures/files/article_with_runkit_tag.txt
new file mode 100644
index 000000000..bc20cc7cb
--- /dev/null
+++ b/spec/fixtures/files/article_with_runkit_tag.txt
@@ -0,0 +1,12 @@
+---
+title: Sample Article
+published: true
+description: this is a sample article
+tags: test
+---
+
+Suspendisse ac lobortis velit, a feugiat sapien. Aenean condimentum, nulla at fermentum sagittis, tellus nisi suscipit velit, vel sollicitudin odio ligula a odio. Integer eget efficitur massa, in sodales velit. Nunc fermentum consequat scelerisque. Morbi elementum tristique faucibus. Nulla vel lectus non justo euismod varius. Vivamus id nisl sit amet odio tincidunt fringilla. Pellentesque odio odio, vulputate in risus eu, lacinia porttitor lorem. Nunc posuere tempus est, imperdiet suscipit odio maximus id. Nam eget feugiat magna.
+
+{% runkit %}
+console.log(42)
+{% endrunkit %}
diff --git a/spec/support/fixtures/approvals/runkit_liquid_tag_spec.approved.html b/spec/support/fixtures/approvals/runkit_liquid_tag_spec.approved.html
index 5bde19443..4ffe12ac3 100644
--- a/spec/support/fixtures/approvals/runkit_liquid_tag_spec.approved.html
+++ b/spec/support/fixtures/approvals/runkit_liquid_tag_spec.approved.html
@@ -6,7 +6,7 @@
     const myVar = 9001
 
   </code>
-  <code>
+  <code style="display: none;">
     // GeoJSON!
 var getJSON = require("async-get-json");
 
diff --git a/spec/support/runkit_tag_context.rb b/spec/support/runkit_tag_context.rb
new file mode 100644
index 000000000..906234ca5
--- /dev/null
+++ b/spec/support/runkit_tag_context.rb
@@ -0,0 +1,18 @@
+shared_context "with runkit_tag" do
+  def compose_runkit_comment(title)
+    <<~COMMENT
+      #{title}
+      {% runkit %}
+      console.log("#{title}")
+      {% endrunkit %}
+    COMMENT
+  end
+
+  def expect_runkit_tag_to_be_active(count: 1)
+    expect(page).to have_css(".runkit-element iframe", count: count)
+  end
+
+  def expect_no_runkit_tag_to_be_active
+    expect_runkit_tag_to_be_active count: 0
+  end
+end
diff --git a/spec/system/articles/user_creates_an_article_spec.rb b/spec/system/articles/user_creates_an_article_spec.rb
index 5ca09c557..d671c61f9 100644
--- a/spec/system/articles/user_creates_an_article_spec.rb
+++ b/spec/system/articles/user_creates_an_article_spec.rb
@@ -1,8 +1,13 @@
 require "rails_helper"
 
 RSpec.describe "Creating an article with the editor", type: :system do
+  include_context "with runkit_tag"
+
   let(:user) { create(:user) }
   let!(:template) { file_fixture("article_published.txt").read }
+  let!(:template_with_runkit_tag) do
+    file_fixture("article_with_runkit_tag.txt").read
+  end
 
   before do
     sign_in user
@@ -14,4 +19,30 @@ RSpec.describe "Creating an article with the editor", type: :system do
     click_button "SAVE CHANGES"
     expect(page).to have_selector("header h1", text: "Sample Article")
   end
+
+  context "with Runkit tag", js: true do
+    it "creates a new article with a Runkit tag" do
+      visit new_path
+      fill_in "article_body_markdown", with: template_with_runkit_tag
+      click_button "SAVE CHANGES"
+
+      expect_runkit_tag_to_be_active
+    end
+
+    it "previews article with a Runkit tag and creates it" do
+      visit new_path
+      fill_in "article_body_markdown", with: template_with_runkit_tag
+      click_button "PREVIEW"
+
+      expect_runkit_tag_to_be_active
+
+      click_button "EDIT"
+
+      expect_no_runkit_tag_to_be_active
+
+      click_button "SAVE CHANGES"
+
+      expect_runkit_tag_to_be_active
+    end
+  end
 end
diff --git a/spec/system/comments/user_fills_out_comment_spec.rb b/spec/system/comments/user_fills_out_comment_spec.rb
index c2c1186a3..e14eaff69 100644
--- a/spec/system/comments/user_fills_out_comment_spec.rb
+++ b/spec/system/comments/user_fills_out_comment_spec.rb
@@ -1,8 +1,13 @@
 require "rails_helper"
 
 RSpec.describe "Creating Comment", type: :system, js: true do
+  include_context "with runkit_tag"
+
   let(:user) { create(:user) }
   let(:raw_comment) { Faker::Lorem.paragraph }
+  let(:runkit_comment) { compose_runkit_comment "comment 1" }
+  let(:runkit_comment2) { compose_runkit_comment "comment 2" }
+
   # the article should be created before signing in
   let!(:article) { create(:article, user_id: user.id, show_comments: true) }
 
@@ -20,6 +25,40 @@ RSpec.describe "Creating Comment", type: :system, js: true do
     expect(page).to have_text(raw_comment)
   end
 
+  context "with Runkit tags" do
+    before do
+      visit article.path.to_s
+
+      wait_for_javascript
+    end
+
+    it "Users fills out comment box with a Runkit tag" do
+      fill_in "text-area", with: runkit_comment
+      click_button("SUBMIT")
+
+      expect_runkit_tag_to_be_active
+    end
+
+    it "Users fills out comment box 2 Runkit tags" do
+      fill_in "text-area", with: runkit_comment
+      click_button("SUBMIT")
+
+      expect_runkit_tag_to_be_active
+
+      fill_in "text-area", with: runkit_comment2
+      click_button("SUBMIT")
+
+      expect_runkit_tag_to_be_active(count: 2)
+    end
+
+    it "User fill out comment box with a Runkit tag, then clicks preview" do
+      fill_in "text-area", with: runkit_comment
+      click_button("PREVIEW")
+
+      expect_runkit_tag_to_be_active
+    end
+  end
+
   it "User fill out comment box then click previews and submit" do
     visit article.path.to_s