Add a new `did_add_note_to_collection` hook?

Why

I’m making an addon that can modify other notes and do other collection-modifying side effects when a new note is added through

  1. the Add cards dialog or
  2. by another addon calling col.add_note(), e.g. AnkiConnect.

It would be desirable that you could only undo the side effects of adding the new note while keeping the note or undo the side effects together with undoing the note addition. So, I’d want to either merge this operation into the “Add Note” undo entry or make a custom entry after that.

Problems with existing hooks

add_cards_did_add_note

Because of wanting AnkiConnect to also trigger these side effects, using this is not an option.

note_will_be_added

In order of significance:

  1. Creating a custom undo entry in note_will_be_added results in that entry being before the “Add Note” undo entry.
    • Because of this it’s not possible to undo the side effects without also undoing the note addition
    • Also, when one undoes the note addition, it’s pretty easy to forget to undo the side effects. This leaves the side effects without the new note…
  2. note.id doesn’t exist so it can’t be used in the side effect
  3. Cards don’t exist yet, so can’t make any edits to those or use their card.id either.

note_will_flush

This actually got called after the note was added and seemed it was possible to make a hack to defer the the undoable side effects to note_will_flush but then I found that this behaviour only occurs when using the Add Cards dialog and not when AnkiConnect calls col.add_note(). And the comment on this hook does say

Allow to change a note before it is added/updated in the database.

So, of course it doesn’t work here.

New hook?

So, any objections If I try to PR a new hook did_add_note_to_collection that’d be run after _to_backend_note has finished in collection.py. Like below, I suppose?

    def add_note(self, note: Note, deck_id: DeckId) -> OpChanges:
        hooks.note_will_be_added(self, note, deck_id)
        out = self._backend.add_note(note=note._to_backend_note(), deck_id=deck_id)
        note.id = NoteId(out.note_id)
        # --------NEW HOOK---------
        hooks.did_add_note_to_collection(self, note)
        # /-------NEW HOOK---------
        return out.changes

    def add_notes(self, requests: Iterable[AddNoteRequest]) -> OpChanges:
        for request in requests:
            hooks.note_will_be_added(self, request.note, request.deck_id)
        out = self._backend.add_notes(
            requests=[
                notes_pb2.AddNoteRequest(
                    note=request.note._to_backend_note(), deck_id=request.deck_id
                )
                for request in requests
            ]
        )
        for idx, request in enumerate(requests):
            request.note.id = NoteId(out.nids[idx])
            # --------NEW HOOK---------
            hooks.did_add_note_to_collection(self, request.note)
            # /-------NEW HOOK---------
        return out.changes
3 Likes

See also conversation in rejected Create notes_were_updated hook pull request

4 Likes

Thanks for that link. I too hadn’t considered new notes coming in from sync. I’ll indeed need to handle that for the case when a note is added on mobile or AnkiWeb. I think I’ll handle that either with a note tag or a custom data flag on the cards and identify new cards from non-desktop sources through their absence.

I think the potential did_add_note hook should not be run during sync as you could get addons running ops twice if a user has two desktop Anki installations both running the same addon. It would make sense to run it on import though and that seems to be more doable than making note_will_be_added get run on import.

I’m afraid I still don’t think it’s practical. You can’t be calling such a hook when importing, as that happens at a lower level. And it won’t help when add-ons directly modify the DB.

1 Like

@dae , I thought about creating a SQLite trigger for insert/update/delete operations on notes table. The trigger will put modified notes into another new table that will be monitored by the addon.
What do you think about this approach?

Modifying the database schema in a persistent way introduces a lot more ways things can go wrong, and I can’t guarantee AnkiWeb would continue to accept such files in the future. I’d recommend you use a sidecar file to store large amounts of data like note id maps, and periodically scan for notes that have been modified more recently.

1 Like

I guess I’ll work around the issue by implementing a system to identify new notes on sync that have not had side-effects run on them yet. That’ll work for imported notes. This is necessary in any case to handle notes added on devices that don’t have the addon.