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

Amazon: Update for new layout and updating tests #2942

Merged
merged 4 commits into from
Dec 20, 2022

Conversation

ajamtli
Copy link
Contributor

@ajamtli ajamtli commented Dec 6, 2022

  • Added extraction of abstractNote from book description expander
  • Rearranged matching for detail bullet list, assuming newer design is most likely to be encountered
  • Supporting new design of bullet list
  • Updated tests to match current content with the following exceptions:
    • Removed wishlist test since the list does not exist
    • 'Adaptation' language list is not sorted from Amazon and test will randomly fail
    • 'audioRecording' - unable to test, Scaffold browser not supported

- Added extraction of abstractNote from book description expander
- Rearranged matching for detail bullet list, assuming newer design is
  most likely to be encountered
- Supporting new design of bullet list
- Updated tests to match current content with the following exceptions:
    - Removed wishlist test since the list does not exist
    - 'Adaptation' language list is not sorted from Amazon and test will
      randomly fail
    - 'audioRecording' - unable to test, Scaffold browser not supported
Copy link
Member

@AbeJellinek AbeJellinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Could you add a test for the new design? Or do any of the tests already cover it?

@@ -354,12 +348,23 @@ function scrape(doc, url) {
}
}
}

if (!els.length) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be moved down?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because movies still use an information table that will match the expression. However this is just a subset of the information and we want to extract from the detailBullets_feature_div for movies as well since this contains all the information elements we want to capture.

ajamtli and others added 2 commits December 10, 2022 07:32
Co-authored-by: Abe Jellinek <jellinek@berkeley.edu>
Co-authored-by: Abe Jellinek <jellinek@berkeley.edu>
@ajamtli
Copy link
Contributor Author

ajamtli commented Dec 10, 2022

The tests already cover the design changes by checking that the details and the abstract note is correctly picked up.

@ajamtli ajamtli requested a review from AbeJellinek December 10, 2022 06:46
@AbeJellinek
Copy link
Member

AbeJellinek commented Dec 14, 2022

OK, please update lastUpdated so this will get pushed out to clients once merged. (You should be using Scaffold, which will do that automatically.)

@ajamtli
Copy link
Contributor Author

ajamtli commented Dec 15, 2022

Timestamp updated. Thank you for your help and guidance!

@AbeJellinek AbeJellinek merged commit fc2a1ca into zotero:master Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants