Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(caa upload): support missing Amazon TLDs #190

Merged
merged 3 commits into from
Oct 27, 2021
Merged

Conversation

kellnerd
Copy link
Collaborator

@kellnerd kellnerd commented Oct 26, 2021

Closes #189.
Trivial fix, tested with the release which is linked via the reported issue.

Should we also add all of the remaining domains which are listed on Wikipedia or should we wait until someone explicitly requests them?

@codecov
Copy link

codecov bot commented Oct 26, 2021

Codecov Report

Merging #190 (cede64b) into main (24b2541) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #190   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           35        35           
  Lines          768       770    +2     
  Branches       139       139           
=========================================
+ Hits           768       770    +2     
Impacted Files Coverage Δ
.../mb_enhanced_cover_art_uploads/providers/amazon.ts 100.00% <ø> (ø)
...hanced_cover_art_uploads/providers/amazon_music.ts 100.00% <ø> (ø)
src/lib/util/domain_dispatch.ts 100.00% <100.00%> (ø)
src/mb_enhanced_cover_art_uploads/fetch.ts 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24b2541...cede64b. Read the comment docs.

@ROpdebee
Copy link
Owner

Should we also add all of the remaining domains which are listed on Wikipedia or should we wait until someone explicitly requests them?

We might as well. Not sure what I based myself on when initially adding the Amazon provider, I guess MB source code?

Couple of requests:

  • Could you add it to Amazon Music too? Preferably just prefixing 'music.' to the domains defined in Amazon.
  • The multipart TLDs need to be added to the domain dispatcher too (com.au, com.tr, com.br, com.mx aren't in there yet).

@kellnerd
Copy link
Collaborator Author

kellnerd commented Oct 26, 2021

Ok, I have added the missing Amazon domains and added a TLD exception for .com.* to the domain splitter of the domain dispatcher. I even tested the importing of images for some of the new domains which do not even have a dedicated music category (.ae and .eg), CDs are listed under "electronics" there.

Adding all of these to Amazon Music too does not seem to be a good idea: While music.amazon.com.au exists, I could not open music.amazon.com.tr and music.amazon.se, the available domains seem to correspond to the "Amazon Music Prime" column in Wikipedia's overview. I have updated the supported domains according to that list (and checked all of them), which also includes the removal of .cn, .nl and .jp (.co.jp works).

@kellnerd kellnerd changed the title feat(caa upload): support amazon.com.au feat(caa upload): support missing Amazon TLDs Oct 26, 2021
@kellnerd kellnerd requested a review from ROpdebee October 26, 2021 21:58
@ROpdebee ROpdebee merged commit bda99da into main Oct 27, 2021
github-actions bot added a commit that referenced this pull request Oct 27, 2021
feat(caa upload): support missing Amazon TLDs (#190)
@github-actions
Copy link

🚀 Released 1 new userscript version(s):

  • mb_enhanced_cover_art_uploads 2021.10.27 in 5fcc540

@kellnerd kellnerd deleted the amazon-australia branch October 31, 2021 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mb_enhanced_cover_art_uploads doesn't support Amazon.com.au
2 participants