Skip to content

Commit

Permalink
Merge pull request #1274 from creative-commoners/pulls/1.10/fix-wrong…
Browse files Browse the repository at this point in the history
…-message

FIX Show correct error message instead of successful message
  • Loading branch information
GuySartorelli authored Jun 9, 2022
2 parents 4e053aa + fb299b3 commit c692b7f
Show file tree
Hide file tree
Showing 11 changed files with 180 additions and 3 deletions.
2 changes: 1 addition & 1 deletion client/dist/js/bundle.js

Large diffs are not rendered by default.

15 changes: 15 additions & 0 deletions client/src/components/AssetDropzone/AssetDropzone.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class AssetDropzone extends Component {
this.handleDragLeave = this.handleDragLeave.bind(this);
this.handleDrop = this.handleDrop.bind(this);
this.handleUploadProgress = this.handleUploadProgress.bind(this);
this.handleUploadComplete = this.handleUploadComplete.bind(this);
this.handleError = this.handleError.bind(this);
this.handleSending = this.handleSending.bind(this);
this.handleSuccess = this.handleSuccess.bind(this);
Expand Down Expand Up @@ -123,6 +124,9 @@ class AssetDropzone extends Component {
// Whenever the file upload progress changes
uploadprogress: this.handleUploadProgress,

// When the file upload complete
complete: this.handleUploadComplete,

// The text used before any files are dropped
dictDefaultMessage: i18n._t('AssetAdmin.DROPZONE_DEFAULT_MESSAGE', 'Drop files here to upload'),

Expand Down Expand Up @@ -301,6 +305,17 @@ class AssetDropzone extends Component {
}
}

/**
* Event handler when a file's upload complete.
*
* @param {object} file - File interface. See https://developer.mozilla.org/en-US/docs/Web/API/File
*/
handleUploadComplete(file) {
if (typeof this.props.onUploadComplete === 'function') {
this.props.onUploadComplete(file.status);
}
}

/**
* Event handler triggered when the user drops a file on the dropzone.
*
Expand Down
30 changes: 28 additions & 2 deletions client/src/components/PreviewImageField/PreviewImageField.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class PreviewImageField extends Component {
this.handleSuccessfulUpload = this.handleSuccessfulUpload.bind(this);
this.handleSending = this.handleSending.bind(this);
this.handleUploadProgress = this.handleUploadProgress.bind(this);
this.handleUploadComplete = this.handleUploadComplete.bind(this);
this.handleCancelUpload = this.handleCancelUpload.bind(this);
this.handleRemoveErroredUpload = this.handleRemoveErroredUpload.bind(this);
this.canFileUpload = this.canFileUpload.bind(this);
Expand Down Expand Up @@ -81,6 +82,7 @@ class PreviewImageField extends Component {
onSuccess: this.handleSuccessfulUpload,
onSending: this.handleSending,
onUploadProgress: this.handleUploadProgress,
onUploadComplete: this.handleUploadComplete,
canFileUpload: this.canFileUpload,
updateFormData: this.updateFormData,
};
Expand Down Expand Up @@ -222,6 +224,15 @@ class PreviewImageField extends Component {
this.props.actions.previewField.updateFile(this.props.id, { progress });
}

/**
* Upload was complete, set status changes to reflect it
*
* @param {object} file
*/
handleUploadComplete(status) {
this.props.actions.previewField.updateStatus(this.props.id, { status });
}

/**
* Build the preview URL
* @param {string} category
Expand Down Expand Up @@ -277,6 +288,8 @@ class PreviewImageField extends Component {
}

const { category, progress, message } = upload;
const errors = upload.errors ? upload.errors[0] : null;
const status = upload.status ? upload.status : null;
const preview = this.preview(category, upload, data);
const image = <img alt="preview" src={preview} className="editor__thumbnail" />;
const linkedImage = (data.url && !progress) ? (
Expand All @@ -296,13 +309,25 @@ class PreviewImageField extends Component {
) : null;
let messageBox = null;

if (message) {
if (errors || status === 'error') {
const errorMessage = errors && errors.value
? errors.value
: i18n._t('AssetAdmin.DROPZONE_RESPONSE_ERROR', 'Server responded with an error.');

const errorType = errors && errors.type ? errors.type : 'error';

messageBox = (
<div className={`preview-image-field__message preview-image-field__message--${errorType}`}>
{errorMessage}
</div>
);
} else if (message) {
messageBox = (
<div className={`preview-image-field__message preview-image-field__message--${message.type}`}>
{message.value}
</div>
);
} else if (progress === 100) {
} else if (progress === 100 && status === 'success') {
messageBox = (
<div className="preview-image-field__message preview-image-field__message--success">
{i18n._t(
Expand Down Expand Up @@ -390,6 +415,7 @@ PreviewImageField.propTypes = {
type: PropTypes.string.isRequired,
value: PropTypes.string.isRequired,
}),
status: PropTypes.string,
}),
actions: PropTypes.object,
securityID: PropTypes.string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,4 +208,68 @@ describe('PreviewImageField', () => {
expect(url).toBe('/logo.jpg?vid=456');
});
});

describe('handleUploadComplete()', () => {
it.each(
[
{
responseStatus: 'success',
upload: {
category: 'image',
extension: 'png',
filename: 'test.png',
},
},
{
responseStatus: 'error',
upload: {
category: 'text',
extension: 'txt',
filename: 'test.txt',
errors: [
{
code: 400,
type: 'error',
value: 'Filesize is too large, maximum 100 KB allowed',
}
]
},
message: 'Filesize is too large, maximum 100 KB allowed',
},
]
)('should wait before upload complete', ({ responseStatus, upload, message }) => {
props.actions = {
previewField: {
updateStatus: jest.fn((id, { status }) => {
props.upload.status = status;
props.id = id;
})
},
};
props.upload = upload;
props.upload.progress = 100;

const item = ReactTestUtils.renderIntoDocument(
<PreviewImageField {...props} />
);

item.handleUploadComplete(responseStatus);

if (responseStatus === 'error') {
expect(props.id).toBe('Form_Test_Field');
expect(props.upload.progress).toBe(100);
expect(props.upload.category).toBe('text');
expect(props.upload.status).toBe(responseStatus);
expect(props.upload.errors[0].code).toBe(400);
expect(props.upload.errors[0].type).toBe(responseStatus);
expect(props.upload.errors[0].value).toBe(message);
} else {
expect(props.id).toBe('Form_Test_Field');
expect(props.upload.status).toBe(responseStatus);
expect(props.upload.errors).toBe(undefined);
expect(props.upload.category).toBe('image');
expect(props.upload.progress).toBe(100);
}
});
});
});
1 change: 1 addition & 0 deletions client/src/state/previewField/PreviewFieldActionTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ export default {
PREVIEWFIELD_REMOVE_FILE: 'PREVIEWFIELD_REMOVE_FILE',
PREVIEWFIELD_UPDATE_FILE: 'PREVIEWFIELD_UPDATE_FILE',
PREVIEWFIELD_FAIL_UPLOAD: 'PREVIEWFIELD_FAIL_UPLOAD',
PREVIEWFIELD_UPDATE_STATUS: 'PREVIEWFIELD_UPDATE_STATUS',
};
7 changes: 7 additions & 0 deletions client/src/state/previewField/PreviewFieldActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,10 @@ export function updateFile(id, data) {
payload: { id, data },
};
}

export function updateStatus(id, status) {
return {
type: ACTION_TYPES.PREVIEWFIELD_UPDATE_STATUS,
payload: { id, status },
};
}
6 changes: 6 additions & 0 deletions client/src/state/previewField/PreviewFieldReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ function previewFieldReducer(state = initialState, action) {
}));
}

case ACTION_TYPES.PREVIEWFIELD_UPDATE_STATUS: {
return deepFreeze(Object.assign({}, state, {
[action.payload.id]: Object.assign({}, state[action.payload.id], action.payload.status),
}));
}

default:
return state;
}
Expand Down
16 changes: 16 additions & 0 deletions client/src/state/previewField/tests/PreviewFieldReducer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,20 @@ describe('previewFieldReducer', () => {
expect(nextState[id].progress).toBe(32);
});
});

describe('PREVIEWFIELD_UPDATE_STATUS', () => {
it('should add the status to the existing file state', () => {
const status = {
status: 'error',
};
initialState[id] = { id: 543 };
const nextState = previewFieldReducer(initialState, {
type: ACTION_TYPES.PREVIEWFIELD_UPDATE_STATUS,
payload: { id, status },
});

expect(nextState[id].id).toBe(543);
expect(nextState[id].status).toBe(status.status);
});
});
});
16 changes: 16 additions & 0 deletions tests/behat/features/replace-file.feature
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,19 @@ Feature: Replace a file with a new file
When I press the "Save" button
Then the "Name" field should contain "document-v2.pdf"
And I should not see a ".preview-image-field__message--success" element

@javascript
Scenario: I upload an image file exceeding maximum file size to replace a file
Given the maximum file size is "5k"
And I go to "/admin/assets"
And I select the file named "folder1" in the gallery
And I click on the file named "file1" in the gallery
When I attach the file "file1.jpg" to dropzone "PreviewImage"
Then I should see a ".preview-image-field__message-button" element
And I should see a ".preview-image-field__message--success" element
And the "Name" field should contain "file1.jpg"
When I attach the file "file3.jpg" to dropzone "PreviewImage"
And I wait for 5 second
Then I should see a ".preview-image-field__message--error" element
And I should see "Filesize is too large, maximum 5 KB allowed" in the ".preview-image-field__message--error" element
And the "Name" field should contain "file1.jpg"
Binary file added tests/behat/files/file3.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
26 changes: 26 additions & 0 deletions tests/behat/src/FixtureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -508,4 +508,30 @@ public function iScrollTheEditorDetailsPanelToTheTop()
$script = "document.querySelector('.editor__details fieldset').scrollTo(0, 0);";
$this->getMainContext()->getSession()->executeScript($script);
}

/**
* Example: Given the maximum file size is 5k
*
* @Given /^the maximum file size is "([^"]+)"$/
* @param string $size Max file size
*/
public function stepCreateMaximumFileSizeStep($size): void
{
$config = <<<YAML
---
name: fileallowedsize
---
SilverStripe\Assets\Upload_Validator:
default_max_file_size:
'*': $size
YAML;

$file = 'file-allowed-size.yml';
$path = $this->getDestinationConfigFolder($file);
file_put_contents($path, $config);

$this->activatedConfigFiles[] = $path;
$this->getMainContext()->visit('dev/build?flush');
}
}

0 comments on commit c692b7f

Please sign in to comment.