Skip to content

Commit

Permalink
Import modal: better error for Invalid file format. (#4159) (#4163)
Browse files Browse the repository at this point in the history
* Import modal: better error for Invalid file format.

we're checking the mime-type before allowing collection upload,
this mostly works, but in some cases (see AAP-15541), it can fail unexpectedly.

This should make debugging that case easier, by including the detected type in the error message.

No-Issue

* make filetype detection error skippable

(cherry picked from commit 9d4df78)
  • Loading branch information
himdel authored Aug 31, 2023
1 parent e63520f commit bac4520
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 8 deletions.
6 changes: 5 additions & 1 deletion src/components/import-modal/import-modal.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ $green: #5bb75b;

.upload-collection {
.file-error-messages {
color: #c00;
color: var(--pf-global--danger-color--100);

&.skipped {
color: var(--pf-global--warning-color--100);
}
}

.upload-file {
Expand Down
43 changes: 36 additions & 7 deletions src/components/import-modal/import-modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { t } from '@lingui/macro';
import { Button, Modal, Radio } from '@patternfly/react-core';
import { FolderOpenIcon, SpinnerIcon } from '@patternfly/react-icons';
import axios from 'axios';
import * as React from 'react';
import cx from 'classnames';
import React from 'react';
import {
AnsibleRepositoryAPI,
AnsibleRepositoryType,
Expand Down Expand Up @@ -36,6 +37,7 @@ interface IProps {
interface IState {
file?: File;
errors?: string;
errorVariant: 'default' | 'skippable' | 'skipped';
uploadProgress: number;
uploadStatus: Status;
allRepos: AnsibleRepositoryType[];
Expand All @@ -57,6 +59,7 @@ export class ImportModal extends React.Component<IProps, IState> {
this.state = {
file: undefined,
errors: '',
errorVariant: 'default' as const,
uploadProgress: 0,
uploadStatus: Status.waiting,
allRepos: [],
Expand Down Expand Up @@ -161,7 +164,13 @@ export class ImportModal extends React.Component<IProps, IState> {
render() {
const { isOpen, collection } = this.props;

const { file, errors, uploadProgress, uploadStatus } = this.state;
const { file, errors, errorVariant, uploadProgress, uploadStatus } =
this.state;
const skipError = () => {
if (errorVariant === 'skippable') {
this.setState({ errorVariant: 'skipped' as const });
}
};

return (
<Modal
Expand Down Expand Up @@ -221,8 +230,14 @@ export class ImportModal extends React.Component<IProps, IState> {
</label>
</form>
{errors ? (
<span className='file-error-messages'>
<i className='pficon-error-circle-o' /> {errors}
<span className={cx('file-error-messages', errorVariant)}>
{errors}
{errorVariant === 'skippable' && (
<>
{' '}
<a onClick={skipError}>{t`Upload anyway?`}</a>
</>
)}
</span>
) : null}
</div>
Expand Down Expand Up @@ -261,7 +276,11 @@ export class ImportModal extends React.Component<IProps, IState> {
fixedRepos={this.state.fixedRepos}
selectedRepos={this.state.selectedRepos}
setSelectedRepos={(repos) =>
this.setState({ selectedRepos: repos, errors: '' })
this.setState({
selectedRepos: repos,
errors: '',
errorVariant: 'default' as const,
})
}
hideFixedRepos={true}
loadRepos={(params, setRepositoryList, setLoading, setItemsCount) =>
Expand All @@ -279,7 +298,7 @@ export class ImportModal extends React.Component<IProps, IState> {
}

private canUpload() {
if (this.state.errors) {
if (this.state.errors && this.state.errorVariant !== 'skipped') {
return false;
}

Expand Down Expand Up @@ -312,16 +331,21 @@ export class ImportModal extends React.Component<IProps, IState> {
if (files.length > 1) {
this.setState({
errors: t`Please select no more than one file.`,
errorVariant: 'default' as const,
});
} else if (!this.acceptedFileTypes.includes(newCollection.type)) {
const detectedType = newCollection.type || t`unknown`;
const acceptedTypes: string = this.acceptedFileTypes.join(', ');
this.setState({
errors: t`Invalid file format.`,
errors: t`Invalid file format: ${detectedType} (expected: ${acceptedTypes}).`,
errorVariant: 'skippable' as const,
file: newCollection,
uploadProgress: 0,
});
} else if (!this.COLLECTION_NAME_REGEX.test(newCollection.name)) {
this.setState({
errors: t`Invalid file name. Collections must be formatted as 'namespace-collection_name-1.0.0'`,
errorVariant: 'default' as const,
file: newCollection,
uploadProgress: 0,
});
Expand All @@ -331,18 +355,21 @@ export class ImportModal extends React.Component<IProps, IState> {
) {
this.setState({
errors: t`The collection you have selected doesn't appear to match ${collection.name}`,
errorVariant: 'default' as const,
file: newCollection,
uploadProgress: 0,
});
} else if (this.props.namespace != newCollection.name.split('-')[0]) {
this.setState({
errors: t`The collection you have selected does not match this namespace.`,
errorVariant: 'default' as const,
file: newCollection,
uploadProgress: 0,
});
} else {
this.setState({
errors: '',
errorVariant: 'default' as const,
file: newCollection,
uploadProgress: 0,
});
Expand Down Expand Up @@ -410,6 +437,7 @@ export class ImportModal extends React.Component<IProps, IState> {
this.setState({
uploadStatus: Status.waiting,
errors: errorMessage,
errorVariant: 'default' as const,
});
})
.finally(() => {
Expand All @@ -428,6 +456,7 @@ export class ImportModal extends React.Component<IProps, IState> {
{
file: undefined,
errors: '',
errorVariant: 'default' as const,
uploadProgress: 0,
uploadStatus: Status.waiting,
},
Expand Down

0 comments on commit bac4520

Please sign in to comment.