Fix YouTube tag (forem#649)

* Fix YouTube tag

* Remove solargraph from VS Code recommendations

* Update editor guide for more clarity

.vscode/extensions.json

@@ -5,7 +5,6 @@
     "donjayamanne.git-extension-pack",
     "msjsdiag.debugger-for-chrome",
     "rebornix.ruby",
-    "castwide.solargraph",
     "MS-vsliveshare.vsliveshare-pack"
   ]
 }

app/liquid_tags/youtube_tag.rb

@@ -21,31 +21,25 @@ def render(_context)
   private
 
   def parse_id(input)
-    input = translate_url(input) if input.include?("watch?v=")
-    input = translate_start_time(input) if input.include?("?t=")
     input_no_space = input.delete(" ")
-    if valid_id?(input_no_space)
-      input_no_space
-    else
-      raise StandardError, "Invalid Youtube Id"
-    end
-  end
-
-  def translate_url(input)
-    input.split("watch?v=")[1].split("\"")[0]
+    raise StandardError, "Invalid YouTube ID" unless valid_id?(input_no_space)
+    return translate_start_time(input_no_space) if input_no_space.include?("?t=")
+    input_no_space
   end
 
   def translate_start_time(id)
     time = id.split("?t=")[-1]
-    if /(\d+)m(\d+)s/.match?(time)
-      time = time.scan(/(\d+)m(\d+)s/)[0].map(&:to_i)
-      seconds = time[1] + (time[0] * 60)
-      "#{id.split('?t=')[0]}?start=#{seconds}"
-    end
+    time_hash = {
+      h: time.scan(/\d+h/)[0]&.delete("h").to_i,
+      m: time.scan(/\d+m/)[0]&.delete("m").to_i,
+      s: time.scan(/\d+s/)[0]&.delete("s").to_i
+    }
+    time_in_seconds = (time_hash[:h] * 3600) + (time_hash[:m] * 60) + time_hash[:s]
+    "#{id.split('?t=')[0]}?start=#{time_in_seconds}"
   end
 
   def valid_id?(id)
-    id =~ /[a-zA-Z0-9_-]{11}(\?start\=\d*)?/
+    id =~ /\A[a-zA-Z0-9_-]{11}((\?t\=)?(\d{1}h)?(\d{1,2}m)?(\d{1,2}s)?){5,11}?\Z/
   end
 end
 

app/views/pages/_editor_guide_text.html.erb

@@ -87,12 +87,16 @@
 
     <h3><strong>GitHub Gist Embed</strong></h3>
     <p>All you need is the gist link:</p>
-    <code>{% gist https://gist.github.com/QuincyLarson/4bb1682ce590dc42402b2edddbca7aaa %}</code>
+    <pre>
+    {% gist https://gist.github.com/QuincyLarson/4bb1682ce590dc42402b2edddbca7aaa %}
+    </pre>
 
     <h3><strong>Video Embed</strong></h3>
     <p>All you need is the <code>id</code> from the URL.
-    <code>{% youtube dQw4w9WgXcQ %}</code>
-    <code>{% vimeo 193110695 %}</code><br />
+    <ul>
+      <li><strong>YouTube:</strong> <code>{% youtube dQw4w9WgXcQ %}</code></li>
+      <li><strong>Vimeo:</strong> <code>{% vimeo 193110695 %}</code></li>
+    </ul>
 
     <h3><strong>CodePen Embed</strong></h3>
     <p>All you need is the full CodePen <code>link</code>, ending in the pen ID code, as follows:</p>

spec/liquid_tags/youtube_tag_spec.rb

@@ -2,7 +2,33 @@
 
 RSpec.describe YoutubeTag, type: :liquid_template do
   describe "#id" do
-    let(:youtube_id) { "dQw4w9WgXcQ" }
+    let(:valid_id_no_time) { "dQw4w9WgXcQ" }
+
+    let(:valid_ids_with_time) do
+      %w(
+        QASbw8_0meM?t=8h12m26s
+        QASbw8_0meM?t=6h34m
+        QASbw8_0meM?t=7h
+        QASbw8_0meM?t=1h57s
+        dQw4w9WgXcQ?t=4m45s
+        dQw4w9WgXcQ?t=5m
+        dQw4w9WgXcQ?t=8s
+      )
+    end
+
+    let(:invalid_id) { Faker::Lorem.characters(rand(12..100)) }
+
+    def parsed_id(id)
+      return id unless id.include?("?t=")
+      id_array = id.split("?t=")
+      time_hash = {
+        h: id_array[1].scan(/\d+h/)[0]&.delete("h").to_i,
+        m: id_array[1].scan(/\d+m/)[0]&.delete("m").to_i,
+        s: id_array[1].scan(/\d+s/)[0]&.delete("s").to_i
+      }
+      time_string = ((time_hash[:h] * 3600) + (time_hash[:m] * 60) + time_hash[:s]).to_s
+      "#{id_array[0]}?start=#{time_string}"
+    end
 
     def generate_new_liquid(id)
       Liquid::Template.register_tag("youtube", YoutubeTag)
@@ -13,24 +39,37 @@ def generate_iframe(id)
       "<iframe "\
         "width=\"710\" "\
         "height=\"399\" "\
-        "src=\"https://www.youtube.com/embed/#{id}\" "\
+        "src=\"https://www.youtube.com/embed/#{parsed_id(id)}\" "\
         "allowfullscreen> "\
       "</iframe>"
     end
 
-    it "accepts youtube video id" do
-      liquid = generate_new_liquid(youtube_id)
-      expect(liquid.render).to eq(generate_iframe(youtube_id))
+    it "accepts a valid YouTube ID with no starting time" do
+      liquid = generate_new_liquid(valid_id_no_time)
+      expect(liquid.render).to eq(generate_iframe(valid_id_no_time))
+    end
+
+    it "accepts valid YouTube IDs with starting times" do
+      valid_ids_with_time.each do |id|
+        generated_liquid = generate_new_liquid(id)
+        expect(generated_liquid.render).to eq generate_iframe(id)
+      end
+    end
+
+    it "accepts YouTube ID with no start time and an empty space" do
+      liquid = generate_new_liquid(valid_id_no_time + " ")
+      expect(liquid.render).to eq(generate_iframe(valid_id_no_time))
     end
 
-    it "accepts youtube video id with empty space" do
-      liquid = generate_new_liquid(youtube_id + " ")
-      expect(liquid.render).to eq(generate_iframe(youtube_id))
+    it "accepts YouTube IDs with start times and one empty space" do
+      valid_ids_with_time.each do |id|
+        generated_liquid = generate_new_liquid(id + " ")
+        expect(generated_liquid.render).to eq generate_iframe(id)
+      end
     end
 
-    it "accepts youtube video id with start time" do
-      liquid = generate_new_liquid(youtube_id + "?t=1m7s")
-      expect(liquid.render).to eq(generate_iframe(youtube_id + "?start=67"))
+    it "raises an error for invalid IDs" do
+      expect { generate_new_liquid(invalid_id).render }.to raise_error("Invalid YouTube ID")
     end
   end
 end