Commit 3ea79419 authored by Robert Knight's avatar Robert Knight

Separate annotation and reply counts in exports

This ensures that the count of annotations displayed in the Export tab matches
the count of top-level annotations that we show in the user list, and actually
import, in the Import tab.

When reporting unsaved annotations, use the term "draft" which covers both
annotations and replies.

Part of https://github.com/hypothesis/client/issues/5742
parent 867af8ee
......@@ -2,6 +2,7 @@ import { Button, CardActions, Input } from '@hypothesis/frontend-shared';
import { useMemo, useState } from 'preact/hooks';
import { downloadJSONFile } from '../../../shared/download-json-file';
import { isReply } from '../../helpers/annotation-metadata';
import { withServices } from '../../service-context';
import type { AnnotationsExporter } from '../../services/annotations-exporter';
import type { ToastMessengerService } from '../../services/toast-messenger';
......@@ -26,8 +27,14 @@ function ExportAnnotations({
const store = useSidebarStore();
const group = store.focusedGroup();
const exportReady = group && !store.isLoading();
const exportableAnnotations = store.savedAnnotations();
const exportCount = exportableAnnotations.length;
const replyCount = useMemo(
() => exportableAnnotations.filter(ann => isReply(ann)).length,
[exportableAnnotations],
);
const nonReplyCount = exportableAnnotations.length - replyCount;
const draftCount = store.countDrafts();
const defaultFilename = useMemo(
......@@ -55,8 +62,8 @@ function ExportAnnotations({
};
// Naive simple English pluralization
const pluralize = (str: string, count: number) => {
return count === 1 ? str : `${str}s`;
const pluralize = (count: number, singular: string, plural: string) => {
return count === 1 ? singular : plural;
};
return (
......@@ -65,13 +72,21 @@ function ExportAnnotations({
onSubmit={exportAnnotations}
data-testid="export-form"
>
{exportCount > 0 ? (
{exportableAnnotations.length > 0 ? (
<>
<label data-testid="export-count" htmlFor="export-filename">
Export{' '}
<strong>
{exportCount} {pluralize('annotation', exportCount)}
{nonReplyCount}{' '}
{pluralize(nonReplyCount, 'annotation', 'annotations')}
</strong>{' '}
{replyCount > 0
? `(and ${replyCount} ${pluralize(
replyCount,
'reply',
'replies',
)}) `
: ''}
in a file named:
</label>
<Input
......@@ -93,15 +108,16 @@ function ExportAnnotations({
)}
{draftCount > 0 && (
<p data-testid="drafts-message">
You have {draftCount} unsaved {pluralize('annotation', draftCount)}
{exportCount > 0 && <> that will not be included</>}.
You have {draftCount} unsaved{' '}
{pluralize(draftCount, 'draft', 'drafts')}
{exportableAnnotations.length > 0 && <> that will not be included</>}.
</p>
)}
<CardActions>
<Button
data-testid="export-button"
variant="primary"
disabled={!exportCount}
disabled={exportableAnnotations.length === 0}
type="submit"
>
Export
......
......@@ -83,24 +83,41 @@ describe('ExportAnnotations', () => {
});
});
it('shows a count of annotations for export', () => {
fakeStore.savedAnnotations.returns([fixtures.oldAnnotation()]);
const wrapperSingular = createComponent();
fakeStore.savedAnnotations.returns([
fixtures.oldAnnotation(),
fixtures.oldAnnotation(),
]);
const wrapperPlural = createComponent();
assert.include(
wrapperSingular.find('[data-testid="export-count"]').text(),
'Export 1 annotation in a file',
);
assert.include(
wrapperPlural.find('[data-testid="export-count"]').text(),
'Export 2 annotations in a file',
);
[
{
annotations: [fixtures.oldAnnotation()],
message: 'Export 1 annotation in a file',
},
{
annotations: [fixtures.oldAnnotation(), fixtures.oldAnnotation()],
message: 'Export 2 annotations in a file',
},
{
annotations: [
fixtures.oldAnnotation(),
fixtures.oldAnnotation(),
fixtures.oldReply(),
],
message: 'Export 2 annotations (and 1 reply) in a file',
},
{
annotations: [
fixtures.oldAnnotation(),
fixtures.oldAnnotation(),
fixtures.oldReply(),
fixtures.oldReply(),
],
message: 'Export 2 annotations (and 2 replies) in a file',
},
].forEach(({ annotations, message }) => {
it('shows a count of annotations for export', () => {
fakeStore.savedAnnotations.returns(annotations);
const wrapper = createComponent();
assert.include(
wrapper.find('[data-testid="export-count"]').text(),
message,
);
});
});
it('provides a filename field with a default suggested name', () => {
......@@ -211,12 +228,12 @@ describe('ExportAnnotations', () => {
assert.include(
wrapperSingular.find('[data-testid="drafts-message"]').text(),
'You have 1 unsaved annotation that',
'You have 1 unsaved draft that',
);
assert.include(
wrapperPlural.find('[data-testid="drafts-message"]').text(),
'You have 2 unsaved annotations that',
'You have 2 unsaved drafts that',
);
});
});
......
......@@ -554,9 +554,10 @@ const orphanCount = createSelector(
/**
* Return all loaded annotations which have been saved to the server
*/
function savedAnnotations(state: State): SavedAnnotation[] {
return state.annotations.filter(ann => isSaved(ann)) as SavedAnnotation[];
}
const savedAnnotations = createSelector(
(state: State) => state.annotations,
annotations => annotations.filter(ann => isSaved(ann)) as SavedAnnotation[],
);
export const annotationsModule = createStoreModule(initialState, {
namespace: 'annotations',
......
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