Draft: feat: add error message to AutoImportFormModal
First off, thanks so much for your work on this project. I've been a long-time (about a year now) user of Jelu. As someone that really wants to track books that I've read in a way that I can own and customize, I really appreciate your effort on this project and for sharing it with us!
Please go easy on me on this one, I'm not a frontend or Kotlin developer, so I've probably majorly messed this up in several different ways. Thus I'm marking this PR as a Draft. I'm super open to any sort of feedback about this PR, even if it is just that the project doesn't like the approach or doesn't agree that this is a problem.
Outstanding Items:
- [ ] Receive feedback on the approach from @bayang
- [ ] Add the rest of the languages for localization text
- [ ] Add error message functionality to other metadata providers
- [ ] Understand if there is anyway good way to localize the error message from the plugin provider
I frequently use the Auto Import functionality of Jelu. One of the few things that I've found frustrating with Jelu is that the Auto-Import somewhat regularly encounters errors. When it happens, I usually have no idea what happens, I'll get a box that looks like:
Since it has a cover image I actually thought for a long time that it had found a result, but that the metadata system just didn't have any info for the book. It is only recently as I started to look into Jelu more, that I realized that this is the screen that is shown when the metadata plugin returns an empty result because of some sort of error.
To that end, in this PR I've attempted to show the user:
- That there is an error
- Give the user an option to go into the weeds and see information directly from the metadata plugin
After this PR (at least in the English localization) it will display the following:
Thank you for both your PRs ! Just give me some time to review them.
No need to add other locales for now, only english is required, the others will be added in crowdin.
We'll see what to do for the other providers later.
Ok let's get this moving and sorry for being long to review. I'm ok to merge this now, but would you agree to add a test case using your new error field (on the backend) ?
Thanks for the reply!
I can totally add a test case for the error field. Give me a few days to finish some other stuff up and I'll add it into this PR.
All right, sorry that took me so long. But I finally got the tests done.
I have kept the tests as a separate commit in case you don't like them, so that I can roll back to only the original commit. Let me know if you want me to squash them to make the commit log more atomic.
All cards on the table, I made extensive use of AI to generate the unit tests. That being said, I definitely went through quite a few iterations and made some manual changes along the way, since, in my experience, AI typically doesn't do the best thing right away. Still, I'm not really a kotlin dev, and I only have a bit of practice with this code base, so let me know if you don't like it.
In as much as possible, I tried to mock out the ProcessBuilder and MetadataProvider so that I could test the actual logic of the fetch process to ensure that errors were only being added on error conditions. This added quite a bit to the CalibreMetadataProviderTest.kt file.
:tada: This PR is included in version 0.69.0 :tada:
The release is available on:
v0.69.0- GitHub release
Your semantic-release bot :package::rocket:
Alright, let's move forward. Thank you very much for your contribution !