Skip to content

Commit

Permalink
Proper messaging when note is too long.
Browse files Browse the repository at this point in the history
  • Loading branch information
DougReeder committed Jan 12, 2024
1 parent 8e0ba7e commit cbea4ee
Show file tree
Hide file tree
Showing 11 changed files with 171 additions and 48 deletions.
53 changes: 37 additions & 16 deletions src/Detail.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// noinspection ExceptionCaughtLocallyJS

import {validate as uuidValidate} from 'uuid';
import {NodeNote} from './Note';
import {CONTENT_TOO_LONG, NodeNote, shortenTitle} from './Note';
import React, {useEffect, useState, useMemo, useCallback, useReducer, useRef} from 'react';
import PropTypes from 'prop-types';
import {ErrorBoundary} from 'react-error-boundary'
Expand Down Expand Up @@ -281,11 +281,12 @@ function Detail({noteId, searchWords = new Set(), focusOnLoadCB, setMustShowPane
}
} catch (err) {
console.error("handleSlateChange:", err);
if (201 === err?.error?.code && '/content' === err?.error?.dataPath) {
transientMsg("Can't save. Split this note into multiple notes");
if (CONTENT_TOO_LONG === err.userMsg) {
transientMsg(CONTENT_TOO_LONG);
} else {
setNoteErr(err);
}
canSave.current = true;
}
}

Expand All @@ -304,20 +305,28 @@ function Detail({noteId, searchWords = new Set(), focusOnLoadCB, setMustShowPane
}
} catch (err) {
console.error("Detail handleDateChange:", err);
transientMsg(extractUserMessage(err))
transientMsg(extractUserMessage(err));
canSave.current = true;
}
}

async function save(date, isLocked) {
canSave.current = false;

const nodeNote = new NodeNote(noteId, editor.subtype, editor.children, date, isLocked);
await upsertNote(await serializeNote(nodeNote), 'DETAIL');
const serializedNote = await serializeNote(nodeNote);
await upsertNote(serializedNote, 'DETAIL');
setTimeout(async () => {
canSave.current = true;
if (shouldSave.current) {
shouldSave.current = false;
await save(noteDate, isLocked);
try {
canSave.current = true;
if (shouldSave.current) {
shouldSave.current = false;
await save(noteDate, isLocked);
}
} catch (err) {
console.error(`follow-on saving “${shortenTitle(serializedNote.title)}”:`, err);
transientMsg(extractUserMessage(err));
canSave.current = true;
}
}, 1500);
}
Expand Down Expand Up @@ -829,9 +838,15 @@ function Detail({noteId, searchWords = new Set(), focusOnLoadCB, setMustShowPane
if (isLocked) {
noteControls = <>
<div style={{margin: '0 1em'}}>{noteDate.toDateString()}</div>
<IconButton title="Unlock note" size="large" onClick={_evt => {
setIsLocked(false);
save(noteDate, false);
<IconButton title="Unlock note" size="large" onClick={async _evt => {
try {
setIsLocked(false);
await save(noteDate, false);
} catch (err) {
console.error("unlocking:", err);
transientMsg(extractUserMessage(err));
canSave.current = true;
}
}}><Lock/></IconButton>
</>;
} else {
Expand Down Expand Up @@ -888,10 +903,16 @@ function Detail({noteId, searchWords = new Set(), focusOnLoadCB, setMustShowPane
}}>
Flip Table Rows To Columns
</MenuItem>
<MenuItem onClick={_evt => {
setDetailsMenuAnchorEl(null);
setIsLocked(true);
save(noteDate, true);
<MenuItem onClick={async _evt => {
try {
setDetailsMenuAnchorEl(null);
setIsLocked(true);
await save(noteDate, true);
} catch (err) {
console.error("locking:", err);
transientMsg(extractUserMessage(err));
canSave.current = true;
}
}}>
Lock note <Lock/>
</MenuItem>
Expand Down
12 changes: 3 additions & 9 deletions src/FileImport.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {isLikelyMarkdown} from "./util";
import {upsertNote} from "./storage";
import {imageFileToDataUrl} from "./util/imageFileToDataUrl";
import {deserializeNote, serializeNote} from "./serializeNote.js";
import {CONTENT_MAX} from "./Note.js";
import QuietError from "./util/QuietError.js";

function FileImport({files, isMultiple, doCloseImport}) {
const [imports, setImports] = useState([]);
Expand Down Expand Up @@ -469,7 +471,7 @@ async function linesToNote(lines, noteDefaultDateValue, coda, parseType) {
return previousValue + currentString.length;
}, 0);
const isMarkdown = 'text/markdown' === parseType;
if (noteChars > (isMarkdown ? 600_000 : 60_000)) {
if (noteChars > (isMarkdown ? CONTENT_MAX : CONTENT_MAX / 10)) {
throw new Error(`Divide manually before importing`);
}
// last line may or may not be date
Expand Down Expand Up @@ -522,14 +524,6 @@ async function importText(text, fileDateValue, coda, parseType) {
}
}

/** Throwable, but should not be reported to the user */
function QuietError(message) {
this.message = message;
}

QuietError.prototype = Object.create(Error.prototype);
QuietError.prototype.name = "QuietError";


export default FileImport;
export {determineParseType, allowedFileTypesNonText, allowedExtensions, checkForMarkdown, importFromFile};
12 changes: 7 additions & 5 deletions src/FileImport.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import '@testing-library/jest-dom/vitest'
import userEvent from '@testing-library/user-event';
import {validate as uuidValidate} from "uuid";
import {dataURItoFile} from "./util/testUtil";
import {CONTENT_MAX} from "./Note.js";
import {CONTENT_TOO_LONG} from "./Note.js";

describe("checkForMarkdown", () => {
it("should throw for non-file", async () => {
Expand Down Expand Up @@ -110,15 +112,15 @@ describe("importFromFile", () => {
it("should reject an overly-long HTML file", async () => {
const fileDate = '2021-08-15T12:00:00Z';
let html = '<li>${Math.random()}</li>\n';
while (html.length < 600_000) {
while (html.length < CONTENT_MAX) {
html += html;
}
const file = new File(['<ol>', html,'</ol>'], "list.html", {type: 'text/html', lastModified: Date.parse(fileDate)});

const {noteIds, message} = await importFromFile(file, 'text/html', true);
expect(noteIds).toBeInstanceOf(Array);
expect(noteIds.length).toEqual(0);
expect(message).toEqual("Too long. Copy the parts you need.");
expect(message).toEqual(CONTENT_TOO_LONG);
});

it("should parse an empty text file as 0 notes in multiple mode", async () => {
Expand Down Expand Up @@ -386,7 +388,7 @@ Feb 16 00:15:30 frodo spindump[24839]: Removing excessive log: file:///Library/L
let listLines = `1. A thing
2. Some other thing
`;
while (listLines.length < 600_000) {
while (listLines.length < CONTENT_MAX) {
listLines += listLines;
}
let lipsum = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.\n\n';
Expand Down Expand Up @@ -459,15 +461,15 @@ Feb 16 00:15:30 frodo spindump[24839]: Removing excessive log: file:///Library/L
it("should reject an overly-long non-plain text file", async () => {
const fileDate = '2021-05-11T12:00:00Z';
let lines = 'foo,42\n';
while (lines.length < 600_000) {
while (lines.length < CONTENT_MAX) {
lines += lines;
}
const file = new File([lines], "too-long.csv", {type: 'text/csv', lastModified: Date.parse(fileDate)});

const {noteIds, message} = await importFromFile(file, 'text/csv', true);
expect(noteIds).toBeInstanceOf(Array);
expect(noteIds.length).toEqual(0);
expect(message).toEqual("Too long. Copy the parts you need.");
expect(message).toEqual(CONTENT_TOO_LONG);
});

it("should parse a text file as one note when flagged single", async () => {
Expand Down
14 changes: 13 additions & 1 deletion src/Note.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { v4 as uuidv4, validate as uuidValidate } from 'uuid';
import normalizeDate from "./util/normalizeDate.js";

const TITLE_MAX = 400;
const CONTENT_MAX = 600_000;
const CONTENT_TOO_LONG = "Too long. Split this into multiple notes";

/**
* @property {string} id: UUID
Expand Down Expand Up @@ -62,5 +64,15 @@ class SerializedNote {
}
}

function shortenTitle(title) {
const shortened = title?.split("\n")?.[0];
if (!shortened) {
return "«untitled»";
} else if (shortened.length <= 27) {
return shortened;
} else {
return shortened.slice(0, 26) + "…";
}
}

export {TITLE_MAX, NodeNote, SerializedNote};
export {TITLE_MAX, CONTENT_MAX, CONTENT_TOO_LONG, NodeNote, SerializedNote, shortenTitle};
28 changes: 20 additions & 8 deletions src/RemoteNotes.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {validate as uuidValidate} from 'uuid';
import {extractUserMessage} from "./util/extractUserMessage";
import {TAG_LENGTH_MAX} from "./storage";
import normalizeDate from "./util/normalizeDate.js";
import {CONTENT_MAX, CONTENT_TOO_LONG, shortenTitle, TITLE_MAX} from "./Note.js";
import QuietError from "./util/QuietError.js";

const STORE_OBJECT_DELAY = 1000;
const DATE_DEFAULT_REMOTE = new Date(2020, 11, 31, 12, 0);
Expand All @@ -27,12 +29,12 @@ const RemoteNotes = {
"content": { // may contain semantic HTML tags
"type": "string",
"default": "",
"maxLength": 600000 // allows for a data URL of one small raster image
"maxLength": CONTENT_MAX // allows for a couple of data URLs of small raster images
},
"title": {
"type": "string",
"default": "☹",
"maxLength": 400
"maxLength": TITLE_MAX
},
"date": { // RFC 3339, section 5.6 (a subset of ISO 8601)
"type": "string",
Expand Down Expand Up @@ -81,7 +83,7 @@ const RemoteNotes = {
const remoteNote = {
id: serializedNote.id,
...(serializedNote.mimeType && { mimeType: serializedNote.mimeType }),
title: serializedNote.title ?? "[Untitled]",
title: serializedNote.title ?? "«untitled»",
content: serializedNote.content,
date: serializedNote.date.toISOString(),
isLocked: Boolean(serializedNote.isLocked),
Expand All @@ -104,15 +106,25 @@ const RemoteNotes = {
setTimeout(() => {
isBusy[id] = false;
storeQueued().catch(err => {
console.error(`while storing queued (${id}):`, err);
window.postMessage({kind: 'TRANSIENT_MSG',
message: "while storing queued: " + extractUserMessage(err), severity: 'error'},
window?.location?.origin);
if (! (err instanceof QuietError)) {
console.error(`while storing queued (${id}):`, err);
window.postMessage({kind: 'TRANSIENT_MSG',
message: "while storing queued: " + extractUserMessage(err), severity: 'error'},
window?.location?.origin);
}
});
}, STORE_OBJECT_DELAY);
} catch (err) {
isBusy[id] = false;
throw err;
if (201 === err?.error?.code && '/content' === err?.error?.dataPath) {
console.error(`while storing “${shortenTitle(note.title)}”:`, err);
window.postMessage({kind: 'TRANSIENT_MSG',
message: CONTENT_TOO_LONG, severity: 'error'},
window?.location?.origin);
throw new QuietError(CONTENT_TOO_LONG, err?.error);
} else {
throw err;
}
}
return note;
}
Expand Down
34 changes: 33 additions & 1 deletion src/RemoteNotes.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
// Copyright © 2021 Doug Reeder

import generateTestId from "./util/generateTestId";
import {SerializedNote} from "./Note";
import {CONTENT_MAX, CONTENT_TOO_LONG, SerializedNote} from "./Note";
import _ from "fake-indexeddb/auto.js";
import RemoteStorage from "remotestoragejs";
import {RemoteNotes, STORE_OBJECT_DELAY} from "./RemoteNotes";
import {NIL} from "uuid";
import {parseWords, TAG_LENGTH_MAX} from "./storage";
import {deserializeNote, serializeNote} from "./serializeNote.js";
import QuietError from "./util/QuietError.js";


describe("RemoteNotes", () => {
Expand Down Expand Up @@ -160,6 +161,37 @@ describe("RemoteNotes", () => {

});
})

it("should message user & not save when content too large", async () => {
const mockPostMessage = window.postMessage = vitest.fn();
const mockConsoleError = console.error = vitest.fn();

const originalText = "a".repeat(CONTENT_MAX);
const original = new SerializedNote(generateTestId(), undefined, "really long", originalText, new Date(2003, 4, 13), false, []);
await remoteStorage.documents.upsert(original);

let retrieved = await remoteStorage.documents.get(original.id);

expect(retrieved.content).toEqual(originalText); // validates success of upsert
expect(retrieved.title).toEqual(original.title);
expect(retrieved.date).toEqual(original.date);
expect(retrieved.mimeType).toEqual(original.mimeType);
expect(retrieved.isLocked).toEqual(original.isLocked);

await new Promise(resolve => setTimeout(resolve, STORE_OBJECT_DELAY + 100));
const updatedText = originalText + "x";
const updated = new SerializedNote(original.id, original.mimeType, "longer", updatedText, original.date, false, []);

await expect(remoteStorage.documents.upsert(updated)).rejects.toThrow(QuietError);

retrieved = await remoteStorage.documents.get(original.id);
expect(retrieved.content).toEqual(originalText); // not updated
expect(retrieved.title).toEqual(original.title); // not updated
expect(mockPostMessage).toHaveBeenCalledOnce();
expect(mockPostMessage).toHaveBeenCalledWith({kind: 'TRANSIENT_MSG',
message: CONTENT_TOO_LONG, severity: 'error'}, window?.location?.origin);
expect(mockConsoleError).toHaveBeenCalledOnce();
});
});

describe("get", () => {
Expand Down
3 changes: 2 additions & 1 deletion src/fileExport.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {findNoteIds, getNote} from "./storage";
import hasTagsLikeHtml from "./util/hasTagsLikeHtml";
import {deserializeHtml} from "./slateHtml";
import {serializeMarkdown} from "./slateMark";
import {shortenTitle} from "./Note.js";

export async function fileExportMarkdown(searchStr, searchWords) {
console.group("Export to Markdown file")
Expand Down Expand Up @@ -48,7 +49,7 @@ export async function fileExportMarkdown(searchStr, searchWords) {
}

const blob = new Blob([content, "\n", note.date?.toISOString(), "\n\n\n\n"], {type: 'text/plain'});
console.info(`writing ${note.id} ${note.date.toISOString()} ${note.title?.split("\n")?.[0]?.slice(0,85)}`)
console.info(`writing ${note.id} ${note.date.toISOString()} ${shortenTitle(note.title)}`)
await writableStream.write(blob);
++numWritten;
}
Expand Down
11 changes: 10 additions & 1 deletion src/serializeNote.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
// serializeNote.js — converts Slate nodes to content & calculates title if needed
// Copyright © 2023 Doug Reeder

import {NodeNote, SerializedNote, TITLE_MAX} from "./Note.js";
import {CONTENT_TOO_LONG, NodeNote, SerializedNote, shortenTitle, TITLE_MAX} from "./Note.js";
import {deserializeHtml, INLINE_ELEMENTS, serializeHtml} from "./slateHtml.jsx";
import {Node as SlateNode} from "slate";
import {parseWords} from "./storage.js";
import {currentSubstitutions} from "./urlSubstitutions.js";
import hasTagsLikeHtml from "./util/hasTagsLikeHtml.js";
import normalizeDate from "./util/normalizeDate.js";
import {CONTENT_MAX} from "./Note.js";

/**
* Converts Slate nodes to text & extracts keywords
Expand All @@ -25,6 +26,14 @@ async function serializeNote(nodeNote) {
[title, content, wordSet] = serializeNoteText(nodeNote);
}

const limit = nodeNote.subtype?.startsWith('html') || nodeNote.subtype?.startsWith('markdown') ?
CONTENT_MAX : CONTENT_MAX / 10;
if (content.length > limit) {
const err = new Error(`“${shortenTitle(title)}” is too long: ${content.length} characters`);
err.userMsg = CONTENT_TOO_LONG;
throw err;
}

for (let candidateWord of wordSet) {
for (let otherWord of wordSet) {
if (otherWord !== candidateWord && candidateWord.startsWith(otherWord)) {
Expand Down
Loading

0 comments on commit cbea4ee

Please sign in to comment.