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

#44 Add order item read & link order with channel resource #47

Merged
merged 4 commits into from
Jun 28, 2018
Merged

Conversation

ronan-gloo
Copy link
Contributor

@ronan-gloo ronan-gloo commented Jun 28, 2018

Link to the issue

#44

Reason for this PR

  • Order details were not accessible from the OrderResource object
  • Channel info were not accessible from the OrderResource object

What does the PR do

Implements accessors for both

Unrelated changes

The AbstractResource::getProperty() method now accept a second argument, which indicates if the resource must loaded from the server to access to that property.

An usage example is here, where orders items are only provided when fetching a single resource:

https://github.com/shoppingflux/php-sdk/pull/47/files#diff-d7931d116e86a29bdd1e9ae70a77fcaaR104

Note that server request is only performed once, and not on every property access demand.

How to test

The following script will loop agains orders and print items / channel data :

$store    = $session->getMainStore();
$orders  = $store->getOrderApi()->getPage(['limit' => 10]);

foreach ($orders as $order) {
    print_r($order->getItems()->toArray());
    print_r($order->getChannel()->toArray());
}

@ronan-gloo ronan-gloo requested review from ddattee and benoit-apk June 28, 2018 11:25
@ronan-gloo
Copy link
Contributor Author

@ddattee I've cleaned a bit the documentation to make it more readable

private $reference;
private $quantity;
private $unitPrice;

Copy link
Contributor

Choose a reason for hiding this comment

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

Add var type in comments

* @return OrderItemCollection A new instance of self, holding relevant items
*/
public static function fromProperties(array $items)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe withItems would more accurate ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

with* are usually instance modifiers for immutable objects (check PSR-7 implementation).

* @param string $property
* @param string $property The property name
* @param bool $initialize Indicates if the resource must be fetched from server
* in order to access to this property (when no present in partial representation)
*
Copy link
Contributor

Choose a reason for hiding this comment

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

'...when NOT present...'

@ronan-gloo
Copy link
Contributor Author

@ddattee review done

@ddattee ddattee merged commit 5416fa7 into master Jun 28, 2018
@ddattee ddattee deleted the issue-44 branch September 3, 2019 15:28
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