Send updates when NoteID is populated

[benjamin-weller]

Jason can you please take a look at generally what I'm thinking about doing and give me a LGTM or not. I just don't want to get going on these changes and be barking up the wrong tree.

Continuation of PR 92.

[jasonwilliams]

Hey @benjamin-weller sorry for being slow to look at this. I've been away i will try and take a look this week

[benjamin-weller]

SG, no worries. I'm relatively green WRT to JavaScript async + await, so I think that's likely where I could have missed something.

Additionally how would you suggest I write unit tests for these changes?

[jasonwilliams]

Yeah so far looks good. Just so I understand, we don't have the code that handles adding the noteID into the markdown just yet right?

So far I see support for parsing the note and updating/instead of pushing if there is a noteid (this PR)

For tests: I would say do the best you can, but don't worry too much about it. Im happy to help out on tests, I may try and migrate to vitest soon anyway so if you're not up to it I can change and add later.

[benjamin-weller]

Just so I understand, we don't have the code that handles adding the noteID into the markdown just yet right?

Yes I have some "working" code for this on my computer. I want to roll this change out first because I perceive that change to be higher risk/would clog up the review for this change.

WRT tests, I manually tested. Idk how exactly I would mock updating a card in Anki here? Unless you have a good idea now on how to do that I'm leaning towards not implementing them for this feature. That won't be the case for the next one (updating the markdown) as that's higher risk imo.

Proof of manual testing (works on my machine):

Deck Prior to run:

Content of MD file:

# Deck Name Is Definately Not Scratch

## NEW NAME IS DEFINATELY NOT YOLO

<!-- notecardId: 1679716453114 -->

New back side of card

Ran: Send To Own Deck

Deck post run:

I want to point out that my changes allow you to update a notecard regardless of which deck it exists in. This may feel like a bug at first, but it's what I want, my use case; I may relocate an entire subsection of markdown document, but that doesn't mean when I re-push to Anki that I want to re-create new cards, I just want to know that the particular cards is up to date.

[jasonwilliams]

I want to point out that my changes allow you to update a notecard regardless of which deck it exists in. This may feel like a bug at first, but it's what I want, my use case; I may relocate an entire subsection of markdown document, but that doesn't mean when I re-push to Anki that I want to re-create new cards, I just want to know that the particular cards is up to date.

sure this sounds fair, but it may conflict with the “Send dir to Anki” command. Which sends cards to Anki based on their directory location. If I have noteids on the cards and move them which operation wins out?

We may want to think about and how we respond to it

[mkraenz]

Hi, awesome job! Updates are the one feature I've been missing, so very happy this is getting tackled. Thank you so much!

In eager awaiting of this, I've written a script to update an existing markdown file (without note ids) with the note ids from Anki, in a similar format as https://github.com/jasonwilliams/anki/pull/106#issuecomment-1483713243 does (though I'm using <!-- noteId: 1636220174001 --> for now).

You can find it in this gist https://gist.github.com/mkraenz/a7e0473ca80c07522ff75dc2fd9e52ba

Maybe it becomes a reference for you or somebody else. In particular, the sanitization of the card front is not trivial. Note that I do assume a nicely linted markdown file.

The summary at the end looks like this for 526 cards with backticks, question marks, parentheses in the card front all being gracefully handled. Only one of the cards cannot be resolved to its note id.

@@@@@@@@@@@@@@@@@@@@@@@ DONE @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
OVERWRITTEN FILE ON DISK:  /home/mirco/Desktop/anki.test.md.withNoteids.md 

Total number of cards processed:  526
Number of successfully written note ids for cards:  525
Number of missing node ids for cards:  1
MISSING NOTE_IDS FOR CARDS:
 linux `$RANDOM`

PS: Sorry for randomly dropping this into this PR.

[benjamin-weller]

I want to point out that my changes allow you to update a notecard regardless of which deck it exists in. This may feel like a bug at first, but it's what I want, my use case; I may relocate an entire subsection of markdown document, but that doesn't mean when I re-push to Anki that I want to re-create new cards, I just want to know that the particular cards is up to date.

sure this sounds fair, but it may conflict with the “Send dir to Anki” command. Which sends cards to Anki based on their directory location. If I have noteids on the cards and move them which operation wins out?

We may want to think about and how we respond to it

I'm not certain exactly which one wins out tbh. I'm willing to feature flag this (only have cards be updated if the user opts in), this ensures that they explicitly agree that things might not be 100% compatible. Is that fair for you?

[benjamin-weller]

Hi, awesome job! Updates are the one feature I've been missing, so very happy this is getting tackled. Thank you so much!

In eager awaiting of this, I've written a script to update an existing markdown file (without note ids) with the note ids from Anki, in a similar format as #106 (comment) does (though I'm using <!-- noteId: 1636220174001 --> for now).

You can find it in this gist https://gist.github.com/mkraenz/a7e0473ca80c07522ff75dc2fd9e52ba

Maybe it becomes a reference for you or somebody else. In particular, the sanitization of the card front is not trivial. Note that I do assume a nicely linted markdown file.

The summary at the end looks like this for 526 cards with backticks, question marks, parentheses in the card front all being gracefully handled. Only one of the cards cannot be resolved to its note id.

@mkraenz It's great to hear that hopefully this feature will have users!

Yes the card sanitation is currently the most tricky part, I assume that the text of the header is equivalent to the text of the front of the card (I ask the HTML for the inner content but even then there can be stuff like <strong> tags which interfere with string matching). I'll take a closer look at your implementation, I'm really hoping that I can find a NPM package that will just simply reverse engineer a given HTML string to markdown.

[jasonwilliams]

I'm not certain exactly which one wins out tbh. I'm willing to feature flag this (only have cards be updated if the user opts in), this ensures that they explicitly agree that things might not be 100% compatible. Is that fair for you?

Ok lets feature flag it for now. I also think at some point in the future we could have a command that re-moves cards based on their new position in the directory (assuming you're using the send to dir feature). This would mitigate the issue we face of moving cards then re-updating to somewhere else.

[benjamin-weller]

Is there anything else I need to do such that these changes are ready to merge?

[jasonwilliams]

@benjamin-weller how has this been for you? Have you been testing it? I guess we need a commit which adds the noteID back into the markdown

[benjamin-weller]

Hey, I've created a new PR, let's talk there.

[Spirarel]

I'm not certain exactly which one wins out tbh. I'm willing to feature flag this (only have cards be updated if the user opts in), this ensures that they explicitly agree that things might not be 100% compatible. Is that fair for you?

Ok lets feature flag it for now. I also think at some point in the future we could have a command that re-moves cards based on their new position in the directory (assuming you're using the send to dir feature). This would mitigate the issue we face of moving cards then re-updating to somewhere else.

Are you guys approaching a release soon or do we need to install from main?