From 91232847e31c070d64fc6837999262b130fb71bd Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Mon, 27 Nov 2023 12:38:52 +1000
Subject: [PATCH] FIX: Video placeholders not auto-linking post uploads
 (#24559)

Followup to 2443446e62bff9dd8a0493d10e5359a5e946a84e

We introduced video placeholders which prevent preloading
metadata for videos in posts. The structure looks like this
in HTML when the post is cooked:

```
<div class="video-placeholder-container" data-video-src="http://some-url.com/video.mp4" dir="ltr" style="cursor: pointer;">
  <div class="video-placeholder-wrapper">
    <div class="video-placeholder-overlay">
      <svg class="fa d-icon d-icon-play svg-icon svg-string" xmlns="http://www.w3.org/2000/svg">
        <use href="#play"></use>
      </svg>
    </div>
  </div>
</div>
```

However, we did not update the code that links post uploads
to the post via UploadReference, so any videos uploaded since
this change are essentially dangling and liable to be deleted.
This also causes some uploads to be marked secure when they
shouldn't be, because they are not picked up and analysed in the
CookedPostProcessor flow.
---
 app/models/post.rb       | 12 ++++++++++--
 spec/models/post_spec.rb | 28 +++++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/app/models/post.rb b/app/models/post.rb
index 55521a1c3c0..1c91e8aba90 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -863,7 +863,7 @@ class Post < ActiveRecord::Base
       bypass_bump: bypass_bump,
       cooking_options: self.cooking_options,
       new_post: new_post,
-      post_id: id,
+      post_id: self.id,
       skip_pull_hotlinked_images: skip_pull_hotlinked_images,
     }
 
@@ -1094,7 +1094,15 @@ class Post < ActiveRecord::Base
     ]
 
     fragments ||= Nokogiri::HTML5.fragment(self.cooked)
-    selectors = fragments.css("a/@href", "img/@src", "source/@src", "track/@src", "video/@poster")
+    selectors =
+      fragments.css(
+        "a/@href",
+        "img/@src",
+        "source/@src",
+        "track/@src",
+        "video/@poster",
+        "div/@data-video-src",
+      )
 
     links =
       selectors
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index ba13f8bf0d4..8dfad16f962 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -1628,6 +1628,7 @@ RSpec.describe Post do
 
   describe "uploads" do
     fab!(:video_upload) { Fabricate(:upload, extension: "mp4") }
+    fab!(:video_upload_2) { Fabricate(:upload, extension: "mp4") }
     fab!(:image_upload) { Fabricate(:upload) }
     fab!(:audio_upload) { Fabricate(:upload, extension: "ogg") }
     fab!(:attachment_upload) { Fabricate(:upload, extension: "csv") }
@@ -1636,6 +1637,7 @@ RSpec.describe Post do
 
     let(:base_url) { "#{Discourse.base_url_no_prefix}#{Discourse.base_path}" }
     let(:video_url) { "#{base_url}#{video_upload.url}" }
+    let(:video_2_url) { "#{base_url}#{video_upload_2.url}" }
     let(:audio_url) { "#{base_url}#{audio_upload.url}" }
 
     let(:raw_multiple) { <<~RAW }
@@ -1653,6 +1655,16 @@ RSpec.describe Post do
         <source src="#{audio_url}">
         <a href="#{audio_url}">#{audio_url}</a>
       </audio>
+
+      <div class="video-placeholder-container" data-video-src="#{video_2_url}" dir="ltr" style="cursor: pointer;">
+        <div class="video-placeholder-wrapper">
+          <div class="video-placeholder-overlay">
+            <svg class="fa d-icon d-icon-play svg-icon svg-string" xmlns="http://www.w3.org/2000/svg">
+              <use href="#play"></use>
+            </svg>
+          </div>
+        </div>
+      </div>
       RAW
 
     let(:post) { Fabricate(:post, raw: raw_multiple) }
@@ -1661,7 +1673,7 @@ RSpec.describe Post do
       post.link_post_uploads
 
       post.trash!
-      expect(UploadReference.count).to eq(6)
+      expect(UploadReference.count).to eq(7)
 
       post.destroy!
       expect(UploadReference.count).to eq(0)
@@ -1673,6 +1685,7 @@ RSpec.describe Post do
 
         expect(UploadReference.where(target: post).pluck(:upload_id)).to contain_exactly(
           video_upload.id,
+          video_upload_2.id,
           image_upload.id,
           audio_upload.id,
           attachment_upload.id,
@@ -1901,6 +1914,7 @@ RSpec.describe Post do
       upload5 = Fabricate(:upload)
       upload6 = Fabricate(:video_upload)
       upload7 = Fabricate(:upload, extension: "vtt")
+      upload8 = Fabricate(:video_upload)
 
       set_cdn_url "https://awesome.com/somepath"
 
@@ -1920,6 +1934,16 @@ RSpec.describe Post do
         <source src="#{Discourse.base_url}#{upload6.url}" type="video/mp4" />
         <track src="#{Discourse.base_url}#{upload7.url}" label="English" kind="subtitles" srclang="en" default />
       </video>
+
+      <div class="video-placeholder-container" data-video-src="#{Discourse.base_url}#{upload8.url}" dir="ltr" style="cursor: pointer;">
+        <div class="video-placeholder-wrapper">
+          <div class="video-placeholder-overlay">
+            <svg class="fa d-icon d-icon-play svg-icon svg-string" xmlns="http://www.w3.org/2000/svg">
+              <use href="#play"></use>
+            </svg>
+          </div>
+        </div>
+      </div>
       RAW
 
       urls = []
@@ -1938,6 +1962,7 @@ RSpec.describe Post do
         "#{Discourse.base_url}#{upload5.url}",
         "#{Discourse.base_url}#{upload6.url}",
         "#{Discourse.base_url}#{upload7.url}",
+        "#{Discourse.base_url}#{upload8.url}",
       )
 
       expect(paths).to contain_exactly(
@@ -1948,6 +1973,7 @@ RSpec.describe Post do
         upload5.url,
         upload6.url,
         upload7.url,
+        upload8.url,
       )
     end