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.

Sign up for GitHub

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

Jump to bottom

Handle known bad Amazon queries #56

Closed
leesharma opened this issue Jun 30, 2017 · 2 comments · Fixed by #173
Closed

Handle known bad Amazon queries #56

leesharma opened this issue Jun 30, 2017 · 2 comments · Fixed by #173
Labels
Amazon Product API Bug Hacktoberfest Help Wanted

Comments

@leesharma
Copy link
Collaborator

There are some queries we know won't be accepted by Amazon (for example, queries can't be blank–it results in a blank result.)

We should validate the search for those very basic checks and supply an error message if a user tries to submit an invalid query.
@leesharma leesharma modified the milestone: MVP Jul 28, 2017
@leesharma
Copy link
Collaborator Author

@zr2d2 said he'd work on this one 🎉

@zr2d2 zr2d2 mentioned this issue Aug 2, 2017
Merged
@leesharma
Copy link
Collaborator Author

Found another invalid query! An apostrophe seems to break signature generation: when I try to add "k'nex" or "sophie's world", I get the following error:

"Error"=>{"Code"=>"SignatureDoesNotMatch", "Message"=>"The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details."}

There are two fixes to make here:

  • We should handle and log the "SignatureDoesNotMatch" error (currently it just shows a blank page)
  • We should make signatures work with apostrophes

@leesharma leesharma reopened this Oct 22, 2017
@leesharma leesharma removed this from the MVP milestone Oct 22, 2017
@leesharma leesharma mentioned this issue Oct 26, 2017
Open
leesharma added a commit to leesharma/playtime that referenced this issue Nov 26, 2017
@leesharma
Fix all Lint/UriEscapeUnescape violations
b24baa7 
Fixes rubyforgood#56

This commit changes all the `URI.escape` calls to `CGI.escape`. All
tests pass and the app still works. As a bonus, this seems to resolve
issue rubyforgood#56 too-apostrophes are now properly escaped.

This resolves all rubocop issues relating to this PR.
@leesharma leesharma mentioned this issue Nov 26, 2017
Merged
leesharma added a commit that referenced this issue Nov 29, 2017
@leesharma
Add Amazon Product API "ItemLookup" endpoint (#173)
d50f959 
* Refactor AmazonProductAPI

In preparation for #62, this commit pulls the endpoint-specific
information out of the HTTPClient class and into and endpoint-specific
class.

Now HTTPClient is responsible only for managing the environment
information and directing the user to the relevant endpoint. Adding a
new endpoint is as simple as adding a method (ex. `item_search`) and
corresponding class (ex. `ItemSearchEndpoint).

The interface is *not* in its final form yet. This is just a good
breaking point for a commit.

* Add ItemSearch endpoint

Building on the last commit, this adds a new ItemLookup endpoint. Now we
can pull details on an individual item based on the ASIN.

The endpoints and responses include a lot of duplication. Some
refactoring will probably be needed.

* Extract spec for ItemSearchEndpoint

When the item search endpoint was extracted, the specs weren't extracted
with it; this means that the HTTPClientSpec was testing everything
relating to the item search endpoint.

This commit extracts all specs relating to the endpoint into a new file.
This can now be refactored and copied over for the
`ItemLookupEndpointSpec`.

* Add spec for ItemLookupEndpoint

* Fix CodeClimate issue

This is a bit of a hack solution, but I don't feel comfortable doing any
major abstraction here yet; I don't think we have enough information.

Hopefully this'll clear up the CodeClimate complaints about duplicated
code!

* Fix easy PR-related rubocop issues

Two big points:

  1. This only fixes easy rubocop issues related to this PR. This
     doesn't touch any of the new capistrano code; I want the diff for
     this PR to be fairly contained.

  2. This includes two rubocop config changes:
      i.  Exclude the vendor directory from linting
      ii. Allow multiline braces in tests (for exception checks, let, etc.)

There are a few more risky/controversial rubocop changes I omitted from
this commit. Those are coming next.

* Fix all Lint/UriEscapeUnescape violations

Fixes #56

This commit changes all the `URI.escape` calls to `CGI.escape`. All
tests pass and the app still works. As a bonus, this seems to resolve
issue #56 too-apostrophes are now properly escaped.

This resolves all rubocop issues relating to this PR.

* Give better name to AWS test credentials in specs

Following the review suggestions. This renames `env` to
`aws_credentials`. The latter is a better name because it represents a
credentials object (built from env vars), not the ENV object itself.

Hopefully this will be clearer!

* Fix item lookup hash

When I fixed the code style for Code Climate in #173, I used the wrong
hash leading to no update attributes being found. This fixes the bug,
adds a test, and renames some of the variables.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Amazon Product API Bug Hacktoberfest Help Wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add Amazon Product API "ItemLookup" endpoint leesharma/playtime
2 participants
@leesharma @micahbales