Skip to content

Commit

Permalink
Move where cardFinsihers and fontBoosts are done to be within modifyC…
Browse files Browse the repository at this point in the history
…ardWithBatch.

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.
  • Loading branch information
jkomoros committed Dec 17, 2023
1 parent dabae77 commit 6522ed9
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 13 deletions.
11 changes: 8 additions & 3 deletions src/actions/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ import {
applyCardDiff,
applyCardFirebaseUpdate,
inboundLinksUpdates,
generateFinalCardDiff,
} from '../card_diff.js';

import {
Expand Down Expand Up @@ -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++;
Expand All @@ -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);

Expand All @@ -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 || '',
Expand Down
10 changes: 5 additions & 5 deletions src/actions/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ import {
} from './data.js';

import {
generateFinalCardDiff,
confirmationsForCardDiff,
cardDiffHasChanges,
cardDiffDescription
cardDiffDescription,
generateCardDiff
} from '../card_diff.js';

import {
Expand Down Expand Up @@ -312,16 +312,16 @@ 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;
}

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.
Expand Down
11 changes: 6 additions & 5 deletions src/card_diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 6522ed9

Please sign in to comment.