fix(ja.soraraw): tolerate null genre rows - #732
Merged
Conversation
kkantan
approved these changes
Aug 31, 2026
Contributor
|
the other pr should increment the version to 3, since it is an additional change and cannot come before aidoku is updated. I would like to suggest not using an llm to generate pr descriptions. this is a very simple fix that shouldn't require more than one or two lines of explanation. I know english is not your first language, so using an llm for translation or assistance is acceptable, but larger descriptions can cause more important information to be obscured. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fetching details for a series whose genre list holds an empty row fails the whole request, so the
series cannot be opened at all.
GenredeclarednameandslugasString. A few series carry genre rows the site never filledin — every field but the id is
null— andserderejects the row, which fails the deserializationof the entire page payload. The series then returns an error instead of details or chapters.
This widens both fields to
Option<String>:Genre::into_tagdrops a row with no name, alongside the existing check that drops an empty oneMangaDetails::viewerpicks the reader from the slugs that are present, usingfilter_mapinstead of
mapNothing else changes. A series with ordinary genre rows behaves exactly as before.
The rows in question
majime-fumajime-maji-koiji-64273is the example the test uses. Itsgenresarray:[ { "id": 12, "name": "日常", "slug": "nichijou" }, { "id": 17, "name": "コメディ", "slug": "komedi" }, { "id": 24, "name": "ロマンス", "slug": "romansu" }, { "id": 2211, "name": null, "slug": null }, { "id": 2212, "name": null, "slug": null } ]Ids 2211 and 2212 are absent from
genres.json, which lists 1909 genres, none of them null. Sothese look like rows the site deleted from the genre table while leaving the join rows behind — the
manga page joins against the missing ids and gets nulls back. That makes it a data-shape the source
has to tolerate rather than something the site is likely to clean up.
Testing
19 tests pass, including a new
test_null_genres, which fetches the series above and asserts thedetails request succeeds and still returns the three real genres as tags. It fails against the
previous revision with
unexpected page data.cargo fmt,cargo clippy --release,cargo test --release,aidoku packageandaidoku verify package.aixare all clean.Not device tested — the fix is in deserialization and the test runner exercises the same path.
version— needs a decisionSet to 2 here, but #697 also raises it from 1 to 2 for the same source. Whichever merges second
needs 3 instead. I have left both at 2 rather than guess at the merge order; tell me which one you
would like to take first and I will rebase the other and set the number.
The two changes do not otherwise overlap: this one touches
models.rs, #697 toucheshelpers.rsand
lib.rs, and the only shared file isres/source.jsonon that one line.This is the smaller and more clearly scoped of the two, and it fixes series that currently do not
open at all, so it may be worth taking first regardless of where #697 lands.