Commit 84a5ef20 authored by Robert Knight's avatar Robert Knight

Add document title to suggested filename for export files

 - Add the document title to the suggested filename for exports, to make
   it unique across different documents by default.

 - Move `export-annotations.ts` to the `helpers/` directory, since it
   contains functions that are very much application-specific. The
   `util/` directory contains code which is potentially usable in other
   applications or libraries / is less domain specific.

 - Add a note about how filenames that are invalid on the user's system
   are handled

Part of https://github.com/hypothesis/client/issues/5709
parent 023cae3f
...@@ -10,11 +10,11 @@ import { downloadJSONFile } from '../../../shared/download-json-file'; ...@@ -10,11 +10,11 @@ import { downloadJSONFile } from '../../../shared/download-json-file';
import type { APIAnnotationData } from '../../../types/api'; import type { APIAnnotationData } from '../../../types/api';
import { annotationDisplayName } from '../../helpers/annotation-user'; import { annotationDisplayName } from '../../helpers/annotation-user';
import { annotationsByUser } from '../../helpers/annotations-by-user'; import { annotationsByUser } from '../../helpers/annotations-by-user';
import { suggestedFilename } from '../../helpers/export-annotations';
import { withServices } from '../../service-context'; import { withServices } from '../../service-context';
import type { AnnotationsExporter } from '../../services/annotations-exporter'; import type { AnnotationsExporter } from '../../services/annotations-exporter';
import type { ToastMessengerService } from '../../services/toast-messenger'; import type { ToastMessengerService } from '../../services/toast-messenger';
import { useSidebarStore } from '../../store'; import { useSidebarStore } from '../../store';
import { suggestedFilename } from '../../util/export-annotations';
import LoadingSpinner from './LoadingSpinner'; import LoadingSpinner from './LoadingSpinner';
export type ExportAnnotationsProps = { export type ExportAnnotationsProps = {
...@@ -34,6 +34,7 @@ function ExportAnnotations({ ...@@ -34,6 +34,7 @@ function ExportAnnotations({
const store = useSidebarStore(); const store = useSidebarStore();
const group = store.focusedGroup(); const group = store.focusedGroup();
const exportReady = group && !store.isLoading(); const exportReady = group && !store.isLoading();
const contentFrame = store.defaultContentFrame();
const exportableAnnotations = store.savedAnnotations(); const exportableAnnotations = store.savedAnnotations();
const defaultAuthority = store.defaultAuthority(); const defaultAuthority = store.defaultAuthority();
...@@ -56,8 +57,12 @@ function ExportAnnotations({ ...@@ -56,8 +57,12 @@ function ExportAnnotations({
const draftCount = store.countDrafts(); const draftCount = store.countDrafts();
const defaultFilename = useMemo( const defaultFilename = useMemo(
() => suggestedFilename({ groupName: group?.name }), () =>
[group], suggestedFilename({
documentMetadata: contentFrame.metadata,
groupName: group?.name,
}),
[contentFrame, group],
); );
const [customFilename, setCustomFilename] = useState<string>(); const [customFilename, setCustomFilename] = useState<string>();
......
...@@ -11,6 +11,7 @@ describe('ExportAnnotations', () => { ...@@ -11,6 +11,7 @@ describe('ExportAnnotations', () => {
let fakeAnnotationsExporter; let fakeAnnotationsExporter;
let fakeToastMessenger; let fakeToastMessenger;
let fakeDownloadJSONFile; let fakeDownloadJSONFile;
let fakeSuggestedFilename;
const fakePrivateGroup = { const fakePrivateGroup = {
type: 'private', type: 'private',
...@@ -40,12 +41,16 @@ describe('ExportAnnotations', () => { ...@@ -40,12 +41,16 @@ describe('ExportAnnotations', () => {
isFeatureEnabled: sinon.stub().returns(true), isFeatureEnabled: sinon.stub().returns(true),
profile: sinon.stub().returns({ userid: 'acct:john@example.com' }), profile: sinon.stub().returns({ userid: 'acct:john@example.com' }),
countDrafts: sinon.stub().returns(0), countDrafts: sinon.stub().returns(0),
defaultContentFrame: sinon.stub().returns({
metadata: { title: 'Example document' },
}),
focusedGroup: sinon.stub().returns(fakePrivateGroup), focusedGroup: sinon.stub().returns(fakePrivateGroup),
isLoading: sinon.stub().returns(false), isLoading: sinon.stub().returns(false),
savedAnnotations: sinon savedAnnotations: sinon
.stub() .stub()
.returns([fixtures.oldAnnotation(), fixtures.oldAnnotation()]), .returns([fixtures.oldAnnotation(), fixtures.oldAnnotation()]),
}; };
fakeSuggestedFilename = sinon.stub().returns('suggested-filename');
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
...@@ -53,10 +58,10 @@ describe('ExportAnnotations', () => { ...@@ -53,10 +58,10 @@ describe('ExportAnnotations', () => {
'../../../shared/download-json-file': { '../../../shared/download-json-file': {
downloadJSONFile: fakeDownloadJSONFile, downloadJSONFile: fakeDownloadJSONFile,
}, },
'../../store': { useSidebarStore: () => fakeStore }, '../../helpers/export-annotations': {
'../../util/export-annotations': { suggestedFilename: fakeSuggestedFilename,
suggestedFilename: () => 'suggested-filename',
}, },
'../../store': { useSidebarStore: () => fakeStore },
}); });
// Restore this very simple component to get it test coverage // Restore this very simple component to get it test coverage
...@@ -163,8 +168,12 @@ describe('ExportAnnotations', () => { ...@@ -163,8 +168,12 @@ describe('ExportAnnotations', () => {
it('provides a filename field with a default suggested name', () => { it('provides a filename field with a default suggested name', () => {
const wrapper = createComponent(); const wrapper = createComponent();
const input = wrapper.find('Input'); const input = wrapper.find('Input');
assert.isTrue(input.exists()); assert.isTrue(input.exists());
assert.calledWith(fakeSuggestedFilename, {
groupName: fakeStore.focusedGroup().name,
documentMetadata: fakeStore.defaultContentFrame().metadata,
});
assert.equal(input.prop('defaultValue'), 'suggested-filename'); assert.equal(input.prop('defaultValue'), 'suggested-filename');
}); });
......
import type { DocumentMetadata } from '../../types/annotator';
/** Format a date as an ISO `YYYY-MM-DD` string. */
export function formatDate(date: Date): string {
const year = date.getFullYear();
const month = `${date.getMonth() + 1}`.padStart(2, '0');
const day = `${date.getDate()}`.padStart(2, '0');
return `${year}-${month}-${day}`;
}
export type SuggestedFilenameOptions = {
/** Metadata of the document the annotations are associated with. */
documentMetadata?: DocumentMetadata;
/** Name of the group containing the exported annotations. */
groupName?: string;
/** Date of the export. Defaults to the current date. */
date?: Date;
};
/**
* Generate a default filename for a file containing an exported set of
* annotations.
*
* The generated filename may not meet the character, length or other
* restrictions of the user's system. It is assumed the browser will modify the
* filename as needed when it downloads the file.
*/
export function suggestedFilename({
documentMetadata,
groupName,
/* istanbul ignore next - test seam */
date = new Date(),
}: SuggestedFilenameOptions) {
const filenameSegments = [formatDate(date)];
if (documentMetadata?.title) {
filenameSegments.push(documentMetadata.title);
} else {
filenameSegments.push('Hypothesis');
}
if (groupName) {
filenameSegments.push(groupName.replace(/ /g, '-'));
}
return filenameSegments.join('-');
}
...@@ -2,6 +2,7 @@ import { suggestedFilename } from '../export-annotations'; ...@@ -2,6 +2,7 @@ import { suggestedFilename } from '../export-annotations';
describe('suggestedFilename', () => { describe('suggestedFilename', () => {
[ [
// Date only
{ {
date: new Date(2023, 5, 23), date: new Date(2023, 5, 23),
expectedResult: '2023-06-23-Hypothesis', expectedResult: '2023-06-23-Hypothesis',
...@@ -10,14 +11,35 @@ describe('suggestedFilename', () => { ...@@ -10,14 +11,35 @@ describe('suggestedFilename', () => {
date: new Date(2019, 0, 5), date: new Date(2019, 0, 5),
expectedResult: '2019-01-05-Hypothesis', expectedResult: '2019-01-05-Hypothesis',
}, },
// Date and group name only
{ {
date: new Date(2020, 10, 5), date: new Date(2020, 10, 5),
groupName: 'My group name', groupName: 'My group name',
expectedResult: '2020-11-05-Hypothesis-My-group-name', expectedResult: '2020-11-05-Hypothesis-My-group-name',
}, },
].forEach(({ date, groupName, expectedResult }) => { {
date: new Date(2020, 10, 5),
groupName: 'My group name',
documentMetadata: {
title: '', // There is a title, but it is empty
},
expectedResult: '2020-11-05-Hypothesis-My-group-name',
},
// Date, group name and non-empty title
{
date: new Date(2020, 10, 5),
groupName: 'My group name',
documentMetadata: {
title: 'Example domain',
},
expectedResult: '2020-11-05-Example domain-My-group-name',
},
].forEach(({ date, documentMetadata, groupName, expectedResult }) => {
it('builds expected filename for provided arguments', () => { it('builds expected filename for provided arguments', () => {
assert.equal(suggestedFilename({ date, groupName }), expectedResult); assert.equal(
suggestedFilename({ date, documentMetadata, groupName }),
expectedResult,
);
}); });
}); });
}); });
const formatDate = (date: Date): string => {
const year = date.getFullYear();
const month = `${date.getMonth() + 1}`.padStart(2, '0');
const day = `${date.getDate()}`.padStart(2, '0');
return `${year}-${month}-${day}`;
};
type SuggestedFilenameOptions = {
groupName?: string;
date?: Date;
};
export const suggestedFilename = ({
groupName,
/* istanbul ignore next - test seam */
date = new Date(),
}: SuggestedFilenameOptions) => {
const filenameSegments = [formatDate(date), 'Hypothesis'];
if (groupName) {
filenameSegments.push(groupName.replace(/ /g, '-'));
}
return filenameSegments.join('-');
};
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