From 6522ed903cc2c4652a22e2eb517c390c26097ed0 Mon Sep 17 00:00:00 2001
From: Alex Komoroske <jkomoros@gmail.com>
Date: Sat, 16 Dec 2023 16:38:35 -0800
Subject: [PATCH] Move where cardFinsihers and fontBoosts are done to be within
 modifyCardWithBatch.

Before, the editor was responsible for running it. But that meant it was possible to have cards who wouldn't have their finishers run.

For example, a suggestor to convert to quote card wouldn't run the finisher, so the title would remain with the old title.

But now, everything right before getting modified gets the final finsiher.

Part of #465.
---
 src/actions/data.ts   | 11 ++++++++---
 src/actions/editor.ts | 10 +++++-----
 src/card_diff.ts      | 11 ++++++-----
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/actions/data.ts b/src/actions/data.ts
index 3b2ba68c..5e4c0fd4 100644
--- a/src/actions/data.ts
+++ b/src/actions/data.ts
@@ -124,6 +124,7 @@ import {
 	applyCardDiff,
 	applyCardFirebaseUpdate,
 	inboundLinksUpdates,
+	generateFinalCardDiff,
 } from '../card_diff.js';
 
 import {
@@ -268,7 +269,8 @@ export const modifyCardsIndividually = (cards : Card[], updates : {[id : CardID]
 		if (!update) continue;
 
 		try {
-			if (modifyCardWithBatch(state, card, update, substantive, batch)) modifiedCount++;
+			const bool = await modifyCardWithBatch(state, card, update, substantive, batch);
+			if (bool) modifiedCount++;
 		} catch (err) {
 			console.warn('Couldn\'t modify card: ' + err);
 			errorCount++;
@@ -293,11 +295,11 @@ export const modifyCardsIndividually = (cards : Card[], updates : {[id : CardID]
 
 //returns true if a modificatioon was made to the card, or false if it was a no
 //op. When an error is thrown, that's an implied 'false'.
-export const modifyCardWithBatch = (state : State, card : Card, update : CardDiff, substantive : boolean, batch : MultiBatch) : boolean => {
+export const modifyCardWithBatch = async (state : State, card : Card, rawUpdate : CardDiff, substantive : boolean, batch : MultiBatch) : Promise<boolean> => {
 
 	//If there aren't any updates to a card, that's OK. This might happen in a
 	//multiModify where some cards already have the items, for example.
-	if (!cardDiffHasChanges(update)) return false;
+	if (!cardDiffHasChanges(rawUpdate)) return false;
 
 	const user = selectUser(state);
 
@@ -309,6 +311,9 @@ export const modifyCardWithBatch = (state : State, card : Card, update : CardDif
 		throw new Error('User isn\'t allowed to edit the given card');
 	}
 
+	//This is where cardFinishers and fontSizeBoosts are actually applied.
+	const update = await generateFinalCardDiff(state, card, rawUpdate);
+
 	const updateObject = {
 		...update,
 		batch: batch.batchID || '',
diff --git a/src/actions/editor.ts b/src/actions/editor.ts
index 5a7ea816..d064ebc3 100644
--- a/src/actions/editor.ts
+++ b/src/actions/editor.ts
@@ -24,10 +24,10 @@ import {
 } from './data.js';
 
 import {
-	generateFinalCardDiff,
 	confirmationsForCardDiff,
 	cardDiffHasChanges,
-	cardDiffDescription
+	cardDiffDescription,
+	generateCardDiff
 } from '../card_diff.js';
 
 import {
@@ -312,7 +312,7 @@ export const editingCommit = () : ThunkSomeAction => async (dispatch, getState)
 
 	let update : CardDiff;
 	try {
-		update = await generateFinalCardDiff(state, underlyingCard, rawUpdatedCard);
+		update = await generateCardDiff(underlyingCard, rawUpdatedCard);
 	} catch(err) {
 		alert(err);
 		return;
@@ -320,8 +320,8 @@ export const editingCommit = () : ThunkSomeAction => async (dispatch, getState)
 
 	if (!update) return;
 
-	//TODO: technically we shouldn't pass rawUpdatedCard, but the one that has
-	//been run through any cardFinishers in generateFinalCardDiff.
+	//Note: cardFinishers are run later; so they will not get any confirmations.
+	//But they;'re mostly used for setting computed fields.
 	if (!confirmationsForCardDiff(update, rawUpdatedCard)) return;
 
 	//modifyCard will fail if the update is a no-op.
diff --git a/src/card_diff.ts b/src/card_diff.ts
index 00376fd4..7b29298e 100644
--- a/src/card_diff.ts
+++ b/src/card_diff.ts
@@ -473,13 +473,14 @@ export const overshadowedDiffChanges = (original : Card | null, snapshot : Card
 	return Object.fromEntries(TypedObject.entries(currentDiff).filter(entry => !NON_AUTOMATIC_MERGE_FIELDS[entry[0]] && snapshotDiff[entry[0]] !== undefined));
 };
 
-//generateFinalCardDiff is like generateCardDiff but also handles fields set by cardFinishers and font size boosts.
-export const generateFinalCardDiff = async (state : State, underlyingCard : Card, rawUpdatedCard : Card) : Promise<CardDiff> => {
+//generateFinalCardDiff is like generateCardDiff but also handles fields set by
+//cardFinishers and font size boosts. modifyCardWithBatch runs it at the very
+//last moment to see if anything different happens.
+export const generateFinalCardDiff = async (state : State, underlyingCard : Card, diff : CardDiff) : Promise<CardDiff> => {
 
-	const cardFinisher = CARD_TYPE_EDITING_FINISHERS[rawUpdatedCard.card_type];
+	const updatedCard = cardFromDiff(underlyingCard, diff);
 
-	//updatedCard is a copy so may be modified
-	const updatedCard = {...rawUpdatedCard};
+	const cardFinisher = CARD_TYPE_EDITING_FINISHERS[updatedCard.card_type];
 
 	try {
 		if(cardFinisher) cardFinisher(updatedCard, state);