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 missing appendArray in OrderReturnLazyArray (BOOM-6039) #9422

Merged
merged 1 commit into from Aug 13, 2018

Conversation

Projects
None yet
6 participants
@jolelievre
Contributor

jolelievre commented Aug 8, 2018

Questions Answers
Branch? develop
Description? The FO page presenting the OrderReturn did not work anymore due to modifications on the OrderReturnPresenter::present method which no longer returns an array but a OrderReturnLazyArray object which is perfect to access the generated fields like detail_url through the getDetailsUrl method. But it removed to access to original array data like state_name and such.

Rather than implementing all the getter methods we simply use the AbstractLazyArray::appendArray method to open access to all the missing array fields.
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? http://forge.prestashop.com/browse/BOOM-6039
How to test? See forge ticket

This change is Reviewable

@prestonBot

This comment has been minimized.

Show comment
Hide comment
@prestonBot

prestonBot Aug 8, 2018

Collaborator

Hello @jolelievre!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

Collaborator

prestonBot commented Aug 8, 2018

Hello @jolelievre!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

@jolelievre jolelievre requested a review from jocel1 Aug 8, 2018

@eternoendless eternoendless added this to the 1.7.5.0 milestone Aug 10, 2018

* @param Link $link
* @param $orderReturn
* @param array $orderReturn

This comment has been minimized.

@PierreRambaud

PierreRambaud Aug 10, 2018

Contributor

If you're sure it's everytime an array, you can edit the signature below =)

@PierreRambaud

PierreRambaud Aug 10, 2018

Contributor

If you're sure it's everytime an array, you can edit the signature below =)

This comment has been minimized.

@jolelievre

jolelievre Aug 10, 2018

Contributor

I hesitated but I was not sure, couldn't there be incompatibilities with old versions of PHP? (<7)

@jolelievre

jolelievre Aug 10, 2018

Contributor

I hesitated but I was not sure, couldn't there be incompatibilities with old versions of PHP? (<7)

This comment has been minimized.

@PierreRambaud

PierreRambaud Aug 10, 2018

Contributor

No, just depends if you pass other thing than an array like Traversable or a generator.
It's maybe a mixed instead of an array. Wdyt?

@PierreRambaud

PierreRambaud Aug 10, 2018

Contributor

No, just depends if you pass other thing than an array like Traversable or a generator.
It's maybe a mixed instead of an array. Wdyt?

This comment has been minimized.

@jolelievre

jolelievre Aug 10, 2018

Contributor

you're right we should force an array because OrderReturnPresenter::present checks that the input is an array


I am gonna change it

@jolelievre

jolelievre Aug 10, 2018

Contributor

you're right we should force an array because OrderReturnPresenter::present checks that the input is an array


I am gonna change it

Add missing appendArray in OrderReturnLazyArray (BOOM-6039)
The FO page presenting the OrderReturn did not work anymore due to
modifications on the OrderReturnPresenter::present method which no
loner returns an array but a OrderReturnLazyArray object which is
perfect to access the generated fields like detail_url through the
getDetailsUrl method. But it removed to access to original array
data like state_name and such.

Rather than implementing all the getter methods we simply use the
AbstractLazyArray::appendArray method to open access to all the
missing array fields.

@marionf marionf self-assigned this Aug 13, 2018

@marionf marionf removed their assignment Aug 13, 2018

@marionf marionf added QA ✔️ and removed waiting for QA labels Aug 13, 2018

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 13, 2018

Contributor

Thanks @jolelievre and everyone for the reviews !

Contributor

mickaelandrieu commented Aug 13, 2018

Thanks @jolelievre and everyone for the reviews !

@mickaelandrieu mickaelandrieu merged commit 09e7919 into PrestaShop:develop Aug 13, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jolelievre jolelievre deleted the jolelievre:BOOM-6039 branch Sep 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment