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

Default carrier name was displayed as 0 #12140

Merged
merged 6 commits into from Jan 22, 2019

Conversation

Projects
None yet
5 participants
@Gitdyr
Copy link
Contributor

Gitdyr commented Jan 14, 2019

Questions Answers
Branch? develop
Description? In the back office order details the carrier name was displayed as "0" if the default carrier was used
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? Install a shop from scratch and create an order where the default carrier is selected (the one bearing the shop's name)

This change is Reviewable

@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Jan 14, 2019

I could not reproduce your bug. Are you sure there is a 0 stored in database?
If it is a data problem which can not be retrieve by sql request, I think it must be changed in PHP (instead of SQL request).

@Gitdyr

This comment has been minimized.

Copy link
Contributor Author

Gitdyr commented Jan 14, 2019

Yes, there is a 0 in the database.

Notice the method replaceZeroByShopName() in classes/Cart.php that fixes the problem in the cart.

@Quetzacoalt91

This comment has been minimized.

Copy link
Member

Quetzacoalt91 commented Jan 15, 2019

Well, even if this sound less optimized, I would rather see it as a for loop, applying the method on the carrier_name of each line.

This would make the code easier to read, and will avoid some potential security issues by applying a PHP variable in the SQL request.

Thanks

@Gitdyr

This comment has been minimized.

Copy link
Contributor Author

Gitdyr commented Jan 16, 2019

Updated pull request.

foreach ($results as &$row) {
if ($row['carrier_name'] == '0') {
$row['carrier_name'] = Carrier::getCarrierNameFromShopName();
}

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Jan 17, 2019

Member

Prefer using directly the existing code.

$row['carrier_name'] = Cart::replaceZeroByShopName($row['carrier_name'], null);
@Gitdyr

This comment has been minimized.

Copy link
Contributor Author

Gitdyr commented Jan 20, 2019

Updated pull request.

@PierreRambaud
Copy link
Contributor

PierreRambaud left a comment

LGTM, wdyt @Quetzacoalt91 ?

@mbadrani mbadrani self-assigned this Jan 21, 2019

@mbadrani mbadrani added QA ✔️ and removed waiting for QA labels Jan 21, 2019

@PierreRambaud PierreRambaud added this to the 1.7.6.0 milestone Jan 21, 2019

@PierreRambaud PierreRambaud dismissed stale reviews from Quetzacoalt91 and themself via c5b4fc8 Jan 21, 2019

@PierreRambaud PierreRambaud merged commit bd87838 into PrestaShop:develop Jan 22, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Jan 22, 2019

Thanks @Gitdyr

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