macr0dev / Audiobooks.bundle

Plex metadata scraper for Audiobooks
600 stars 65 forks source link

Fix english but non-US site being forced to default US site #23

Open jmeosbn opened 6 years ago

jmeosbn commented 6 years ago

Hi there, just some minor fixes noted when selecting the UK audible site - main one is that .com is always used (see first line that's repeated from earlier in def).

jmeosbn commented 6 years ago

To clarify, passing lang='en' and base='www.audible.co.uk' to SetupUrls, runs into:

if sitetype: ... if base in sites_langs : (yes, it is)

then, base=intl_sites[lang]['url'] (looks up url for en in intl_sites)

Which overwrites base with www.audible.com - so I think that last line is mistakenly duplicated from further down in def where it appears in the else clause of if sitetype:.

Presumably, same will happen if wanting to use www.audible.com.au

macr0dev commented 6 years ago

I'll be back home on Tuesday and can run some thorough tests to see what's going on here. The initial international support based on library language was done by another developer and I wrapped the option to manually select it around that code. I'm working on a laptop in a different U.S. state than usual due to holiday traveling.

jmeosbn commented 6 years ago

Yes, I think that line must either be a duplication error, or remnant from some history where non-US-but-still-english sites were not added.

Commenting that single line seems to be all that's required, will probably rebase on latest to fix merge conflict on current head.

jmeosbn commented 6 years ago

After refactoring of the parameter localisation stuff into sites_langs along the other changes above, I get the following results with the same test library of six books, two of which have mangled info:

UK:

US:

However, initial library refresh still seems to skip updating despite matching 100% - it actually [edit: I've committed changes that seem to have resolved this issue)

jmeosbn commented 6 years ago

Added fallback workaround for title/searchTitle - will conflict with #28 so pulled that to here:

I've been having better results using advsearchKeywords search instead of explicitly specifying a title search (which is much more strict and fails on things like numbers prefixed etc.). Author can still be specified which results in a more flexible title search still sensibly constrained when author known.

Nice side effect is that advsearchKeywords= survives when searching the .com site from outside the US - unlike title=.

jmeosbn commented 6 years ago

Looks like the title/searchTitle issue when using .com outside the US got fixed today.. so the "Provide additional search parameter as fallback" commit may no longer be needed.

However, it doesn't hurt to have it and it provides a fallback that could simplify things going forward.

edit: switches back and forth so best keep it in..

jmeosbn commented 6 years ago

note: additional commits pruned back as I'm unable to keep stale PRs from conflicting between each other and my current local copy