bandantonio / obsidian-apple-books-highlights-plugin

Import highlights and notes from your Apple Books to Obsidian
https://obsidian.md/plugins?id=apple-books-import-highlights
MIT License
53 stars 1 forks source link

[Enhancement]: When backups are disabled, content overwrite during subsequent imports is not obvious #34

Closed JeffreyGH10 closed 3 months ago

JeffreyGH10 commented 4 months ago

Bug Report Checklist

Plugin version

1.3.0

Obsidian version

1.6.5

macOS version

macOS Monterey

What happened?

Hello bandantonio, I encountered an issue while using the plugin. For example, when I choose to import the highlights from book "A," the import is successful, and the import file for "A" appears in the folder. However, when I subsequently choose to import the highlights from book "B," the import is successful, and the import file for "B" appears in the folder, but the import file for "A" disappears. I have repeated the test several times, and the same issue occurs every time. I hope you can check whether this is a bug. Thank you.

The issue occurs when

Importing a single book

How many books do you have in your Apple Books library?

20 or less

Code of Conduct

bandantonio commented 4 months ago

@JeffreyGH10 Thank you for the report. According to what you said, I believe that the Backup highlights setting is disabled in your configuration. In this case, this is the expected behavior. Since the option is disabled, you're giving the plugin your consent to overwrite contents of the Highlights folder before each import. This flow is the same for both types of import. From my perspective, it's not a bug, but it's worth making the import flows a bit more straightforward.

Just to confirm that we're on the same page - your expectation was to have highlights from books A and B to be saved side-by-side in the Highlights folder?

JeffreyGH10 commented 4 months ago

@bandantonio Thank you for your response. I understand what you said. I indeed did not enable the "Backup highlights" feature, which is why the later imported file overwrites the previous one. Personally, I hope that both files A and B can exist simultaneously in the folder. From the perspective of an ordinary user, a file should only be deleted when the user executes the delete command. Alternatively, if overwriting is to be performed, could there be a popup message saying, "The new imported file will overwrite the existing file. Are you sure you want to proceed?" This way, users would not perceive it as a bug. Regardless, I am very grateful for the plugin you developed, which has made organizing my reading notes much more convenient.

bandantonio commented 4 months ago

@JeffreyGH10 Thanks for your response. Indeed, the import logic should be improved.

Alternatively, if overwriting is to be performed, could there be a popup message saying, "The new imported file will overwrite the existing file. Are you sure you want to proceed?"

Absolutely, this was the first thing that came to mind, so it will definitely be added once I refactor the import logic.

I am very grateful for the plugin you developed, which has made organizing my reading notes much more convenient.

🩷 Thank you for choosing my plugin, I'm grateful for your feedback and happy to make the plugin even better!

bandantonio commented 4 months ago

@bandantonio (Self-reminder) Action points to keep in mind:

mbarritt commented 3 months ago

bandantonio,

I'm glad to have just found your Apple Books importer plugin and have begun testing it for daily use (re-running it many times as I experiment with a custom template). I want to follow up on the issue @JeffreyGH10 raised here.

I have 70 highlighted books I'm testing with, the full imports work well. However if/when I add highlights to a book already in the collection I assumed, given the "Import highlights from a specific book..." feature, that I could import only that book/note file again, expecting the plugin to overwrite just that one book note file. Unfortunately, as an expansion of the example @JeffreyGH10 pointed out above, all 69 of the other highlight note files are removed to be replaced by just the one I am re-importing.

Providing an "Import highlights from a specific book..." option as you have, suggests to me that only that file would be overwritten (if it already existed in an earlier version). What was your intent for this feature? If it always replaces the entire collection of note files with just that one it seems...well problematic. I'd very much like it to continue to be available, but where it writes over only the note file for the book selected, leaving the rest of the library of reading notes (highlights) intact. I'd use it actively when I go back to a previously read book with additional notes/highlights to update just that entry in Obsidian (or when finishing a book that had only been partially read and highlighted previously though it was already in the Apple Highlights imported directory). Given that existing note files have (can have, maybe required to have) the assetid of that particular book in their metadata area, would it be possible to reliably confirm the identify of only the one, previously existing import file (from matching both the book-title-generated file name and the assetid), and write over just it?

Of course, for now, I can re-import and write over the entire directory of books notes with each import. Your plug-in runs quickly enough that this works well. Any book notes I wish to add to inside Obsidian (adding additional notes not present in Apple Books) would have to be moved out of the Apple Books Highlights folder to avoid being overwritten (which is also a requirement with any of the other methods I've found so far for importing Apple Books highlights).

P.S. I've used three or four other methods of keeping my Apple Books highlights and notes updated in Obsidian over the past few years. I'm moving to your plugin, it seems to be the best (fast, dependable, with a user editable template(!!!), and hopefully with selective single book importing/overwriting). Thanks for creating and sharing it!

bandantonio commented 3 months ago

@mbarritt Thank you so much for the detailed explanation of your workflow!

I understand your frustration, a single book import is not perfect indeed. For the last couple of weeks, I've been playing around with it without the backup feature enabled. Eventually, I wasn't satisfied with the overall experience, mainly because I stumbled upon the same use case you mentioned (reflecting on the previously read/partially read books). And when @JeffreyGH10 raised the issue, it became clear that the import logic needed to be improved.

Your suggestion to rely on some assetId is interesting and totally makes sense but as long as all the variables are optional, I would rather think of passing that asset id as a comment, like <!-- serviceData-: assetId --> as a workaround in order not to force users to always define a specifically-formatted variable in their templates. This approach, potentially has a performance impact, as searching through a large collection may be slow, but as we're talking about a single book import, it shouldn't be a big deal.

P.S. I've used three or four other methods of keeping my Apple Books highlights and notes updated in Obsidian over the past few years. I'm moving to your plugin, it seems to be the best (fast, dependable, with a user editable template(!!!), and hopefully with selective single book importing/overwriting). Thanks for creating and sharing it!

🩷 Thank you for choosing my plugin, I'm so happy to hear that you found it useful and it has all the features you were looking for! Frankly speaking, this exact list of features motivated me to create this plugin because when I was exploring the existing solutions, all the plugins failed to work the way I wanted them to and didn't have all the features I expected. So, I had no other choice left 😀

mbarritt commented 3 months ago

@bandantonio, Glad these comments seemed on the mark and are helpful.

I had the same thoughts about my suggestion regarding a performance hit and it being less than ideal to require/hardwire in an assetid variable in the template. As you point out, with just one file to match hopefully not too much of a performance hit (and if the routine can start out going directly to a file with the same derived-from-the-book-title file name and use the assetId--wherever it is stored--as a confirmation, if that's even necessary, hopefully not much of a hit).

If you pass the assetId as a Finder comment is there a danger it is removed (by the user, or some other process) later? I have this vague memory from the early days of discussions about Macintosh file tagging that the comment on a file is not a reliable place to store critical info? (Vague memory, and not my speciality at all, but it comes to mind.) But...as the assetId might be only a second factor-like confirmation, perhaps not such a big deal If it was missing on the rare occasion? You might pop-up a dialog identifying the note file that was about to be replaced and ask for user confirmation (or, perhaps, do that regardless?).

Glad to hear the features I noticed in your plug-in were in your design goals. Good to hear you're managing to create what you intended, and that we all get to benefit. Thanks!

(Once I have my import template nailed down I'll share it in the "Showcase Your Templates" discussion you created.)

bandantonio commented 3 months ago

@mbarritt

to a file with the same derived-from-the-book-title file name

File names can also be changed after the import, we can't expect them to stay the same.

If you pass the assetId as a Finder comment is there a danger it is removed (by the user, or some other process) later?

Yeah, there is a certain risk of it, for sure. But as I'm expecting this attribute to be generated automatically when creating highlights files, even if the user deletes the attribute, they can "regenerate" it by re-importing the corresponding file.

the comment on a file is not a reliable place to store critical info

Not the best one indeed, but technically, the default template has no required parameters, so you may, for example, create highlights files with only {{{note}}} content, for example. The "problem" is that the plugin is stateless and it knows nothing about the already imported files and how to associate them with the data from the database. So we need to introduce this link.

Another problem is that there is no fixed field we can 100% rely on, and still, I don't want to explicitly force the users to use one. It's easier to use such a field implicitly. From this point of view, data-comments seem to be a reasonable workaround and evil that we can (probably) proceed with. But that's just my assumption and my thoughts aloud. Still brainstorming it.

You might pop-up a dialog identifying the note file that was about to be replaced and ask for user confirmation

Pop-up dialog is a must, that's for sure. I've already mentioned it in my action points. But before showing that pop-up, we need to know what associates with what.

mbarritt commented 3 months ago

@bandantonio

On Jul 31, 2024, at 1:41 PM, Antonio @.***> wrote:

@mbarritt https://github.com/mbarritt to a file with the same derived-from-the-book-title file name

File names can also be changed after the import, we can't expect them to stay the same.

Last thought for now. If a user changed a file name and then re-imported the highlights/notes for that book at least it would not be destructive, the prior version would not be matched and would not be overwritten. A little messy (two highlights/notes files for that book in the directory with slightly different names), but a low cost error. I suppose it is even possible that a user would change a file name intentionally to preserve it as it is (a snapshot of a sort). I can imagine putting the date at the end of the file name or some such to, in effect, freeze it in place. (Not sure of the use case for this, but it comes to mind and is the kind of archiving/snapshot I sometimes do).

Look forward to using the solution you come up with. Thanks.

If you pass the assetId as a Finder comment is there a danger it is removed (by the user, or some other process) later?

Yeah, there is a certain risk of it, for sure. But as I'm expecting this attribute to be generated automatically when creating highlights files, even if the user deletes the attribute, they can "regenerate" it by re-importing the corresponding file.

the comment on a file is not a reliable place to store critical info

Not the best one indeed, but technically, the default template has no required parameters, so you may, for example, create highlights files with only {{{note}}} content, for example. The "problem" is that the plugin is stateless and it knows nothing about the already imported files and how to associate them with the data from the database. So we need to introduce this link.

Another problem is that there is no fixed field we can 100% rely on, and still, I don't want to explicitly force the users to use one. It's easier to use such a field implicitly. From this point of view, data-comments seem to be a reasonable workaround and evil that we can (probably) proceed with. But that's just my assumption and my thoughts aloud. Still brainstorming it.

You might pop-up a dialog identifying the note file that was about to be replaced and ask for user confirmation

Pop-up dialog is a must, that's for sure. I've already mentioned https://github.com/bandantonio/obsidian-apple-books-highlights-plugin/issues/34#issuecomment-2231429171 it in my action points. But before showing that pop-up, we need to know what associates with what.

— Reply to this email directly, view it on GitHub https://github.com/bandantonio/obsidian-apple-books-highlights-plugin/issues/34#issuecomment-2261033604, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVQL7PYBAHCCU2ZKAMMBBPDZPEOVBAVCNFSM6AAAAABK4UBGSGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRRGAZTGNRQGQ. You are receiving this because you were mentioned.

bandantonio commented 3 months ago

Hi @JeffreyGH10, @mbarritt, I have some updates regarding this feature. I'm finalizing the refactoring process for the import logic, just wanted to align with you to ensure that the updated logic makes sense to you.

So, this is what I've come up so far:

When backups are disabled:

When backups are enabled:

The same logic applies to bulk import, so the most "destructive" case is importing all books when backups are disabled. The will be the corresponding popup:

all-books-overwrite-popup

Do these schemes look intuitive to you? I may be pretty biased atm, but I feel like there is no longer room for guesswork.

mbarritt commented 3 months ago

@bandantonio

Thanks for the opportunity to comment on your planned changes.

On Aug 8, 2024, at 4:23 PM, Antonio @.***> wrote:

Hi @JeffreyGH10 https://github.com/JeffreyGH10, @mbarritt https://github.com/mbarritt, I have some updates regarding this feature. I'm finalizing the refactoring process for the import logic, just wanted to align with you to ensure that the updated logic makes sense to you.

So, this is what I've come up so far:

When backups are disabled:

Importing one book and the book doesn't exist - no popup is displayed

So the new book's highlights/notes file is added to the default Highlights directory and all the other files (books) already there are left untouched. Correct? (If so that's great.) Importing one book and the book already exists - popup is displayed Screenshot.2024-08-08.at.10.54.23.PM.png (view on web) https://github.com/user-attachments/assets/931f8624-ecb9-44aa-8aa1-f02491a58225 Perfect. When backups are enabled:

Importing one book and the book doesn't exist - import takes place (nothing to backup), no popup is displayed

Sounds good.

Importing one book and the book already exists - the existing book is backed up as --bk-Date.now(), import takes place, no popup is displayed

Interesting. What directory would this single file with timestamp be placed in?

I'm liking the idea that a single file re-imported with backups turned on goes into the main Highlights directory (with its distinguishing time stamp appended to the file name where it will alphabetize neatly next to the original).

The same logic applies to bulk import, so the most "destructive" case is importing all books when backups are disabled. The will be the corresponding popup:

Screenshot.2024-08-08.at.11.16.10.PM.png (view on web) https://github.com/user-attachments/assets/208db625-c4da-4eb9-a29f-54c06d1c3ff6 Again, seems good to me.

What about the middle case where someone multi-selects a number of files, but not all files for an import? (Will this be supported?) Would the pop-up selectively display the names of only those files in the import that previously existed that would result in an overwrite? (Might not be all the imported files, if some of them existed already and some of them didn't.)

Do these schemes look intuitive to you? I may be pretty biased atm, but I feel like there is no longer room for guesswork.

Yes, these proposed schemes do feel intuitive to me (with the caveat of the questions above).

I'm really looking forward to these additions. With this new logic my first/primary (and frequent) use will be to import new books to the primary Highlights directory without the previously existing book highlights/notes files being touched (great!). Second will be to re-import books (to primary Highlights directory) for which I've changed/added highlights and notes after having imported them previously. Given the structures you've outlined above I'll most likely keep backups off and selectively overwrite the prior versions without any need for backup.

In fact, I'm not sure my workflow will see much use of the backup facility. I have Time Machine backups should I ever need to go back to an earlier version of the highlights directory, and I'll be happy to overwrite individual books as I add/update highlights and notes for them. One of my goals is to have one clean Highlights directory in Obsidian, so the backup directory would create duplication (in searches, links, etc.) that might turn into an encumbrance. Not to say there isn't a use case for the backup directories, but I think they fall outside my use case.

This is all great. Looking forward to giving the new logic a try when you roll it out. Thanks!

— Reply to this email directly, view it on GitHub https://github.com/bandantonio/obsidian-apple-books-highlights-plugin/issues/34#issuecomment-2276590605, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVQL7PZG6JSXQNNONSNF7ODZQPHS5AVCNFSM6AAAAABK4UBGSGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZWGU4TANRQGU. You are receiving this because you were mentioned.

bandantonio commented 3 months ago

@mbarritt

So the new book's highlights/notes file is added to the default Highlights directory and all the other files (books) already there are left untouched. Correct? (If so that's great.)

Yes, exactly.

Interesting. What directory would this single file with timestamp be placed in? I'm liking the idea that a single file re-imported with backups turned on goes into the main Highlights directory (with its distinguishing time stamp appended to the file name where it will alphabetize neatly next to the original).

You will like the plugin even more because this is exactly how I implemented the logic: single file imports will always be placed inside the highlights directory, next to the original. Backup directory with all the books will be created only when doing a bulk import. In this case, the backup directory will be placed next to the main highlights directory as well.

What about the middle case where someone multi-selects a number of files, but not all files for an import? (Will this be supported?)

As far as I remember, the modal window where you query the books does not support multi-selection. So this case would be out of scope for now.

Yes, these proposed schemes do feel intuitive to me

Great, thank you. That's such a relief to me. According what you said, your workflow will be greatly simplified with the new logic. Can't wait for you to try it out.

mbarritt commented 3 months ago

@bandantonio

Glad to see your responses to my most recent questions. Seems great. The lack of multi-selection will not be hard to get around (re-export books with new highlights/notes one at a time, should work fine). Looking forward to using it.

Thanks!

On Aug 9, 2024, at 1:16 PM, Antonio @.***> wrote:

@mbarritt https://github.com/mbarritt So the new book's highlights/notes file is added to the default Highlights directory and all the other files (books) already there are left untouched. Correct? (If so that's great.)

Yes, exactly.

Interesting. What directory would this single file with timestamp be placed in? I'm liking the idea that a single file re-imported with backups turned on goes into the main Highlights directory (with its distinguishing time stamp appended to the file name where it will alphabetize neatly next to the original).

You will like the plugin even more because this is exactly how I implemented the logic: single file imports will always be placed inside the highlights directory, next to the original. Backup directory with all the books will be created only when doing a bulk import. In this case, the backup directory will be placed next to the main highlights directory as well.

What about the middle case where someone multi-selects a number of files, but not all files for an import? (Will this be supported?)

As far as I remember, the modal window where you query the books does not support multi-selection. So this case would be out of scope for now.

Yes, these proposed schemes do feel intuitive to me

Great, thank you. That's such a relief to me. According what you said, your workflow will be greatly simplified with the new logic. Can't wait for you to try it out.

— Reply to this email directly, view it on GitHub https://github.com/bandantonio/obsidian-apple-books-highlights-plugin/issues/34#issuecomment-2278393954, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVQL7PY7HWSAHGT6XFUZNVTZQT2P3AVCNFSM6AAAAABK4UBGSGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZYGM4TGOJVGQ. You are receiving this because you were mentioned.

bandantonio commented 3 months ago

@mbarritt @JeffreyGH10 Version 1.5.0 is waiting for you to try it out. Waiting for your feedback on your (hopefully) improved experience.

mbarritt commented 3 months ago

Great first quick test. I re-imported one book with backup off, saw the pop-up and it replaced the file. Did it again with backups on and it created the time stamped file next to the original. works well. Files are as they should be.

Do you know what the sorting is used in the dialog where one selects which single book to import? It didn't seem to be alphabetical or by recently opened. Bit of a struggle to find the book I was testing. (Alphabetical would be great. Don't know if this is in your control.)

I will give it more of a test over the next couple days (in regular use), but it seems great with this quick test. Thanks again.

bandantonio commented 3 months ago

@mbarritt Good, glad to hear that the quick test didn't reveal something unexpected.

Do you know what the sorting is used in the dialog where one selects which single book to import? It didn't seem to be alphabetical or by recently opened. Bit of a struggle to find the book I was testing. (Alphabetical would be great. Don't know if this is in your control.)

That's a good question. My initial research didn't reveal any obvious patterns, so I should dive deep here and play around with different data passed to the list. The interesting thing is that still the list is consistent across Obsidian launches, so there is some sort of a pattern, we only need to figure it out.


P.S.: I will be updating the readme soon so I would like to ask your permission to use a slightly updated version of your previous feedback for the "What users say" block. Would you be ok with the modified version (without the word "hopefully") below?

I've used three or four other methods of keeping my Apple Books highlights and notes updated in Obsidian over the past few years. I'm moving to your plugin, it seems to be the best (fast, dependable, with a user editable template(!!!), and hopefully with selective single book importing/overwriting). Thanks for creating and sharing it!

mbarritt commented 3 months ago

Antonio,

On Aug 10, 2024, at 7:48 PM, Antonio @.***> wrote:

@mbarritt https://github.com/mbarritt Good, glad to hear that the quick test didn't reveal something unexpected.

Do you know what the sorting is used in the dialog where one selects which single book to import? It didn't seem to be alphabetical or by recently opened. Bit of a struggle to find the book I was testing. (Alphabetical would be great. Don't know if this is in your control.)

That's a good question. My initial research didn't reveal any obvious patterns, so I will dive deep here and play around with different data pass to the list. The interesting thing is that still the list is consistent across Obsidian launches, so there is some sort of a pattern, we only need to figure it out.

It will be much easier to do single book re-exporting with alphabetical sort order. Fingers crossed, hope you find something workable and not hard to implement.

P.S.: I will be updating the readme soon so I would like to ask your permission to use a slightly updated version of your previous feedback https://github.com/bandantonio/obsidian-apple-books-highlights-plugin/issues/34#issuecomment-2259090562 for the "What users say" block https://github.com/bandantonio/obsidian-apple-books-highlights-plugin?tab=readme-ov-file#what-users-say. Would you be ok with the modified version (without the word "hopefully") below?

I've used three or four other methods of keeping my Apple Books highlights and notes updated in Obsidian over the past few years. I'm moving to your plugin, it seems to be the best (fast, dependable, with a user editable template(!!!), and hopefully with selective single book importing/overwriting). Thanks for creating and sharing it!

Yes, I'm fine with you using that text as edited. Thanks for asking.