fix(app): a card's edit builds on what it started from
The composer saved a stored card with the card's current heads as its parents, so whatever had arrived meanwhile was replaced, not merged: - another device's change to the side the person left alone was overwritten silently; - an edit made while saving, saved before the card was read back, stood beside the first save: a conflict with itself; - a new card left again before it was read back wasn't written at all, while it showed as saved. A draft now keeps the revisions its content came from (`base`): taken with the stored content while the person hasn't changed it, and its own revision after each save. A save replaces exactly those (`CardRepository.saveEdit`), so what another device wrote meanwhile stays a head of its own and merges per side (ADR 0003): a side only it changed comes through, a side both changed is a conflict to settle. A changed draft takes new conflicts only once it is saved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tizian 「ティツィアーン」 <raisondetredev@pm.me>
This commit is contained in:
parent
079179d2ed
commit
3b10922888
7 changed files with 192 additions and 15 deletions
|
|
@ -102,8 +102,11 @@ class CardComposerBloc(
|
||||||
|
|
||||||
/// The state with the stored [cards]: drafts of cards not yet in it are
|
/// The state with the stored [cards]: drafts of cards not yet in it are
|
||||||
/// added, those gone (deleted, here or elsewhere) dropped, and the others
|
/// added, those gone (deleted, here or elsewhere) dropped, and the others
|
||||||
/// take the stored version as their baseline, and as their content unless
|
/// take the stored version as their baseline. Those the person hasn't
|
||||||
/// the person changed them.
|
/// changed take it whole: content, conflicts, and the revisions it came
|
||||||
|
/// from. A changed one keeps the revisions it started from until it is
|
||||||
|
/// saved, so its save stands beside what another device wrote meanwhile
|
||||||
|
/// and merges with it per side, rather than replacing it unseen.
|
||||||
CardComposerState _withStored(List<Card> cards) {
|
CardComposerState _withStored(List<Card> cards) {
|
||||||
final byId = {for (final draft in state.items) draft.cardId: draft};
|
final byId = {for (final draft in state.items) draft.cardId: draft};
|
||||||
final saved = {
|
final saved = {
|
||||||
|
|
@ -120,14 +123,19 @@ class CardComposerBloc(
|
||||||
cardId: card.id,
|
cardId: card.id,
|
||||||
content: card.content,
|
content: card.content,
|
||||||
saved: card.content,
|
saved: card.content,
|
||||||
|
base: card.heads,
|
||||||
tagIds: tagIds,
|
tagIds: tagIds,
|
||||||
savedTagIds: tagIds,
|
savedTagIds: tagIds,
|
||||||
conflicts: card.conflicts,
|
conflicts: card.conflicts,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
if (draft.isDirty) {
|
||||||
|
return draft.copyWith(saved: card.content);
|
||||||
|
}
|
||||||
return draft.copyWith(
|
return draft.copyWith(
|
||||||
saved: card.content,
|
saved: card.content,
|
||||||
content: draft.isDirty ? null : card.content,
|
content: card.content,
|
||||||
|
base: card.heads,
|
||||||
conflicts: card.conflicts,
|
conflicts: card.conflicts,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
@ -263,15 +271,17 @@ class CardComposerBloc(
|
||||||
_update(emit, draft.id, (d) => d.copyWith(status: CardDraftStatus.saving));
|
_update(emit, draft.id, (d) => d.copyWith(status: CardDraftStatus.saving));
|
||||||
try {
|
try {
|
||||||
final String cardId;
|
final String cardId;
|
||||||
|
var base = draft.base;
|
||||||
if (draft.cardId case final id?) {
|
if (draft.cardId case final id?) {
|
||||||
cardId = id;
|
cardId = id;
|
||||||
final card = _stored[id];
|
// On top of the revisions it started from: all heads once the
|
||||||
if (card != null &&
|
// person chose a version of every field in conflict.
|
||||||
(content != draft.saved.normalized || card.hasConflict)) {
|
if (content != draft.saved.normalized || draft.conflicts.isNotEmpty) {
|
||||||
await _cards.save(card, content);
|
base = {await _cards.saveEdit(id, content, basedOn: draft.base)};
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
cardId = await _cards.create(_studysetId, content);
|
cardId = await _cards.create(_studysetId, content);
|
||||||
|
base = await _cards.headsOf(cardId);
|
||||||
}
|
}
|
||||||
if (!_sameSet(tagIds, draft.savedTagIds)) {
|
if (!_sameSet(tagIds, draft.savedTagIds)) {
|
||||||
await _tags.setTagsOf(cardId, tagIds);
|
await _tags.setTagsOf(cardId, tagIds);
|
||||||
|
|
@ -282,6 +292,8 @@ class CardComposerBloc(
|
||||||
(d) => d.copyWith(
|
(d) => d.copyWith(
|
||||||
cardId: cardId,
|
cardId: cardId,
|
||||||
saved: content,
|
saved: content,
|
||||||
|
// What a change made while saving builds on.
|
||||||
|
base: base,
|
||||||
savedTagIds: tagIds,
|
savedTagIds: tagIds,
|
||||||
resolved: const {},
|
resolved: const {},
|
||||||
savedHere: true,
|
savedHere: true,
|
||||||
|
|
@ -406,6 +418,10 @@ final class const CardDraft({
|
||||||
/// The content as stored; empty for a new card.
|
/// The content as stored; empty for a new card.
|
||||||
required final CardContent saved,
|
required final CardContent saved,
|
||||||
|
|
||||||
|
/// The revisions the person's content started from, which its next save
|
||||||
|
/// replaces; empty for a new card.
|
||||||
|
final Set<String> base = const {},
|
||||||
|
|
||||||
/// The ids of the tags the person gave it.
|
/// The ids of the tags the person gave it.
|
||||||
required final Set<String> tagIds,
|
required final Set<String> tagIds,
|
||||||
|
|
||||||
|
|
@ -453,6 +469,7 @@ final class const CardDraft({
|
||||||
CardDraft copyWith({
|
CardDraft copyWith({
|
||||||
CardContent? content,
|
CardContent? content,
|
||||||
CardContent? saved,
|
CardContent? saved,
|
||||||
|
Set<String>? base,
|
||||||
Set<String>? tagIds,
|
Set<String>? tagIds,
|
||||||
Set<String>? savedTagIds,
|
Set<String>? savedTagIds,
|
||||||
String? cardId,
|
String? cardId,
|
||||||
|
|
@ -464,6 +481,7 @@ final class const CardDraft({
|
||||||
id: id,
|
id: id,
|
||||||
content: content ?? this.content,
|
content: content ?? this.content,
|
||||||
saved: saved ?? this.saved,
|
saved: saved ?? this.saved,
|
||||||
|
base: base ?? this.base,
|
||||||
tagIds: tagIds ?? this.tagIds,
|
tagIds: tagIds ?? this.tagIds,
|
||||||
savedTagIds: savedTagIds ?? this.savedTagIds,
|
savedTagIds: savedTagIds ?? this.savedTagIds,
|
||||||
cardId: cardId ?? this.cardId,
|
cardId: cardId ?? this.cardId,
|
||||||
|
|
@ -478,6 +496,7 @@ final class const CardDraft({
|
||||||
id,
|
id,
|
||||||
content,
|
content,
|
||||||
saved,
|
saved,
|
||||||
|
base,
|
||||||
tagIds,
|
tagIds,
|
||||||
savedTagIds,
|
savedTagIds,
|
||||||
cardId,
|
cardId,
|
||||||
|
|
|
||||||
|
|
@ -44,8 +44,9 @@ abstract interface class CardGateway() {
|
||||||
/// once; returns their ids in the same order. Their positions differ.
|
/// once; returns their ids in the same order. Their positions differ.
|
||||||
Future<List<String>> insertCards(String studysetId, List<NewCardRow> cards);
|
Future<List<String>> insertCards(String studysetId, List<NewCardRow> cards);
|
||||||
|
|
||||||
/// Appends a revision to the card [cardId] that replaces [parents].
|
/// Appends a revision to the card [cardId] that replaces [parents];
|
||||||
Future<void> insertRevision({
|
/// returns its id.
|
||||||
|
Future<String> insertRevision({
|
||||||
required String cardId,
|
required String cardId,
|
||||||
required Set<String> parents,
|
required Set<String> parents,
|
||||||
required ByteData payload,
|
required ByteData payload,
|
||||||
|
|
|
||||||
|
|
@ -184,12 +184,35 @@ class CardRepository(final CardGateway _gateway) {
|
||||||
if (content == card.content && !card.hasConflict) {
|
if (content == card.content && !card.hasConflict) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
await _gateway.insertRevision(
|
await saveEdit(card.id, content, basedOn: card.heads);
|
||||||
cardId: card.id,
|
}
|
||||||
parents: card.heads,
|
|
||||||
|
/// Stores [content] as the next revision of the card [cardId], replacing
|
||||||
|
/// [basedOn]: the revisions the edit started from. What another device
|
||||||
|
/// wrote meanwhile stays a head of its own and merges with this one per
|
||||||
|
/// side (ADR 0003), so it is never replaced unseen. Returns the new
|
||||||
|
/// revision's id, for the next edit to build on.
|
||||||
|
Future<String> saveEdit(
|
||||||
|
String cardId,
|
||||||
|
CardContent content, {
|
||||||
|
required Set<String> basedOn,
|
||||||
|
}) async {
|
||||||
|
final id = await _gateway.insertRevision(
|
||||||
|
cardId: cardId,
|
||||||
|
parents: basedOn,
|
||||||
payload: encodeCardPayload(content),
|
payload: encodeCardPayload(content),
|
||||||
);
|
);
|
||||||
_changes.add(null);
|
_changes.add(null);
|
||||||
|
return id;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The current heads of the card [cardId]: what an edit of it as it is
|
||||||
|
/// now replaces. Empty for a card not on this device.
|
||||||
|
Future<Set<String>> headsOf(String cardId) async {
|
||||||
|
final revisions = [
|
||||||
|
for (final row in await _gateway.revisionsOf({cardId})) ?_decode(row),
|
||||||
|
];
|
||||||
|
return mergeRevisions(revisions)?.heads ?? const {};
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Moves [card] to [toIndex] among the cards of [studysetId].
|
/// Moves [card] to [toIndex] among the cards of [studysetId].
|
||||||
|
|
|
||||||
|
|
@ -117,12 +117,12 @@ class SyncCardGateway(
|
||||||
}
|
}
|
||||||
|
|
||||||
@override
|
@override
|
||||||
Future<void> insertRevision({
|
Future<String> insertRevision({
|
||||||
required String cardId,
|
required String cardId,
|
||||||
required Set<String> parents,
|
required Set<String> parents,
|
||||||
required ByteData payload,
|
required ByteData payload,
|
||||||
}) async {
|
}) async {
|
||||||
await protocol.CardRevision.db.insertRow(
|
final revision = await protocol.CardRevision.db.insertRow(
|
||||||
_db,
|
_db,
|
||||||
protocol.CardRevision(
|
protocol.CardRevision(
|
||||||
cardId: protocol.UuidValue.withValidation(cardId),
|
cardId: protocol.UuidValue.withValidation(cardId),
|
||||||
|
|
@ -135,6 +135,7 @@ class SyncCardGateway(
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
_sync.requestSync();
|
_sync.requestSync();
|
||||||
|
return revision.id!.uuid;
|
||||||
}
|
}
|
||||||
|
|
||||||
@override
|
@override
|
||||||
|
|
|
||||||
|
|
@ -317,6 +317,116 @@ void main() {
|
||||||
);
|
);
|
||||||
expect(state.byId(neko)!.status, CardDraftStatus.clean);
|
expect(state.byId(neko)!.status, CardDraftStatus.clean);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('keeps what another device changed meanwhile in a side the person '
|
||||||
|
'left alone', () async {
|
||||||
|
final neko = await cards.create('japanese', card('猫', 'cat'));
|
||||||
|
start();
|
||||||
|
await loaded();
|
||||||
|
final stored = (await cards.cardsOf('japanese')).single;
|
||||||
|
|
||||||
|
bloc.add(CardComposerEventChanged(neko, card('ねこ', 'cat')));
|
||||||
|
await until((s) => s.byId(neko)?.status == CardDraftStatus.dirty);
|
||||||
|
gateway.receiveRemoteRevision(
|
||||||
|
id: 'rev-9000',
|
||||||
|
cardId: neko,
|
||||||
|
parents: stored.heads,
|
||||||
|
payload: encodeCardPayload(card('猫', 'kitty')),
|
||||||
|
);
|
||||||
|
await until((s) => s.byId(neko)?.saved == card('猫', 'kitty'));
|
||||||
|
bloc.add(CardComposerEventLeft(neko));
|
||||||
|
await until((s) => s.byId(neko)?.content == card('ねこ', 'kitty'));
|
||||||
|
|
||||||
|
final merged = (await cards.cardsOf('japanese')).single;
|
||||||
|
expect(merged.content, card('ねこ', 'kitty'));
|
||||||
|
expect(merged.hasConflict, isFalse);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a side both changed meanwhile becomes a conflict, never '
|
||||||
|
'overwritten', () async {
|
||||||
|
final neko = await cards.create('japanese', card('猫', 'cat'));
|
||||||
|
start();
|
||||||
|
await loaded();
|
||||||
|
final stored = (await cards.cardsOf('japanese')).single;
|
||||||
|
|
||||||
|
bloc.add(CardComposerEventChanged(neko, card('猫', 'a cat')));
|
||||||
|
await until((s) => s.byId(neko)?.status == CardDraftStatus.dirty);
|
||||||
|
gateway.receiveRemoteRevision(
|
||||||
|
id: 'rev-9000',
|
||||||
|
cardId: neko,
|
||||||
|
parents: stored.heads,
|
||||||
|
payload: encodeCardPayload(card('猫', 'kitty')),
|
||||||
|
);
|
||||||
|
await until((s) => s.byId(neko)?.saved == card('猫', 'kitty'));
|
||||||
|
bloc.add(CardComposerEventLeft(neko));
|
||||||
|
final conflicted = await until(
|
||||||
|
(s) => s.byId(neko)?.conflicts.isNotEmpty ?? false,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(conflicted.byId(neko)!.conflicts, {
|
||||||
|
CardField.back: [text('a cat'), text('kitty')],
|
||||||
|
});
|
||||||
|
expect(
|
||||||
|
(await cards.cardsOf('japanese')).single.conflicts[CardField.back],
|
||||||
|
[text('a cat'), text('kitty')],
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('builds an edit made while saving on that save, not beside '
|
||||||
|
'it', () async {
|
||||||
|
final neko = await cards.create('japanese', card('猫', 'cat'));
|
||||||
|
start();
|
||||||
|
await loaded();
|
||||||
|
|
||||||
|
bloc
|
||||||
|
..add(CardComposerEventChanged(neko, card('猫', 'a cat')))
|
||||||
|
..add(CardComposerEventLeft(neko))
|
||||||
|
..add(CardComposerEventChanged(neko, card('猫', 'the cat')));
|
||||||
|
await until(
|
||||||
|
(s) =>
|
||||||
|
s.byId(neko)?.savedHere == true &&
|
||||||
|
s.byId(neko)?.status == CardDraftStatus.dirty,
|
||||||
|
);
|
||||||
|
bloc.add(CardComposerEventLeft(neko));
|
||||||
|
await until(
|
||||||
|
(s) =>
|
||||||
|
s.byId(neko)?.status == CardDraftStatus.saved &&
|
||||||
|
s.byId(neko)?.saved == card('猫', 'the cat'),
|
||||||
|
);
|
||||||
|
|
||||||
|
final stored = (await cards.cardsOf('japanese')).single;
|
||||||
|
expect(stored.hasConflict, isFalse);
|
||||||
|
expect(stored.heads, hasLength(1));
|
||||||
|
expect(stored.content, card('猫', 'the cat'));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('builds an edit made while a new card is created on that '
|
||||||
|
'card', () async {
|
||||||
|
start();
|
||||||
|
await loaded();
|
||||||
|
|
||||||
|
bloc
|
||||||
|
..add(const CardComposerEventDraftAdded('new'))
|
||||||
|
..add(CardComposerEventChanged('new', card('犬', 'dog')))
|
||||||
|
..add(const CardComposerEventLeft('new'))
|
||||||
|
..add(CardComposerEventChanged('new', card('犬', 'a dog')));
|
||||||
|
await until(
|
||||||
|
(s) =>
|
||||||
|
s.byId('new')?.savedHere == true &&
|
||||||
|
s.byId('new')?.status == CardDraftStatus.dirty,
|
||||||
|
);
|
||||||
|
bloc.add(const CardComposerEventLeft('new'));
|
||||||
|
await until(
|
||||||
|
(s) =>
|
||||||
|
s.byId('new')?.status == CardDraftStatus.saved &&
|
||||||
|
s.byId('new')?.saved == card('犬', 'a dog'),
|
||||||
|
);
|
||||||
|
|
||||||
|
final stored = (await cards.cardsOf('japanese')).single;
|
||||||
|
expect(stored.hasConflict, isFalse);
|
||||||
|
expect(stored.heads, hasLength(1));
|
||||||
|
expect(stored.content, card('犬', 'a dog'));
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
test('a duplicate copies content and tags as a new card', () async {
|
test('a duplicate copies content and tags as a new card', () async {
|
||||||
|
|
|
||||||
|
|
@ -150,6 +150,28 @@ void main() {
|
||||||
expect(gateway.written.last.parents, conflicted.heads);
|
expect(gateway.written.last.parents, conflicted.heads);
|
||||||
expect(conflicted.heads, hasLength(2));
|
expect(conflicted.heads, hasLength(2));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('an edit saved on what it started from stands beside what came '
|
||||||
|
'meanwhile, and merges with it', () async {
|
||||||
|
remoteEdit(card('猫', 'a cat'));
|
||||||
|
|
||||||
|
final id = await repository.saveEdit(
|
||||||
|
created.id,
|
||||||
|
card('ねこ', 'cat'),
|
||||||
|
basedOn: created.heads,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(gateway.written.last.id, id);
|
||||||
|
expect(gateway.written.last.parents, created.heads);
|
||||||
|
final merged = (await repository.cardsOf('japanese')).single;
|
||||||
|
expect(merged.content, card('ねこ', 'a cat'));
|
||||||
|
expect(merged.heads, {id, 'rev-9000'});
|
||||||
|
expect(await repository.headsOf(created.id), {id, 'rev-9000'});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a card not on this device has no heads', () async {
|
||||||
|
expect(await repository.headsOf('card-9999'), isEmpty);
|
||||||
});
|
});
|
||||||
|
|
||||||
test(
|
test(
|
||||||
|
|
|
||||||
|
|
@ -100,7 +100,7 @@ class FakeCardGateway() implements CardGateway {
|
||||||
}
|
}
|
||||||
|
|
||||||
@override
|
@override
|
||||||
Future<void> insertRevision({
|
Future<String> insertRevision({
|
||||||
required String cardId,
|
required String cardId,
|
||||||
required Set<String> parents,
|
required Set<String> parents,
|
||||||
required ByteData payload,
|
required ByteData payload,
|
||||||
|
|
@ -116,6 +116,7 @@ class FakeCardGateway() implements CardGateway {
|
||||||
);
|
);
|
||||||
_revisions[revision.id] = revision;
|
_revisions[revision.id] = revision;
|
||||||
written.add(revision);
|
written.add(revision);
|
||||||
|
return revision.id;
|
||||||
}
|
}
|
||||||
|
|
||||||
@override
|
@override
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue