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

Fix bug for SessionStorage#with_shopify_session #1488

Conversation

m11o
Copy link
Contributor

@m11o m11o commented Aug 16, 2022

What this PR does

ShopifyApp::SessionStorage#with_shopify_session is given proc instance in block argument now. But, it should call block and be supplied session instance (ShopifyAPI::Auth::Session) to block. So I modified it and wrote test.

How to use

shop = Shop.find_by shopify_domain: 'example.myshopify.com'
shop.with_shopify_session do |session|
  client = ShopifyAPI::Clients::Graphql::Admin.new(session: session)
  response = client.query(query: query)
end

Reviewer's guide to testing

Run following

bundle exec rake test TEST=test/shopify_app/session/session_storage_test.rb

Things to focus on

  1. lib/shopify_app/session/session_storage.rb
  2. test/shopify_app/session/session_storage_test.rb

Checklist

Before submitting the PR, please consider if any of the following are needed:

  • Update CHANGELOG.md if the changes would impact users
  • Update README.md, if appropriate.
  • Update any relevant pages in /docs, if necessary
  • For security fixes, the Disclosure Policy must be followed.

@m11o
Copy link
Contributor Author

m11o commented Aug 16, 2022

I've signed the CLA!

@m11o m11o changed the title Fix bug for ShopifyApp::SessionStorage#with_shopify_session Fix bug for SessionStorage#with_shopify_session Aug 16, 2022
@yourivdlans
Copy link

Would be awesome to get this merged!

cc @andyw8 @teddyhwang @nelsonwittwer

@ElliottRoche
Copy link

Is this any closer to being merged? Could really use it in our app code that we're trying to upgrade. with_shopify_session is a super important and useful method for us and I'm sure a lot of other people.

@nelsonwittwer
Copy link
Contributor

🤦 This sat open for way too long. This is the correct fix, thank you for providing it @m11o !! ❤️

@nelsonwittwer nelsonwittwer merged commit 5aa51c4 into Shopify:main Apr 13, 2023
6 checks passed
@m11o m11o deleted the feature/fix-bug-for-with_shopify_session-in-session_storage branch April 13, 2023 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants