Commit c2d543aa authored by Robert Knight's avatar Robert Knight

Disable import button if necessary data is not available

Prevent the user from importing annotations until we have:

 - A group to import into
 - A set of existing annotations to match against
 - Document metadata to use for the new annotations

I considered replacing the whole dialog with a `LoadingSpinner` until these
conditions are met, like the Export dialog does, but it makes for a less jumpy
transition when eg. switching groups if we just disable the button.
parent e225bc09
......@@ -87,8 +87,6 @@ function ImportAnnotations({
[annotations, getDisplayName],
);
const importsPending = store.importsPending();
// Parse input file, extract annotations and update the user list.
useEffect(() => {
if (!currentUser || !file) {
......@@ -135,11 +133,27 @@ function ImportAnnotations({
);
}
// In order to perform an import, we need:
//
// 1. A group to import into
// 2. A frame from which to get the document URI and metadata
// 3. Existing annotations for the group loaded so we can de-duplicate against
// them
// 4. A file to import from and a selection of what to import
// (`importAnnotations` will be falsey if this is not the case).
const importReady = Boolean(
store.focusedGroup() &&
store.mainFrame() &&
store.hasFetchedAnnotations() &&
!store.isFetchingAnnotations() &&
importAnnotations,
);
// True if we're validating a JSON file after it has been selected.
const parseInProgress = file && !annotations && !error;
// True if we're validating or importing.
const busy = parseInProgress || importsPending > 0;
const busy = parseInProgress || store.importsPending() > 0;
return (
<>
......@@ -183,7 +197,7 @@ function ImportAnnotations({
<CardActions>
<Button
data-testid="import-button"
disabled={!importAnnotations || busy}
disabled={!importReady || busy}
onClick={importAnnotations}
variant="primary"
>
......
......@@ -17,10 +17,14 @@ describe('ImportAnnotations', () => {
};
fakeStore = {
profile: sinon.stub().returns({ userid: 'acct:john@example.com' }),
defaultAuthority: sinon.stub().returns('example.com'),
isFeatureEnabled: sinon.stub().returns(true),
focusedGroup: sinon.stub().returns({ id: 'group-1' }),
importsPending: sinon.stub().returns(0),
isFeatureEnabled: sinon.stub().returns(true),
hasFetchedAnnotations: sinon.stub().returns(true),
isFetchingAnnotations: sinon.stub().returns(false),
mainFrame: sinon.stub().returns({}),
profile: sinon.stub().returns({ userid: 'acct:john@example.com' }),
};
$imports.$mock({
......@@ -42,13 +46,6 @@ describe('ImportAnnotations', () => {
);
}
it('shows a notice if the user is not logged in', () => {
fakeStore.profile.returns({ userid: null });
const wrapper = createImportAnnotations();
assert.isTrue(wrapper.exists('[data-testid="log-in-message"]'));
assert.isFalse(wrapper.exists('input[type="file"]'));
});
function getImportButton(wrapper) {
return wrapper.find('button[data-testid="import-button"]');
}
......@@ -69,9 +66,59 @@ describe('ImportAnnotations', () => {
fileInput.simulate('change');
}
function importDisabled(wrapper) {
return Boolean(getImportButton(wrapper).prop('disabled'));
}
it('shows a notice if the user is not logged in', () => {
fakeStore.profile.returns({ userid: null });
const wrapper = createImportAnnotations();
assert.isTrue(wrapper.exists('[data-testid="log-in-message"]'));
assert.isFalse(wrapper.exists('input[type="file"]'));
});
it('disables import button if group, document or annotations are not loaded', async () => {
fakeStore.mainFrame.returns(null); // Document metadata not available
fakeStore.focusedGroup.returns(null); // No group set
fakeStore.hasFetchedAnnotations.returns(false); // Annotations still loading
fakeStore.isFetchingAnnotations.returns(true);
// Select annotations to import.
const wrapper = createImportAnnotations();
const annotations = [
{
user: 'acct:john@example.com',
user_info: {
display_name: 'John Smith',
},
text: 'Test annotation',
},
];
selectFile(wrapper, annotations);
const userList = await waitForElement(wrapper, 'select');
assert.ok(userList.getDOMNode().value); // Current user should be auto-selected
// Import button should be disabled since we don't have the things we need
// to perform the import.
assert.isTrue(importDisabled(wrapper));
fakeStore.mainFrame.returns({}); // Document metadata available
wrapper.setProps({});
assert.isTrue(importDisabled(wrapper));
fakeStore.focusedGroup.returns({ id: 'group-1' }); // Group selected
wrapper.setProps({});
assert.isTrue(importDisabled(wrapper));
fakeStore.hasFetchedAnnotations.returns(true); // Annotation loading finishes
fakeStore.isFetchingAnnotations.returns(false);
wrapper.setProps({});
assert.isFalse(importDisabled(wrapper)); // Finally, we can start an import
});
it('disables "Import" button when no file is selected', () => {
const wrapper = createImportAnnotations();
assert.isTrue(getImportButton(wrapper).prop('disabled'));
assert.isTrue(importDisabled(wrapper));
});
[
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment