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.

Sign up for GitHub

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

Jump to bottom

feat: validate submission response required fields #241

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

feat: validate submission response required fields #241

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e9e12b6
feat: add required property to both FileResponseConfig and TextRespon…
hajorg Sep 20, 2024
bc708c1
feat: add a validation hook for required submission fields
hajorg Sep 20, 2024
6499d95
feat: validate submission required fields
hajorg Sep 20, 2024
d6648b4
fix: resolve linting issue
hajorg Sep 20, 2024
db0168e
feat: add test for useSubmissionValidationStatus
hajorg Sep 23, 2024
f1a8bff
feat: remove unsupported multiple attribute from dropzone
hajorg Oct 2, 2024
c6544a3
Merge branch 'master' into hajorg/au-2106-validate-required-submissio…
hajorg Oct 2, 2024
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
94 changes: 55 additions & 39 deletions src/components/FileUpload/__snapshots__/index.test.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ exports[`<FileUpload /> render default 1`] = `
<div>
<h3>
File upload

</h3>
<b>
Uploaded files
Expand Down Expand Up @@ -57,20 +58,24 @@ exports[`<FileUpload /> render default 1`] = `
]
}
/>
<Dropzone
accept={
{
"*": [
".pdf",
".jpg",
],
<Form.Group
isInValid={true}
>
<Dropzone
accept={
{
"*": [
".pdf",
".jpg",
],
}
}
}
maxSize={123456}
multiple={true}
onProcessUpload={[MockFunction onProcessUpload]}
progressVariant="bar"
/>
maxSize={123456}
multiple={true}
onProcessUpload={[MockFunction onProcessUpload]}
progressVariant="bar"
/>
</Form.Group>
</div>
`;

Expand All @@ -82,6 +87,7 @@ exports[`<FileUpload /> render extra columns when activeStepName is submission 1
<div>
<h3>
File upload

</h3>
<b>
Uploaded files
Expand Down Expand Up @@ -140,27 +146,32 @@ exports[`<FileUpload /> render extra columns when activeStepName is submission 1
]
}
/>
<Dropzone
accept={
{
"*": [
".pdf",
".jpg",
],
<Form.Group
isInValid={true}
>
<Dropzone
accept={
{
"*": [
".pdf",
".jpg",
],
}
}
}
maxSize={123456}
multiple={true}
onProcessUpload={[MockFunction onProcessUpload]}
progressVariant="bar"
/>
maxSize={123456}
multiple={true}
onProcessUpload={[MockFunction onProcessUpload]}
progressVariant="bar"
/>
</Form.Group>
</div>
`;

exports[`<FileUpload /> render without dropzone and confirm modal when isReadOnly 1`] = `
<div>
<h3>
File upload

</h3>
<FilePreview
defaultCollapsePreview={false}
Expand Down Expand Up @@ -224,6 +235,7 @@ exports[`<FileUpload /> render without file preview if uploadedFiles are empty a
<div>
<h3>
File upload

</h3>
<b>
Uploaded files
Expand Down Expand Up @@ -312,20 +324,24 @@ exports[`<FileUpload /> render without header 1`] = `
]
}
/>
<Dropzone
accept={
{
"*": [
".pdf",
".jpg",
],
<Form.Group
isInValid={true}
>
<Dropzone
accept={
{
"*": [
".pdf",
".jpg",
],
}
}
}
maxSize={123456}
multiple={true}
onProcessUpload={[MockFunction onProcessUpload]}
progressVariant="bar"
/>
maxSize={123456}
multiple={true}
onProcessUpload={[MockFunction onProcessUpload]}
progressVariant="bar"
/>
</Form.Group>
</div>
`;

Expand Down
30 changes: 18 additions & 12 deletions src/components/FileUpload/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import filesize from 'filesize';

import { DataTable, Dropzone } from '@openedx/paragon';
import { DataTable, Dropzone, Form } from '@openedx/paragon';
import { useIntl } from '@edx/frontend-platform/i18n';

import { nullMethod } from 'utils';
Expand Down Expand Up @@ -35,6 +35,7 @@ const FileUpload = ({
onDeletedFile,
defaultCollapsePreview,
hideHeader,
isInValid,
}) => {
const { formatMessage } = useIntl();
const {
Expand All @@ -47,7 +48,7 @@ const FileUpload = ({
const viewStep = useViewStep();
const activeStepName = useActiveStepName();
const {
enabled, fileUploadLimit, allowedExtensions, maxFileSize,
enabled, fileUploadLimit, allowedExtensions, maxFileSize, required,
} = useFileUploadConfig() || {};

if (!enabled || viewStep === stepNames.studentTraining) {
Expand Down Expand Up @@ -78,7 +79,7 @@ const FileUpload = ({

return (
<div>
{!hideHeader && <h3>{formatMessage(messages.fileUploadTitle)}</h3>}
{!hideHeader && <h3>{formatMessage(messages.fileUploadTitle)} {required && <span>(required)</span>}</h3>}
{uploadedFiles.length > 0 && isReadOnly && (
<FilePreview defaultCollapsePreview={defaultCollapsePreview} />
)}
Expand All @@ -93,15 +94,18 @@ const FileUpload = ({
columns={columns}
/>
{!isReadOnly && fileUploadLimit > uploadedFiles.length && (
<Dropzone
multiple
onProcessUpload={onProcessUpload}
progressVariant="bar"
accept={{
'*': (allowedExtensions || []).map((ext) => `.${ext}`),
}}
maxSize={maxFileSize}
/>
<Form.Group isInValid>
<Dropzone
multiple
Copy link
Member

Choose a reason for hiding this comment

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

I know that you didn't change this, but to my knowledge the paragon version of dropzone does not support multiple files. I would double check this functionality and remove the attribute if multiple files are not supported.

Copy link
Author

Choose a reason for hiding this comment

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

Correct. I attempted to upload two files at a time but came across this error Only one upload permitted.

onProcessUpload={onProcessUpload}
progressVariant="bar"
accept={{
'*': (allowedExtensions || []).map((ext) => `.${ext}`),
}}
maxSize={maxFileSize}
/>
{isInValid && <Form.Control.Feedback type="invalid">{formatMessage(messages.required)}</Form.Control.Feedback>}
</Form.Group>
)}
{!isReadOnly && isModalOpen && (
<UploadConfirmModal
Expand All @@ -122,6 +126,7 @@ FileUpload.defaultProps = {
onDeletedFile: nullMethod,
defaultCollapsePreview: false,
hideHeader: false,
isInValid: false,
};
FileUpload.propTypes = {
isReadOnly: PropTypes.bool,
Expand All @@ -137,6 +142,7 @@ FileUpload.propTypes = {
onDeletedFile: PropTypes.func,
defaultCollapsePreview: PropTypes.bool,
hideHeader: PropTypes.bool,
isInValid: PropTypes.bool,
};

export default FileUpload;
5 changes: 5 additions & 0 deletions src/components/FileUpload/messages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ const messages = defineMessages({
defaultMessage: 'File description',
description: 'Popover title for file description',
},
required: {
id: 'frontend-app-ora.FileUpload.required',
defaultMessage: 'File Upload is required',
description: 'Indicating file upload is required',
},
});

export default messages;
2 changes: 2 additions & 0 deletions src/data/services/lms/types/blockInfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface TextResponseConfig {
optional: boolean,
editorType: 'text' | 'tinymce',
allowLatexPreview: boolean,
required: boolean,
}

export interface FileResponseConfig {
Expand All @@ -27,6 +28,7 @@ export interface FileResponseConfig {
allowedExtensions: string[],
blockedExtensions: string[],
fileTypeDescription: string,
required: boolean,
}

export interface SubmissionConfig {
Expand Down
8 changes: 6 additions & 2 deletions src/hooks/actions/useConfirmAction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,14 @@ export const assessmentSteps = [
stepNames.peer,
];

const useConfirmAction = () => {
const useConfirmAction = (validateBeforeOpen = () => true) => {
const [isOpen, setIsOpen] = useKeyedState(stateKeys.isOpen, false);
const close = React.useCallback(() => setIsOpen(false), [setIsOpen]);
const open = React.useCallback(() => setIsOpen(true), [setIsOpen]);
const open = React.useCallback(() => {
if (validateBeforeOpen()) {
setIsOpen(true);
}
}, [setIsOpen, validateBeforeOpen]);
return React.useCallback((config) => {
const { description, title } = config;
const action = config.action.action ? config.action.action : config.action;
Expand Down
14 changes: 8 additions & 6 deletions src/hooks/actions/useConfirmAction.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ const nestedActionConfig = {
action: { action: config.action },
};

const validateBeforeOpen = jest.fn(() => true);

let out;
describe('useConfirmAction', () => {
beforeEach(() => {
jest.clearAllMocks();
state.mock();
out = useConfirmAction();
out = useConfirmAction(validateBeforeOpen);
});
afterEach(() => { state.resetVals(); });
describe('behavior', () => {
Expand All @@ -44,7 +46,7 @@ describe('useConfirmAction', () => {
state.expectSetStateCalledWith(stateKeys.isOpen, false);
};
const testOpen = (openFn) => {
expect(openFn.useCallback.prereqs).toEqual([state.setState[stateKeys.isOpen]]);
expect(openFn.useCallback.prereqs).toEqual([state.setState[stateKeys.isOpen], validateBeforeOpen]);
openFn.useCallback.cb();
state.expectSetStateCalledWith(stateKeys.isOpen, true);
};
Expand All @@ -62,13 +64,13 @@ describe('useConfirmAction', () => {
expect(out.useCallback.prereqs[1]).toEqual(true);
});
test('open callback', () => {
out = useConfirmAction();
out = useConfirmAction(validateBeforeOpen);
testOpen(prereqs[2]);
});
});
describe('callback', () => {
it('returns action with labels from config action', () => {
out = useConfirmAction().useCallback.cb(config);
out = useConfirmAction(validateBeforeOpen).useCallback.cb(config);
testOpen(out.action.onClick);
expect(out.action.children).toEqual(config.action.labels.default);
expect(out.confirmProps.isOpen).toEqual(false);
Expand All @@ -78,7 +80,7 @@ describe('useConfirmAction', () => {
testClose(out.confirmProps.close);
});
it('returns nested action from config action', () => {
out = useConfirmAction().useCallback.cb(nestedActionConfig);
out = useConfirmAction(validateBeforeOpen).useCallback.cb(nestedActionConfig);
testOpen(out.action.onClick);
expect(out.action.children).toEqual(nestedActionConfig.action.action.labels.default);
expect(out.confirmProps.isOpen).toEqual(false);
Expand All @@ -89,7 +91,7 @@ describe('useConfirmAction', () => {
});
it('returns action with children from config action', () => {
state.mockVals({ [stateKeys.isOpen]: true });
out = useConfirmAction().useCallback.cb(noLabelConfig);
out = useConfirmAction(validateBeforeOpen).useCallback.cb(noLabelConfig);
testOpen(out.action.onClick);
expect(out.action.children).toEqual(noLabelConfig.action.children);
expect(out.confirmProps.isOpen).toEqual(true);
Expand Down
4 changes: 2 additions & 2 deletions src/hooks/actions/useSubmitResponseAction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import messages, { confirmDescriptions, confirmTitles } from './messages';
*/
const useSubmitResponseAction = ({ options }) => {
const { formatMessage } = useIntl();
const { submit, submitStatus } = options;
const confirmAction = useConfirmAction();
const { submit, submitStatus, validateBeforeConfirmation } = options;
const confirmAction = useConfirmAction(validateBeforeConfirmation);
return confirmAction({
action: {
onClick: submit,
Expand Down
9 changes: 5 additions & 4 deletions src/hooks/actions/useSubmitResponseAction.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ jest.mock('./useConfirmAction', () => ({
default: jest.fn(),
}));

const mockConfirmAction = jest.fn(args => ({ confirmAction: args }));
when(useConfirmAction).calledWith().mockReturnValue(mockConfirmAction);

const options = {
submit: jest.fn(),
submitStatus: 'test-submit-status',
validateBeforeConfirmation: jest.fn(),
};

const mockConfirmAction = jest.fn(args => ({ confirmAction: args }));
when(useConfirmAction).calledWith(options.validateBeforeConfirmation).mockReturnValue(mockConfirmAction);

let out;
describe('useSubmitResponseAction', () => {
beforeEach(() => {
Expand All @@ -28,7 +29,7 @@ describe('useSubmitResponseAction', () => {
describe('behavior', () => {
it('loads internatioonalization and confirm action from hooks', () => {
expect(useIntl).toHaveBeenCalledWith();
expect(useConfirmAction).toHaveBeenCalledWith();
expect(useConfirmAction).toHaveBeenCalledWith(options.validateBeforeConfirmation);
});
});
describe('output confirmAction', () => {
Expand Down
Loading
Loading