discourse/spec/system/composer_uploads_spec.rb

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

74 lines
2.3 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
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
fab!(:current_user) { Fabricate(:user) }
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
sign_in(current_user)
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
sign_in(current_user)
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
# TODO (martin): Video streaming is not yet available in Chrome for Testing,
# we need to come back to this in a few months and try again.
#
# c.f. https://groups.google.com/g/chromedriver-users/c/1SMbByMfO2U
xit "generates a thumbnail for the video" do
sign_in(current_user)
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")
composer.submit
expect(find("#topic-title")).to have_content("Video upload test")
expect(topic.image_upload_id).to eq(Upload.last.id)
end
end
end