Skip to content

Commit

Permalink
core validateRestrictions: return error directly vs the result/reason…
Browse files Browse the repository at this point in the history
… obj (#3951)

* core validateRestrictions: return error directly vs the result/reason obj

* Refacrtor to actually use restrictionError instead of {reason, message}

* Return error instead of throwing
  • Loading branch information
arturi committed Aug 8, 2022
1 parent a5c73c3 commit 346c2fc
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 26 deletions.
6 changes: 2 additions & 4 deletions packages/@uppy/core/src/Uppy.js
Expand Up @@ -390,14 +390,12 @@ class Uppy {
}

validateRestrictions (file, files = this.getFiles()) {
// TODO: directly return the Restriction error in next major version.
// we create RestrictionError's just to discard immediately, which doesn't make sense.
try {
this.#restricter.validate(file, files)
return { result: true }
} catch (err) {
return { result: false, reason: err.message }
return err
}
return null
}

#checkRequiredMetaFieldsOnFile (file) {
Expand Down
14 changes: 2 additions & 12 deletions packages/@uppy/core/src/Uppy.test.js
Expand Up @@ -1804,18 +1804,8 @@ describe('src/Core', () => {
const validateRestrictions1 = core.validateRestrictions(newFile)
const validateRestrictions2 = core2.validateRestrictions(newFile)

expect(validateRestrictions1).toMatchObject(
{
result: false,
reason: 'This file is smaller than the allowed size of 293 KB',
},
)
expect(validateRestrictions2).toMatchObject(
{
result: false,
reason: 'You can only upload: image/png',
},
)
expect(validateRestrictions1.message).toEqual('This file is smaller than the allowed size of 293 KB')
expect(validateRestrictions2.message).toEqual('You can only upload: image/png')
})

it('should emit `restriction-failed` event when some rule is violated', () => {
Expand Down
6 changes: 3 additions & 3 deletions packages/@uppy/provider-views/src/Browser.jsx
Expand Up @@ -101,7 +101,7 @@ function Browser (props) {
})}

{files.map((file) => {
const validated = validateRestrictions(
const restrictionError = validateRestrictions(
remoteFileObjToLocal(file),
[...uppyFiles, ...currentSelection],
)
Expand All @@ -119,8 +119,8 @@ function Browser (props) {
viewType,
i18n,
type: 'file',
isDisabled: !validated.result && !isChecked(file),
restrictionReason: validated.reason,
isDisabled: restrictionError && !isChecked(file),
restrictionError,
})
})}
</ul>
Expand Down
4 changes: 2 additions & 2 deletions packages/@uppy/provider-views/src/Item/components/GridLi.jsx
Expand Up @@ -4,7 +4,7 @@ function GridListItem (props) {
const {
className,
isDisabled,
restrictionReason,
restrictionError,
isChecked,
title,
itemIconEl,
Expand All @@ -18,7 +18,7 @@ function GridListItem (props) {
return (
<li
className={className}
title={isDisabled ? restrictionReason : null}
title={isDisabled ? restrictionError?.message : null}
>
<input
type="checkbox"
Expand Down
4 changes: 2 additions & 2 deletions packages/@uppy/provider-views/src/Item/components/ListLi.jsx
Expand Up @@ -11,7 +11,7 @@ function ListItem (props) {
const {
className,
isDisabled,
restrictionReason,
restrictionError,
isCheckboxDisabled,
isChecked,
toggleCheckbox,
Expand All @@ -28,7 +28,7 @@ function ListItem (props) {
return (
<li
className={className}
title={isDisabled ? restrictionReason : null}
title={isDisabled ? restrictionError?.message : null}
>
{!isCheckboxDisabled ? (
<input
Expand Down
7 changes: 4 additions & 3 deletions packages/@uppy/provider-views/src/SharedHandler.js
Expand Up @@ -51,14 +51,15 @@ export default class SharedHandler {
// reduce it to only contain files that pass restrictions
for (const item of currentSelection) {
const { uppy } = this.plugin
const validatedRestrictions = uppy.validateRestrictions(
const restrictionError = uppy.validateRestrictions(
remoteFileObjToLocal(item),
[...uppy.getFiles(), ...reducedCurrentSelection],
)
if (validatedRestrictions.result) {

if (!restrictionError) {
reducedCurrentSelection.push(item)
} else {
uppy.info({ message: validatedRestrictions.reason }, 'error', uppy.opts.infoTimeout)
uppy.info({ message: restrictionError.message }, 'error', uppy.opts.infoTimeout)
}
}
this.plugin.setPluginState({ currentSelection: reducedCurrentSelection })
Expand Down

0 comments on commit 346c2fc

Please sign in to comment.