Updating the Yamtrack CSV import to allow specific book importing.

[psyciknz]

The regular import will work with media_id and source set. But then can now be empty and a book search by title will be performed. The title can be a book title or an ISBN number.

[psyciknz]

I had some changes for the wiki as well to describe the differences

[FuzzyGrim]

Hi, thanks for the PR.

Instead of _handle_missing_book_metadata() I think it would be better to have a generic function that works for all media types, so instead of manually using Hardcover if no souce is provided, you can obtain the default source for each media type from the app/media_type_config.py file.

Also I think we can remove src/integrations/tests/mock_data/import_books.csv because it's not used, and in the test case you are using import_books_yamtrack2.csv.

[psyciknz]

Hi, thanks for the PR.

Instead of _handle_missing_book_metadata() I think it would be better to have a generic function that works for all media types, so instead of manually using Hardcover if no souce is provided, you can obtain the default source for each media type from the app/media_type_config.py file.

Also I think we can remove src/integrations/tests/mock_data/import_books.csv because it's not used, and in the test case you are using import_books_yamtrack2.csv.

The reason I chose to hardcode hardcover was because it seemed to support isbn numbers as a title search.

By making it a generic method, wont I then have ot add test case for missing metadata for all the other types of media as well? Where at the moment I've only done the specific code for books.

I'll clean up the test case and remove the extra support files. This would be a hangover from my dedicated book import.

[FuzzyGrim]

Both Hardcover and OpenLibrary suport ISBN numbers in their title search. But I think it's a good idea to have it generic, with that we could say that the import from CSV can work either by providing media_id + media_type or title/isbn + media_type. The logic is similar because services.search() accepts media_types and sources as arguments.

metadata = services.search(
                    media_type,
                    searchquery,
                    1,
                    Sources.HARDCOVER.value,
                )

Also you don't need to manage this case in _handle_missing_book_metadata():

            if row["source"] != "":
                metadata = services.get_media_metadata(
                    media_type,
                    row["media_id"],
                    row["source"],
                )
                row["title"] = metadata["title"]
                row["image"] = metadata["image"]

Because it's already in _handle_missing_metadata():

    def _handle_missing_metadata(self, row, media_type, season_number, episode_number):
        """Handle missing metadata by fetching from provider."""
        if row["source"] == Sources.MANUAL.value and row["image"] == "":
            row["image"] = settings.IMG_NONE
        elif media_type == MediaTypes.BOOK:
            self._handle_missing_book_metadata(row,media_type)
        else:
            try:
                metadata = services.get_media_metadata(
                    media_type,
                    row["media_id"],
                    row["source"],
                    season_number,
                    episode_number,
                )
                row["title"] = metadata["title"]
                row["image"] = metadata["image"]
            except services.ProviderAPIError as e:
                self.warnings.append(
                    f"Failed to fetch metadata for {row['media_id']}: {e!s}",
                )
                raise

And in the above function I don't think you need to add the try/except for services.ProviderAPIError because it should be already handled in import_data():

        for row in reader:
            try:
                self._process_row(row)
            except services.ProviderAPIError as error:
                error_msg = (
                    f"Error processing entry with ID {row['media_id']} "
                    f"({app_tags.media_type_readable(row['media_type'])}): {error}"
                )
                self.warnings.append(error_msg)
                continue
            except Exception as error:
                error_msg = f"Error processing entry: {row}"
                raise MediaImportUnexpectedError(error_msg) from error

For the test case, you can include a CSV containing an example of each media type, but you can leave it to me if you prefer.

[psyciknz]

So I think I've pushed what I think you intended.

I haven;t removed the specific book handler yet, but it's not used.

And i guess I should update the import_books_yamtrack to make it generic and add other media types with no source_id?

[psyciknz]

Updated the naming of the tests i wrote to be more generic, called them yamtrack_partials, as opposed to explicitly books
Added a movie to the import_partial csv to the process.

[psyciknz]

Any update to this being merged? I'm hoping to import all my books and stat using it.

[FuzzyGrim]

Yes! Thanks.