Deleting cards that have an error #2056
Conversation
…ere we want to delete the errored card
…t what was passed into it
This is because we want to be able to delete cards that are in error state, and those don't have a CardDef, only the id
Host Test Results 1 files ±0 1 suites ±0 22m 20s ⏱️ +40s Results for commit 8db0daf. ± Comparison against base commit cc54e6e. This pull request removes 2 and adds 4 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
| const stackItem = here.allStackItems.find( | ||
| (item) => item.card === loadedCard, | ||
| ); | ||
| // if is workspace index card, do not allow deletion |
There was a problem hiding this comment.
We're not even showing a button to delete the card when the card is an index card, and that is covered with tests, so this check here is probably not needed.
| @@ -172,27 +175,32 @@ export default class OperatorModeStateService extends Service { | |||
| } | |||
| } | |||
|
|
|||
| async deleteCard(card: CardDef) { | |||
| async deleteCard(cardId: string) { | |||
There was a problem hiding this comment.
the parameter changed from CardDef to a cardId because we want to support deleting a card that has an error, but in this case it is not possible to load the card so we only have its id. Card's id is the lowest common denonimator of a loaded card and the card's error response, so we use that.
| @@ -152,6 +153,7 @@ export class RealmIndexQueryEngine { | |||
| errorDetail: instance.error, | |||
| scopedCssUrls, | |||
| lastKnownGoodHtml: instance.isolatedHtml ?? null, | |||
| cardTitle: instance.searchDoc?.title ?? null, | |||
There was a problem hiding this comment.
I'm adding this so that we can show the card's title in the delete modal (when the card has an error):
This PR:
This needed a refactor in the card deletion logic where previously these functions were accepting a
CardDef–but in case when card has an error we do not actually have aCardDefsince the card cannot be loaded. So we simply pass down the card'sidinstead.