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

Always return a current order state #7186

Merged
merged 1 commit into from Dec 29, 2016
Merged

Always return a current order state #7186

merged 1 commit into from Dec 29, 2016

Conversation

maximebiloe
Copy link
Contributor

@maximebiloe maximebiloe commented Dec 8, 2016

Questions Answers
Branch? 1.7.0.x
Description? Always return a current order state to prevent fatal on front office in order history
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? http://forge.prestashop.com/browse/BOOM-1994

@maximebiloe maximebiloe added this to the 1.7.0.4 milestone Dec 8, 2016
@vTerenti vTerenti added the QA ✔️ Status: check done, code approved label Dec 8, 2016
@Flowster
Copy link
Contributor

Flowster commented Dec 8, 2016

Shouldn't you put getDefaultHistory in Order class next to getHistory function? And get dynamically the same fields as in the initial query?

So that if a field is added to OrderHistory or OrderState it will be present in this default history.

@maximebiloe
Copy link
Contributor Author

As I fill the data with an nonexistent order state, I prefer to do it into the presenter, it's just to present the data.
Also, even in the Order class, I wouldn't be able to load the state dynamically because it does not exist.

@aleeks
Copy link
Contributor

aleeks commented Dec 29, 2016

Thank you @maximebiloe

@aleeks aleeks merged commit 5bbcf75 into PrestaShop:1.7.0.x Dec 29, 2016
@aleeks aleeks deleted the fix-current-order-state branch December 29, 2016 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA ✔️ Status: check done, code approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants