Skip to content

Commit

Permalink
Merge pull request #657 from adobe/issue-656
Browse files Browse the repository at this point in the history 
fix: single file selection should not trigger bulk operation
  • Loading branch information
@rofe
rofe authored Jan 18, 2024
2 parents 2e271be + 066f31e commit 5533e2e
Show file tree
Hide file tree
Showing 8 changed files with 188 additions and 42 deletions.
32 changes: 32 additions & 0 deletions src/extension/module.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1828,6 +1828,38 @@ import sampleRUM from './rum.js';
return;
}
try {
if (paths.length === 1) {
// single operation
const [path] = paths;
const url = getAdminUrl(config, route, path);
try {
const resp = await fetch(url, {
...getAdminFetchOptions(),
method,
});
if (!resp.ok) {
throw new Error(resp.headers['x-error']);
} else {
showBulkOperationSummary({
operation,
resources: [{ path, status: resp.status }],
host,
});
}
} catch (e) {
console.error(`bulk ${operation} failed: ${e.message}`);
sk.showModal({
message: [
getBulkText([1], 'result', operation, 'failure'),
e.message || i18n(sk, 'bulk_error'),
],
level: 0,
sticky: true,
});
}
return;
}
// bulk operation
const bulkUrl = getAdminUrl(config, route, '/*');
const bulkResp = await fetch(bulkUrl, {
...getAdminFetchOptions(),
Expand Down
2 changes: 1 addition & 1 deletion src/extension/options.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ window.addEventListener('DOMContentLoaded', () => {
// create file link on the fly
const exportLink = document.createElement('a');
exportLink.download = `helix-sidekick-backup-${Date.now()}.json`;
exportLink.href = `data:application/json;charset=utf-8,${encodeURIComponent(data)}`;
exportLink.href = `data:application/x-file-download;charset=utf-8,${encodeURIComponent(data)}`;
// exportLink.classList.add('hidden');
document.body.append(exportLink);
exportLink.click();
Expand Down
29 changes: 19 additions & 10 deletions test/bulk-copy-urls.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('Test bulk copy URLs plugin', () => {
loadModule: true,
acceptDialogs: true,
}).run();
assert.deepStrictEqual(buttonTexts, ['Copy URL', 'Copy URLs'], 'Did not adapt button text');
assert.deepStrictEqual(buttonTexts, ['Copy URLs', 'Copy URL'], 'Did not adapt button text');
}).timeout(IT_DEFAULT_TIMEOUT);

it('Bulk copy URLs plugin hidden on empty selection', async () => {
Expand All @@ -105,8 +105,9 @@ describe('Test bulk copy URLs plugin', () => {
fixture: TESTS[0].fixture,
url: setup.getUrl('edit', 'admin'),
pre: (p) => p.evaluate(() => {
// user deselects file
// user deselects all
document.getElementById('file-pdf').click();
document.getElementById('file-word').click();
}),
loadModule: true,
}).run();
Expand All @@ -131,8 +132,9 @@ describe('Test bulk copy URLs plugin', () => {
url: setup.getUrl('edit', 'admin'),
plugin: 'bulk-copy-preview-urls',
pre: (p) => p.evaluate(() => {
// user deselects file
// user deselects all
document.getElementById('file-pdf').click();
document.getElementById('file-word').click();
}),
loadModule: true,
}).run();
Expand Down Expand Up @@ -248,6 +250,9 @@ describe('Test bulk copy URLs plugin', () => {
url: setup.getUrl('edit', 'admin'),
plugin: 'bulk-copy-preview-urls',
pluginSleep: 500,
// only select first file
pre: (p) => p.evaluate(() => document.querySelectorAll('div.file')
.forEach((file, i) => file.setAttribute('aria-selected', `${i === 0}`))),
post: (p) => p.evaluate(() => {
window.hlx.clipboardText = 'dummy';
window.navigator.clipboard.writeText = (text) => {
Expand Down Expand Up @@ -280,11 +285,9 @@ describe('Test bulk copy URLs plugin', () => {
url: setup.getUrl('edit', 'admin'),
plugin: 'bulk-copy-preview-urls',
pluginSleep: 500,
pre: (p) => p.evaluate(() => {
// user selects more files
document.getElementById('file-word').click();
document.getElementById('file-excel').click();
}),
// select all supported files
pre: (p) => p.evaluate(() => document.querySelectorAll('div.file')
.forEach((file) => file.setAttribute('aria-selected', 'true'))),
post: (p) => p.evaluate(() => {
window.hlx.clipboardText = 'dummy';
window.navigator.clipboard.writeText = (text) => {
Expand All @@ -298,8 +301,8 @@ describe('Test bulk copy URLs plugin', () => {
clipboardText.split('\n'),
[
`https://${ref}--${repo}--${owner}.hlx.page/documents/file.pdf`,
`https://${ref}--${repo}--${owner}.hlx.page/documents/document${env === 'gdrive' ? '.docx' : ''}`,
`https://${ref}--${repo}--${owner}.hlx.page/documents/spreadsheet${env === 'gdrive' ? '.xlsx' : '.json'}`,
`https://${ref}--${repo}--${owner}.hlx.page/documents/document`,
`https://${ref}--${repo}--${owner}.hlx.page/documents/spreadsheet.json`,
],
`URLs not copied to clipboard in ${env}`,
);
Expand All @@ -322,6 +325,9 @@ describe('Test bulk copy URLs plugin', () => {
url: setup.getUrl('edit', 'admin'),
plugin: 'bulk-copy-live-urls',
pluginSleep: 500,
// only select first file
pre: (p) => p.evaluate(() => document.querySelectorAll('div.file')
.forEach((file, i) => file.setAttribute('aria-selected', `${i === 0}`))),
post: (p) => p.evaluate(() => {
window.hlx.clipboardText = 'dummy';
window.navigator.clipboard.writeText = (text) => {
Expand Down Expand Up @@ -355,6 +361,9 @@ describe('Test bulk copy URLs plugin', () => {
url: setup.getUrl('edit', 'admin'),
plugin: 'bulk-copy-prod-urls',
pluginSleep: 500,
// only select first file
pre: (p) => p.evaluate(() => document.querySelectorAll('div.file')
.forEach((file, i) => file.setAttribute('aria-selected', `${i === 0}`))),
post: (p) => p.evaluate(() => {
window.hlx.clipboardText = 'dummy';
window.navigator.clipboard.writeText = (text) => {
Expand Down
6 changes: 3 additions & 3 deletions test/bulk-info.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('Test bulk info plugin', () => {
route: 'status',
type: 'admin',
});
const { checkPageResult: sizeCheck } = await new SidekickTest({
const { checkPageResult: size } = await new SidekickTest({
browser,
page,
fixture: fixtureList,
Expand All @@ -75,10 +75,10 @@ describe('Test bulk info plugin', () => {
const num = +(info.textContent
.split(' ')
.shift());
return Number.isNaN(num) ? false : num === 1;
return num;
}),
}).run();
assert.ok(sizeCheck, 'Wrong selection size displayed');
assert.strictEqual(size, 2, 'Wrong selection size displayed');
}).timeout(IT_DEFAULT_TIMEOUT);

it(`Bulk info plugin displays selection size in ${env} grid view`, async () => {
Expand Down
88 changes: 79 additions & 9 deletions test/bulk-preview.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,9 @@ describe('Test bulk preview plugin', () => {
fixture: TESTS[0].fixture,
url: setup.getUrl('edit', 'admin'),
pre: (p) => p.evaluate(() => {
// user deselects file
// user deselects all
document.getElementById('file-pdf').click();
document.getElementById('file-word').click();
}),
loadModule: true,
}).run();
Expand Down Expand Up @@ -100,12 +101,13 @@ describe('Test bulk preview plugin', () => {
fixture: TESTS[0].fixture,
url: setup.getUrl('edit', 'admin'),
pre: (p) => p.evaluate(() => {
// user deselects file
// user deselects all
document.getElementById('file-pdf').click();
document.getElementById('file-word').click();
}),
post: (p) => p.evaluate(() => {
// user deselects another file
document.getElementById('file-word').click();
// user selects a file
document.getElementById('file-excel').click();
}),
loadModule: true,
}).run();
Expand All @@ -129,8 +131,9 @@ describe('Test bulk preview plugin', () => {
url: setup.getUrl('edit', 'admin'),
plugin: 'bulk-preview',
pre: (p) => p.evaluate(() => {
// user deselects file
// user deselects all
document.getElementById('file-pdf').click();
document.getElementById('file-word').click();
}),
loadModule: true,
}).run();
Expand Down Expand Up @@ -216,6 +219,43 @@ describe('Test bulk preview plugin', () => {
}).timeout(IT_DEFAULT_TIMEOUT);
});

it('Bulk preview plugin previews single selection without creating a job', async () => {
const { setup, fixture } = TESTS[0];
const { sidekickConfig, configJson } = setup;
nock.sidekick(setup);
nock.admin(setup, {
route: 'status',
persist: true,
});
nock.admin(setup, {
route: 'preview',
method: 'post',
});
const { notification, requestsMade } = await new SidekickTest({
browser,
page,
fixture,
sidekickConfig,
configJson,
url: setup.getUrl('edit', 'admin'),
plugin: 'bulk-preview',
pluginSleep: 500,
// only select first file
pre: (p) => p.evaluate(() => document.querySelectorAll('div.file')
.forEach((file, i) => file.setAttribute('aria-selected', `${i === 0}`))),
loadModule: true,
acceptDialogs: true,
}).run();
assert.ok(
notification?.message.startsWith('Preview of this file successfully generated'),
'Did not bulk publish single selection',
);
assert.ok(
requestsMade.find((req) => req.method === 'POST' && !new URL(req.url).pathname.endsWith('/*')),
'Did not bulk preview single selection without creating a job',
);
}).timeout(IT_DEFAULT_TIMEOUT);

it('Bulk preview plugin handles API error', async () => {
const { setup } = TESTS[0];
nock.sidekick();
Expand Down Expand Up @@ -266,6 +306,40 @@ describe('Test bulk preview plugin', () => {
);
}).timeout(IT_DEFAULT_TIMEOUT);

it('Bulk preview plugin handles API error with single selection', async () => {
const { setup } = TESTS[0];
nock.sidekick();
nock.admin(setup, {
route: 'status',
type: 'admin',
persist: true,
});
nock.admin(setup, {
route: 'preview',
type: 'html',
method: 'post',
status: [500],
});
const test = await new SidekickTest({
browser,
page,
fixture: SHAREPOINT_FIXTURE,
url: setup.getUrl('edit', 'admin'),
plugin: 'bulk-preview',
pluginSleep: 500,
// only select first file
pre: (p) => p.evaluate(() => document.querySelectorAll('div.file')
.forEach((file, i) => file.setAttribute('aria-selected', `${i === 0}`))),
loadModule: true,
acceptDialogs: true,
});
const { notification } = await test.run();
assert.ok(
notification?.className?.includes('level-0'),
'Did not handle 500 error with single selection',
);
}).timeout(IT_DEFAULT_TIMEOUT);

it('Bulk preview plugin handles content error', async () => {
const { setup } = TESTS[0];
nock.sidekick();
Expand Down Expand Up @@ -309,10 +383,6 @@ describe('Test bulk preview plugin', () => {
url: setup.getUrl('edit', 'admin'),
plugin: 'bulk-preview',
pluginSleep: 5000,
pre: (p) => p.evaluate(() => {
// select another file
document.getElementById('file-word').click();
}),
loadModule: true,
acceptDialogs: true,
}).run();
Expand Down
55 changes: 45 additions & 10 deletions test/bulk-publish.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ describe('Test bulk publish plugin', () => {
fixture: TESTS[0].fixture,
url: setup.getUrl('edit', 'admin'),
pre: (p) => p.evaluate(() => {
// user deselects file
// user deselects all
document.getElementById('file-pdf').click();
document.getElementById('file-word').click();
}),
loadModule: true,
}).run();
Expand Down Expand Up @@ -101,12 +102,13 @@ describe('Test bulk publish plugin', () => {
url: setup.getUrl('edit', 'admin'),
loadModule: true,
pre: (p) => p.evaluate(() => {
// user deselects file
// user deselects all
document.getElementById('file-pdf').click();
document.getElementById('file-word').click();
}),
post: (p) => p.evaluate(() => {
// user selects another file
document.getElementById('file-word').click();
document.getElementById('file-excel').click();
}),
}).run();
assert.ok(
Expand All @@ -130,8 +132,9 @@ describe('Test bulk publish plugin', () => {
url: setup.getUrl('edit', 'admin'),
plugin: 'bulk-publish',
pre: (p) => p.evaluate(() => {
// user deselects file
// user deselects all
document.getElementById('file-pdf').click();
document.getElementById('file-word').click();
}),
loadModule: true,
}).run();
Expand Down Expand Up @@ -191,8 +194,7 @@ describe('Test bulk publish plugin', () => {
plugin: 'bulk-publish',
pluginSleep: 5000,
pre: (p) => p.evaluate(() => {
// user selects more files
document.getElementById('file-word').click();
// user selects one more file
document.getElementById('file-excel').click();
}),
checkPage: (p) => p.evaluate(() => {
Expand Down Expand Up @@ -225,6 +227,43 @@ describe('Test bulk publish plugin', () => {
}).timeout(IT_DEFAULT_TIMEOUT);
});

it('Bulk publish plugin previews single selection without creating a job', async () => {
const { setup, fixture } = TESTS[0];
const { sidekickConfig, configJson } = setup;
nock.sidekick(setup);
nock.admin(setup, {
route: 'status',
persist: true,
});
nock.admin(setup, {
route: 'live',
method: 'post',
});
const { notification, requestsMade } = await new SidekickTest({
browser,
page,
fixture,
sidekickConfig,
configJson,
url: setup.getUrl('edit', 'admin'),
plugin: 'bulk-publish',
pluginSleep: 500,
// only select first file
pre: (p) => p.evaluate(() => document.querySelectorAll('div.file')
.forEach((file, i) => file.setAttribute('aria-selected', `${i === 0}`))),
loadModule: true,
acceptDialogs: true,
}).run();
assert.ok(
notification?.message.startsWith('1 file successfully published'),
'Did not bulk publish single selection',
);
assert.ok(
requestsMade.find((req) => req.method === 'POST' && !new URL(req.url).pathname.startsWith('/*')),
'Did not bulk publish single selection without creating a job',
);
}).timeout(IT_DEFAULT_TIMEOUT);

it('Bulk publish plugin handles error response', async () => {
const { setup } = TESTS[0];
nock.sidekick(setup);
Expand Down Expand Up @@ -274,10 +313,6 @@ describe('Test bulk publish plugin', () => {
url: setup.getUrl('edit', 'admin'),
plugin: 'bulk-publish',
pluginSleep: 5000,
pre: (p) => p.evaluate(() => {
// select another file
document.getElementById('file-word').click();
}),
loadModule: true,
acceptDialogs: true,
}).run();
Expand Down
Loading

0 comments on commit 5533e2e

Please sign in to comment.