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

@uppy/transloadit: close assembly if upload is cancelled #3591

Merged
merged 4 commits into from
Mar 24, 2022

Conversation

aduh95
Copy link
Member

@aduh95 aduh95 commented Mar 22, 2022

Fixes: #3156

Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but sounds like a good use case for a test?

@aduh95
Copy link
Member Author

aduh95 commented Mar 22, 2022

good use case for a test?

You're right it is 🥲

@aduh95 aduh95 added the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label Mar 24, 2022
@aduh95 aduh95 requested a review from Murderlon March 24, 2022 11:10
@github-actions github-actions bot removed pending end-to-end tests safe to test Add this label on trustworthy PRs to spawn the e2e test suite labels Mar 24, 2022
@aduh95 aduh95 added the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label Mar 24, 2022
@github-actions github-actions bot removed the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label Mar 24, 2022
@aduh95 aduh95 added the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label Mar 24, 2022
@Murderlon
Copy link
Member

Murderlon commented Mar 24, 2022

Final touches:

  1. We should add this command:
Cypress.Commands.add('getBySel', (selector, ...args) => {
  return cy.get(`[data-test=${selector}]`, ...args)
})
  1. Also add data-cy for upload/retry/pause buttons
  2. Go through the tests and replace all button queries with new command / query

@aduh95
Copy link
Member Author

aduh95 commented Mar 24, 2022

  1. We should add this command:

I disagree, I find the cy.get([data-cy=…]) much more readable than cy.getBySel(…).

  • Also add data-cy for upload/retry/pause buttons
  • Go through the tests and replace all button queries with new command / query

I think it's out of the scope of this PR, and it shouldn't hold back the release.

@Murderlon
Copy link
Member

I think it's out of the scope of this PR, and it shouldn't hold back the release.

Well it has to be done sooner or later in my opinion. We can walk into it again in another PR for which it's also out of scope, or we remember to follow up on it. Either way is fine.

@github-actions github-actions bot removed pending end-to-end tests safe to test Add this label on trustworthy PRs to spawn the e2e test suite labels Mar 24, 2022
@aduh95 aduh95 merged commit aba7b29 into transloadit:main Mar 24, 2022
@aduh95 aduh95 deleted the fix-assembly-polling-when-cancelled branch March 24, 2022 12:57
@github-actions github-actions bot mentioned this pull request Mar 24, 2022
github-actions bot added a commit that referenced this pull request Mar 24, 2022
| Package           | Version | Package           | Version |
| ----------------- | ------- | ----------------- | ------- |
| @uppy/companion   |   3.5.0 | @uppy/webcam      |   2.1.0 |
| @uppy/status-bar  |   2.1.3 | @uppy/robodog     |   2.5.0 |
| @uppy/transloadit |   2.1.2 | uppy              |   2.9.0 |
| @uppy/tus         |   2.2.1 |                   |         |

- @uppy/transloadit: close assembly if upload is cancelled (Antoine du Hamel / #3591)
- @uppy/companion: Companion server upload events (Mikael Finstad / #3544)
- @uppy/tus: fix double requests sent when rate limiting (Antoine du Hamel / #3595)
- website: fix linter error on blog post (Antoine du Hamel / #3596)
- @uppy/companion: fix `yarn test` command (Antoine du Hamel / #3590)
- @uppy/webcam: Mime types in webcam options type (Sobakin Sviatoslav / #3593)
- website: Some polish and a better (?) intro for the recent update post (AJvanLoon / #3588)
- @uppy/companion: Allow setting no ACL (Mikael Finstad / #3577)
- @uppy/companion: Small companion code and doc changes (Mikael Finstad / #3586)
- @uppy/robodog: fix CDN bundle (Antoine du Hamel / #3587)
- website: Fix broken link (YukeshShr / #3581)
@aduh95
Copy link
Member Author

aduh95 commented Mar 24, 2022

2. Also add data-cy for upload/retry/pause buttons

It looks like the upload button already has a specific CSS class associated with it, is there really useful to add data-cy on top of it?
AFAICS, we have no tests that target retry or pause buttons, let's add some.

HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
| Package           | Version | Package           | Version |
| ----------------- | ------- | ----------------- | ------- |
| @uppy/companion   |   3.5.0 | @uppy/webcam      |   2.1.0 |
| @uppy/status-bar  |   2.1.3 | @uppy/robodog     |   2.5.0 |
| @uppy/transloadit |   2.1.2 | uppy              |   2.9.0 |
| @uppy/tus         |   2.2.1 |                   |         |

- @uppy/transloadit: close assembly if upload is cancelled (Antoine du Hamel / transloadit#3591)
- @uppy/companion: Companion server upload events (Mikael Finstad / transloadit#3544)
- @uppy/tus: fix double requests sent when rate limiting (Antoine du Hamel / transloadit#3595)
- website: fix linter error on blog post (Antoine du Hamel / transloadit#3596)
- @uppy/companion: fix `yarn test` command (Antoine du Hamel / transloadit#3590)
- @uppy/webcam: Mime types in webcam options type (Sobakin Sviatoslav / transloadit#3593)
- website: Some polish and a better (?) intro for the recent update post (AJvanLoon / transloadit#3588)
- @uppy/companion: Allow setting no ACL (Mikael Finstad / transloadit#3577)
- @uppy/companion: Small companion code and doc changes (Mikael Finstad / transloadit#3586)
- @uppy/robodog: fix CDN bundle (Antoine du Hamel / transloadit#3587)
- website: Fix broken link (YukeshShr / transloadit#3581)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Canceled/failed upload to transloadit keeps polling assembly
2 participants