discourse/spec/system/composer_uploads_spec.rb

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

249 lines
8.8 KiB
Ruby
Raw Normal View History

FIX: Video thumbnail uploads interfering with subsequent uploads (#23216) Short answer -- the problem is the video thumbnail generator & uploader code added a couple of months back in f144c64e139e41f176ea2ec3433a468fa49b955f. It was implemented as another Mixin which overrides `this._uppyInstance` when uploading the video thumbnail after the initial upload is complete, which means the composer's `this._uppyInstance` value is overridden, and it loses all of its preprocessors & upload code. This is generally a problem with the Mixin based architecture that I used for the Uppy code, which we need to remove at some point and refacotr. The most ideal thing to do here would be to convert this video thumbnail code into an Uppy [postprocessor](https://uppy.io/docs/uppy/#addpostprocessorfn) plugin, which runs on each upload after they are complete. I started looking into this, and the main hurdle here is adding support to tracking the progress of postprocessors to [ExtendableUploader](https://github.com/discourse/discourse/blob/cf42466dea75f1df97d580d2a5e6c221358137a8/app/assets/javascripts/discourse/app/mixins/extendable-uploader.js) so that is out of scope at this time. The fix here makes it so the ComposerVideoThumbnailUppy code is no longer a Mixin, but acts more like a normal class, a pattern which we have used in chat. I also clean up a lot of the thumbnail uploader code and remove some unnecessary things. Attempted to add a system spec, but video streaming does not work in Chrome for Testing at this time, and it is needed for the onloadedmetadata event.
2023-08-24 12:04:26 +08:00
# frozen_string_literal: true
describe "Uploading files in the composer", type: :system do
fab!(:current_user) { Fabricate(:user, refresh_auto_groups: true) }
FIX: Video thumbnail uploads interfering with subsequent uploads (#23216) Short answer -- the problem is the video thumbnail generator & uploader code added a couple of months back in f144c64e139e41f176ea2ec3433a468fa49b955f. It was implemented as another Mixin which overrides `this._uppyInstance` when uploading the video thumbnail after the initial upload is complete, which means the composer's `this._uppyInstance` value is overridden, and it loses all of its preprocessors & upload code. This is generally a problem with the Mixin based architecture that I used for the Uppy code, which we need to remove at some point and refacotr. The most ideal thing to do here would be to convert this video thumbnail code into an Uppy [postprocessor](https://uppy.io/docs/uppy/#addpostprocessorfn) plugin, which runs on each upload after they are complete. I started looking into this, and the main hurdle here is adding support to tracking the progress of postprocessors to [ExtendableUploader](https://github.com/discourse/discourse/blob/cf42466dea75f1df97d580d2a5e6c221358137a8/app/assets/javascripts/discourse/app/mixins/extendable-uploader.js) so that is out of scope at this time. The fix here makes it so the ComposerVideoThumbnailUppy code is no longer a Mixin, but acts more like a normal class, a pattern which we have used in chat. I also clean up a lot of the thumbnail uploader code and remove some unnecessary things. Attempted to add a system spec, but video streaming does not work in Chrome for Testing at this time, and it is needed for the onloadedmetadata event.
2023-08-24 12:04:26 +08:00
let(:modal) { PageObjects::Modals::Base.new }
let(:composer) { PageObjects::Components::Composer.new }
let(:topic) { PageObjects::Pages::Topic.new }
let(:cdp) { PageObjects::CDP.new }
FIX: Video thumbnail uploads interfering with subsequent uploads (#23216) Short answer -- the problem is the video thumbnail generator & uploader code added a couple of months back in f144c64e139e41f176ea2ec3433a468fa49b955f. It was implemented as another Mixin which overrides `this._uppyInstance` when uploading the video thumbnail after the initial upload is complete, which means the composer's `this._uppyInstance` value is overridden, and it loses all of its preprocessors & upload code. This is generally a problem with the Mixin based architecture that I used for the Uppy code, which we need to remove at some point and refacotr. The most ideal thing to do here would be to convert this video thumbnail code into an Uppy [postprocessor](https://uppy.io/docs/uppy/#addpostprocessorfn) plugin, which runs on each upload after they are complete. I started looking into this, and the main hurdle here is adding support to tracking the progress of postprocessors to [ExtendableUploader](https://github.com/discourse/discourse/blob/cf42466dea75f1df97d580d2a5e6c221358137a8/app/assets/javascripts/discourse/app/mixins/extendable-uploader.js) so that is out of scope at this time. The fix here makes it so the ComposerVideoThumbnailUppy code is no longer a Mixin, but acts more like a normal class, a pattern which we have used in chat. I also clean up a lot of the thumbnail uploader code and remove some unnecessary things. Attempted to add a system spec, but video streaming does not work in Chrome for Testing at this time, and it is needed for the onloadedmetadata event.
2023-08-24 12:04:26 +08:00
before { sign_in(current_user) }
it "uploads multiple files at once" do
visit "/new-topic"
expect(composer).to be_opened
file_path_1 = file_from_fixtures("logo.png", "images").path
file_path_2 = file_from_fixtures("logo.jpg", "images").path
attach_file([file_path_1, file_path_2]) { composer.click_toolbar_button("upload") }
expect(composer).to have_no_in_progress_uploads
expect(composer.preview).to have_css(".image-wrapper", count: 2)
end
it "allows cancelling uploads" do
visit "/new-topic"
expect(composer).to be_opened
file_path_1 = file_from_fixtures("huge.jpg", "images").path
cdp.with_slow_upload do
attach_file(file_path_1) { composer.click_toolbar_button("upload") }
expect(composer).to have_in_progress_uploads
find("#cancel-file-upload").click
FIX: Video thumbnail uploads interfering with subsequent uploads (#23216) Short answer -- the problem is the video thumbnail generator & uploader code added a couple of months back in f144c64e139e41f176ea2ec3433a468fa49b955f. It was implemented as another Mixin which overrides `this._uppyInstance` when uploading the video thumbnail after the initial upload is complete, which means the composer's `this._uppyInstance` value is overridden, and it loses all of its preprocessors & upload code. This is generally a problem with the Mixin based architecture that I used for the Uppy code, which we need to remove at some point and refacotr. The most ideal thing to do here would be to convert this video thumbnail code into an Uppy [postprocessor](https://uppy.io/docs/uppy/#addpostprocessorfn) plugin, which runs on each upload after they are complete. I started looking into this, and the main hurdle here is adding support to tracking the progress of postprocessors to [ExtendableUploader](https://github.com/discourse/discourse/blob/cf42466dea75f1df97d580d2a5e6c221358137a8/app/assets/javascripts/discourse/app/mixins/extendable-uploader.js) so that is out of scope at this time. The fix here makes it so the ComposerVideoThumbnailUppy code is no longer a Mixin, but acts more like a normal class, a pattern which we have used in chat. I also clean up a lot of the thumbnail uploader code and remove some unnecessary things. Attempted to add a system spec, but video streaming does not work in Chrome for Testing at this time, and it is needed for the onloadedmetadata event.
2023-08-24 12:04:26 +08:00
expect(composer).to have_no_in_progress_uploads
expect(composer.preview).to have_no_css(".image-wrapper")
end
FIX: Video thumbnail uploads interfering with subsequent uploads (#23216) Short answer -- the problem is the video thumbnail generator & uploader code added a couple of months back in f144c64e139e41f176ea2ec3433a468fa49b955f. It was implemented as another Mixin which overrides `this._uppyInstance` when uploading the video thumbnail after the initial upload is complete, which means the composer's `this._uppyInstance` value is overridden, and it loses all of its preprocessors & upload code. This is generally a problem with the Mixin based architecture that I used for the Uppy code, which we need to remove at some point and refacotr. The most ideal thing to do here would be to convert this video thumbnail code into an Uppy [postprocessor](https://uppy.io/docs/uppy/#addpostprocessorfn) plugin, which runs on each upload after they are complete. I started looking into this, and the main hurdle here is adding support to tracking the progress of postprocessors to [ExtendableUploader](https://github.com/discourse/discourse/blob/cf42466dea75f1df97d580d2a5e6c221358137a8/app/assets/javascripts/discourse/app/mixins/extendable-uploader.js) so that is out of scope at this time. The fix here makes it so the ComposerVideoThumbnailUppy code is no longer a Mixin, but acts more like a normal class, a pattern which we have used in chat. I also clean up a lot of the thumbnail uploader code and remove some unnecessary things. Attempted to add a system spec, but video streaming does not work in Chrome for Testing at this time, and it is needed for the onloadedmetadata event.
2023-08-24 12:04:26 +08:00
end
context "when video thumbnails are enabled" do
before do
SiteSetting.video_thumbnails_enabled = true
SiteSetting.authorized_extensions += "|mp4"
end
it "generates a topic preview thumbnail from the video" do
FIX: Video thumbnail uploads interfering with subsequent uploads (#23216) Short answer -- the problem is the video thumbnail generator & uploader code added a couple of months back in f144c64e139e41f176ea2ec3433a468fa49b955f. It was implemented as another Mixin which overrides `this._uppyInstance` when uploading the video thumbnail after the initial upload is complete, which means the composer's `this._uppyInstance` value is overridden, and it loses all of its preprocessors & upload code. This is generally a problem with the Mixin based architecture that I used for the Uppy code, which we need to remove at some point and refacotr. The most ideal thing to do here would be to convert this video thumbnail code into an Uppy [postprocessor](https://uppy.io/docs/uppy/#addpostprocessorfn) plugin, which runs on each upload after they are complete. I started looking into this, and the main hurdle here is adding support to tracking the progress of postprocessors to [ExtendableUploader](https://github.com/discourse/discourse/blob/cf42466dea75f1df97d580d2a5e6c221358137a8/app/assets/javascripts/discourse/app/mixins/extendable-uploader.js) so that is out of scope at this time. The fix here makes it so the ComposerVideoThumbnailUppy code is no longer a Mixin, but acts more like a normal class, a pattern which we have used in chat. I also clean up a lot of the thumbnail uploader code and remove some unnecessary things. Attempted to add a system spec, but video streaming does not work in Chrome for Testing at this time, and it is needed for the onloadedmetadata event.
2023-08-24 12:04:26 +08:00
visit "/new-topic"
expect(composer).to be_opened
topic.fill_in_composer_title("Video upload test")
file_path_1 = file_from_fixtures("small.mp4", "media").path
attach_file(file_path_1) { composer.click_toolbar_button("upload") }
expect(composer).to have_no_in_progress_uploads
expect(composer.preview).to have_css(".onebox-placeholder-container")
FIX: Video thumbnail uploads interfering with subsequent uploads (#23216) Short answer -- the problem is the video thumbnail generator & uploader code added a couple of months back in f144c64e139e41f176ea2ec3433a468fa49b955f. It was implemented as another Mixin which overrides `this._uppyInstance` when uploading the video thumbnail after the initial upload is complete, which means the composer's `this._uppyInstance` value is overridden, and it loses all of its preprocessors & upload code. This is generally a problem with the Mixin based architecture that I used for the Uppy code, which we need to remove at some point and refacotr. The most ideal thing to do here would be to convert this video thumbnail code into an Uppy [postprocessor](https://uppy.io/docs/uppy/#addpostprocessorfn) plugin, which runs on each upload after they are complete. I started looking into this, and the main hurdle here is adding support to tracking the progress of postprocessors to [ExtendableUploader](https://github.com/discourse/discourse/blob/cf42466dea75f1df97d580d2a5e6c221358137a8/app/assets/javascripts/discourse/app/mixins/extendable-uploader.js) so that is out of scope at this time. The fix here makes it so the ComposerVideoThumbnailUppy code is no longer a Mixin, but acts more like a normal class, a pattern which we have used in chat. I also clean up a lot of the thumbnail uploader code and remove some unnecessary things. Attempted to add a system spec, but video streaming does not work in Chrome for Testing at this time, and it is needed for the onloadedmetadata event.
2023-08-24 12:04:26 +08:00
composer.submit
expect(find("#topic-title")).to have_content("Video upload test")
expect(Topic.last.image_upload_id).to eq(Upload.last.id)
end
it "generates a thumbnail from the video" do
visit "/new-topic"
expect(composer).to be_opened
topic.fill_in_composer_title("Video upload test")
file_path_1 = file_from_fixtures("small.mp4", "media").path
attach_file(file_path_1) { composer.click_toolbar_button("upload") }
expect(composer).to have_no_in_progress_uploads
expect(composer.preview).to have_css(".onebox-placeholder-container")
expect(page).to have_css(
'.onebox-placeholder-container[style*="background-image"]',
wait: Capybara.default_max_wait_time,
)
composer.submit
expect(find("#topic-title")).to have_content("Video upload test")
selector = topic.post_by_number_selector(1)
expect(page).to have_css(selector)
within(selector) do
expect(page).to have_css(".video-placeholder-container[data-thumbnail-src]")
end
FIX: Video thumbnail uploads interfering with subsequent uploads (#23216) Short answer -- the problem is the video thumbnail generator & uploader code added a couple of months back in f144c64e139e41f176ea2ec3433a468fa49b955f. It was implemented as another Mixin which overrides `this._uppyInstance` when uploading the video thumbnail after the initial upload is complete, which means the composer's `this._uppyInstance` value is overridden, and it loses all of its preprocessors & upload code. This is generally a problem with the Mixin based architecture that I used for the Uppy code, which we need to remove at some point and refacotr. The most ideal thing to do here would be to convert this video thumbnail code into an Uppy [postprocessor](https://uppy.io/docs/uppy/#addpostprocessorfn) plugin, which runs on each upload after they are complete. I started looking into this, and the main hurdle here is adding support to tracking the progress of postprocessors to [ExtendableUploader](https://github.com/discourse/discourse/blob/cf42466dea75f1df97d580d2a5e6c221358137a8/app/assets/javascripts/discourse/app/mixins/extendable-uploader.js) so that is out of scope at this time. The fix here makes it so the ComposerVideoThumbnailUppy code is no longer a Mixin, but acts more like a normal class, a pattern which we have used in chat. I also clean up a lot of the thumbnail uploader code and remove some unnecessary things. Attempted to add a system spec, but video streaming does not work in Chrome for Testing at this time, and it is needed for the onloadedmetadata event.
2023-08-24 12:04:26 +08:00
end
it "handles a video where dimensions can't be read gracefully" do
visit "/new-topic"
expect(composer).to be_opened
topic.fill_in_composer_title("Zero Width Video Test")
# Inject JavaScript to mock video dimensions
page.execute_script <<-JS
HTMLVideoElement.prototype.__defineGetter__('videoWidth', function() { return 0; });
HTMLVideoElement.prototype.__defineGetter__('videoHeight', function() { return 0; });
JS
file_path_1 = file_from_fixtures("small.mp4", "media").path
attach_file(file_path_1) { composer.click_toolbar_button("upload") }
expect(composer).to have_no_in_progress_uploads
expect(composer.preview).to have_css(".onebox-placeholder-container")
composer.submit
expect(find("#topic-title")).to have_content("Zero Width Video Test")
selector = topic.post_by_number_selector(1)
expect(page).to have_css(selector)
within(selector) do
expect(page).to have_no_css(".video-placeholder-container[data-thumbnail-src]")
end
end
it "handles a video load error gracefully" do
visit "/new-topic"
expect(composer).to be_opened
topic.fill_in_composer_title("Video Load Error Test")
# Inject JavaScript to simulate an invalid video file that triggers onerror
page.execute_script <<-JS
const originalCreateObjectURL = URL.createObjectURL;
URL.createObjectURL = function(blob) {
// Simulate an invalid video source by returning a fake object URL
return 'invalid_video_source.mp4';
};
JS
file_path_1 = file_from_fixtures("small.mp4", "media").path
attach_file(file_path_1) { composer.click_toolbar_button("upload") }
expect(composer).to have_no_in_progress_uploads
expect(composer.preview).to have_css(".onebox-placeholder-container")
end
it "shows video player in composer" do
SiteSetting.enable_diffhtml_preview = true
visit "/new-topic"
expect(composer).to be_opened
topic.fill_in_composer_title("Video upload test")
file_path_1 = file_from_fixtures("small.mp4", "media").path
attach_file(file_path_1) { composer.click_toolbar_button("upload") }
expect(composer).to have_no_in_progress_uploads
expect(composer.preview).to have_css(".video-container video")
expect(page).to have_css(
".video-container video source[src]",
visible: false,
wait: Capybara.default_max_wait_time,
)
end
FIX: Video thumbnail uploads interfering with subsequent uploads (#23216) Short answer -- the problem is the video thumbnail generator & uploader code added a couple of months back in f144c64e139e41f176ea2ec3433a468fa49b955f. It was implemented as another Mixin which overrides `this._uppyInstance` when uploading the video thumbnail after the initial upload is complete, which means the composer's `this._uppyInstance` value is overridden, and it loses all of its preprocessors & upload code. This is generally a problem with the Mixin based architecture that I used for the Uppy code, which we need to remove at some point and refacotr. The most ideal thing to do here would be to convert this video thumbnail code into an Uppy [postprocessor](https://uppy.io/docs/uppy/#addpostprocessorfn) plugin, which runs on each upload after they are complete. I started looking into this, and the main hurdle here is adding support to tracking the progress of postprocessors to [ExtendableUploader](https://github.com/discourse/discourse/blob/cf42466dea75f1df97d580d2a5e6c221358137a8/app/assets/javascripts/discourse/app/mixins/extendable-uploader.js) so that is out of scope at this time. The fix here makes it so the ComposerVideoThumbnailUppy code is no longer a Mixin, but acts more like a normal class, a pattern which we have used in chat. I also clean up a lot of the thumbnail uploader code and remove some unnecessary things. Attempted to add a system spec, but video streaming does not work in Chrome for Testing at this time, and it is needed for the onloadedmetadata event.
2023-08-24 12:04:26 +08:00
end
context "when multiple images are uploaded" do
before { SiteSetting.experimental_auto_grid_images = true }
it "automatically wraps images in [grid] tags on 3 or more images" do
visit "/new-topic"
expect(composer).to be_opened
file_path_1 = file_from_fixtures("logo.png", "images").path
file_path_2 = file_from_fixtures("logo.jpg", "images").path
file_path_3 = file_from_fixtures("downsized.png", "images").path
attach_file([file_path_1, file_path_2, file_path_3]) do
composer.click_toolbar_button("upload")
end
expect(composer).to have_no_in_progress_uploads
expect(composer.composer_input.value).to match(
%r{\[grid\].*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*?\[/grid\]}m,
)
end
it "does not wrap [grid] tags on less than 3 images" do
visit "/new-topic"
expect(composer).to be_opened
file_path_1 = file_from_fixtures("logo.png", "images").path
file_path_2 = file_from_fixtures("logo.jpg", "images").path
attach_file([file_path_1, file_path_2]) { composer.click_toolbar_button("upload") }
expect(composer).to have_no_in_progress_uploads
expect(composer.composer_input.value).to match(
%r{!\[.*?\]\(upload://.*?\).*?!\[.*?\]\(upload://.*?\)}m,
)
end
it "automatically wraps images in [grid] tags even after clearing previous uploads" do
visit "/new-topic"
expect(composer).to be_opened
file_path_1 = file_from_fixtures("logo.png", "images").path
file_path_2 = file_from_fixtures("logo.jpg", "images").path
file_path_3 = file_from_fixtures("downsized.png", "images").path
file_path_4 = file_from_fixtures("logo-dev.png", "images").path
file_path_5 = file_from_fixtures("large_icon_correct.png", "images").path
file_path_6 = file_from_fixtures("large_icon_incorrect.png", "images").path
attach_file([file_path_1, file_path_2, file_path_3]) do
composer.click_toolbar_button("upload")
end
expect(composer).to have_no_in_progress_uploads
expect(composer.composer_input.value).to match(
%r{\[grid\].*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*?\[/grid\]}m,
)
composer.clear_content
attach_file([file_path_4, file_path_5, file_path_6]) do
composer.click_toolbar_button("upload")
end
expect(composer).to have_no_in_progress_uploads
expect(composer.composer_input.value).to match(
%r{\[grid\].*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*?\[/grid\]}m,
)
end
it "does not automatically wrap images in [grid] tags when setting is disabled" do
SiteSetting.experimental_auto_grid_images = false
visit "/new-topic"
expect(composer).to be_opened
file_path_1 = file_from_fixtures("logo.png", "images").path
file_path_2 = file_from_fixtures("logo.jpg", "images").path
file_path_3 = file_from_fixtures("downsized.png", "images").path
attach_file([file_path_1, file_path_2, file_path_3]) do
composer.click_toolbar_button("upload")
end
expect(composer).to have_no_in_progress_uploads
expect(composer.composer_input.value).to match(
%r{!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\).*!\[.*?\]\(upload://.*?\)}m,
)
end
end
FIX: Video thumbnail uploads interfering with subsequent uploads (#23216) Short answer -- the problem is the video thumbnail generator & uploader code added a couple of months back in f144c64e139e41f176ea2ec3433a468fa49b955f. It was implemented as another Mixin which overrides `this._uppyInstance` when uploading the video thumbnail after the initial upload is complete, which means the composer's `this._uppyInstance` value is overridden, and it loses all of its preprocessors & upload code. This is generally a problem with the Mixin based architecture that I used for the Uppy code, which we need to remove at some point and refacotr. The most ideal thing to do here would be to convert this video thumbnail code into an Uppy [postprocessor](https://uppy.io/docs/uppy/#addpostprocessorfn) plugin, which runs on each upload after they are complete. I started looking into this, and the main hurdle here is adding support to tracking the progress of postprocessors to [ExtendableUploader](https://github.com/discourse/discourse/blob/cf42466dea75f1df97d580d2a5e6c221358137a8/app/assets/javascripts/discourse/app/mixins/extendable-uploader.js) so that is out of scope at this time. The fix here makes it so the ComposerVideoThumbnailUppy code is no longer a Mixin, but acts more like a normal class, a pattern which we have used in chat. I also clean up a lot of the thumbnail uploader code and remove some unnecessary things. Attempted to add a system spec, but video streaming does not work in Chrome for Testing at this time, and it is needed for the onloadedmetadata event.
2023-08-24 12:04:26 +08:00
end