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

Add option to Change the created_via field. #609

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

scottyzen
Copy link
Contributor

Can be useful when WooCommerce is being used from multiply sources. For example, with plugins like Point of Sale for WooCommerce.

Your checklist for this pull request

Thanks for sending a pull request! Please make sure you click the link above to view the contribution guidelines, then fill out the blanks below.

🚨Please review the guidelines for contributing to this repository.

  • Make sure you are making a pull request against the develop branch (left side). Also you should start your branch off our develop.
  • Make sure you are requesting to pull request from a topic/feature/bugfix/devops branch (right side). Don't pull request from your master!
  • Have you ensured/updated that CLI tests to extend coverage to any new logic. Learn how to modify the tests here.

What does this implement/fix? Explain your changes.

Does this close any currently open issues?

Any relevant logs, error output, GraphiQL screenshots, etc?

(If it’s long, please paste to https://ghostbin.com/ and insert the link here.)

Any other comments?

Where has this been tested?

  • WooGraphQL Version: 0.10.7
  • WPGraphQL Version: 1.6.12
  • WordPress Version: 5.9
  • WooCommerce Version: 6.2.0

@codeclimate
Copy link

codeclimate bot commented Feb 19, 2022

Code Climate has analyzed commit 352ebb9 and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category Count
Style 3
Complexity 1

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 75.0% (0.0% change).

View more on Code Climate.

Copy link
Member

@kidunot89 kidunot89 left a comment

Choose a reason for hiding this comment

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

I'm fine with this change, but add it to the unit tests here and here

@sbolinger-godaddy
Copy link
Contributor

Can this also be added to the checkout mutation? Since the checkout creates an order, the checkout input should accept createdVia.

@kidunot89 kidunot89 self-requested a review May 22, 2023 19:46
@kidunot89
Copy link
Member

@scottyzen Get this rebased and we can merge it if it passes CI.

Can be useful when WooCommerce is being used from multiply sources. For example, with plugins like Point of Sale for WooCommerce.
Comment on lines +88 to +259
nodes {
url
accessExpires
downloadId
downloadsRemaining
name
product {
databaseId
}
download {
downloadId
}
}
}
needsPayment
needsProcessing
metaData {
key
value
}
couponLines {
nodes {
databaseId
orderId
code
discount
discountTax
coupon {
id
}
}
}
feeLines {
nodes {
databaseId
orderId
amount
name
taxStatus
total
totalTax
taxClass
}
}
shippingLines {
nodes {
databaseId
orderId
methodTitle
total
totalTax
taxClass
}
}
taxLines {
nodes {
rateCode
label
taxTotal
shippingTaxTotal
isCompound
taxRate {
databaseId
}
}
}
lineItems {
nodes {
productId
variationId
quantity
taxClass
subtotal
subtotalTax
total
totalTax
taxStatus
product {
... on SimpleProduct {
id
}
... on VariableProduct {
id
}
}
variation {
id
}
}
}
}
customer {
id
}
result
redirect
}
}
';
}
Copy link
Member

Choose a reason for hiding this comment

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

@scottyzen What's the purpose of this change? Also you didn't change the checkout mutation but the createOrder mutation. You should update the OrderMutationsTest instead.

@@ -483,7 +484,7 @@ public function testUpdateOrderMutation() {
],
'paymentMethod' => 'bacs',
'paymentMethodTitle' => 'Direct Bank Transfer',
'billing' => [
'billing' => array(
Copy link
Member

Choose a reason for hiding this comment

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

Please revert WooGraphQL uses short array syntax [ ]

@@ -808,7 +809,7 @@ public function testDeleteOrderMutation() {
],
'paymentMethod' => 'bacs',
'paymentMethodTitle' => 'Direct Bank Transfer',
'billing' => [
'billing' => array(
Copy link
Member

Choose a reason for hiding this comment

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

Please revert WooGraphQL uses short array syntax [ ]

@@ -959,7 +960,7 @@ public function testDeleteOrderItemsMutation() {
],
'paymentMethod' => 'bacs',
'paymentMethodTitle' => 'Direct Bank Transfer',
'billing' => [
'billing' => array(
Copy link
Member

Choose a reason for hiding this comment

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

Please revert WooGraphQL uses short array syntax [ ]

Copy link
Member

@kidunot89 kidunot89 left a comment

Choose a reason for hiding this comment

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

@scottyzen Your current changes only effect the createOrder mutation, you might want to use this functionality in the checkout mutation as well. Also, these changes need to be tested in the CheckoutMutationTest and OrderMutationsTest

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