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

Feature/fix missing properties #70

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

WyekS
Copy link

@WyekS WyekS commented Oct 29, 2021

Title

Adding and creating new fields to get Orders, Products and Customer properly

Description of Changes

This models have been created:

  • AbstractModel (to implement admin_graphql_api_id field in some models)
  • model/discount/
    • DiscountAllocation
    • DiscountApplication
    • DiscountCode
  • model/price/
    • Money
    • PriceSet
  • Duty
  • PaymentDetails
  • Property
  • SMSMarketingConsent

This models have been change with new necessary attributes

  • Image
  • ShopifyAddress
  • ShopifyCustomer
  • ShopifyFulfillment
  • ShopifyLineItem
  • ShopifyOrder
  • ShopifyProduct
  • ShopifyShippingLine
  • ShopifyTaxLine
  • ShopifyVariant

After this changes Orders, Products and Customer have been obtained correctly
In Shop model has been ignored the new attributes for the moment

Possible Drawbacks

Without these changes the SDK did not work when getting the shop and when you wanted to get orders, products or customers. Now it's possible.

@ryankazokas ryankazokas self-requested a review October 29, 2021 11:35
@dimaloop
Copy link

dimaloop commented Feb 1, 2022

Any plans for review of this PR?
These changes are very important!

@ryankazokas
Copy link
Member

ryankazokas commented Feb 1, 2022

Thanks for the PR! Can you switch your PR target to be the develop branch? Also update it with the base so that github actions run.

Without these changes the SDK did not work when getting the shop and when you wanted to get orders, products or customers. Now it's possible.

What errors are you seeing? We recently made changes to the SDK to include API version. This API has been tested with version 2021-07.

@WyekS
Copy link
Author

WyekS commented Feb 2, 2022

Hi! I made those changes in Octuber '21. So, I think you probably fixed all things in hotfix 2.4.3 (December '21), but I need a few days to review that new version and if you need some of my changes, I can implement them in the current development branch if you think it's ok 🙂

@ryankazokas
Copy link
Member

Sounds good. Some things have changed since then, including we moved to Github actions. So if you want to keep this PR open and use this if you see any changes that need to be made, just make sure you switch target to develop and pull the changes, otherwise we won't be able to merge the PR unless all of the checks pass.

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.

3 participants