Fix admin order sorting functionality #19
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Due to a missing query API privided by OXID, SQL strings need to be manipulated directly in admin list controllers. However, this is very error prone if multiple modules do this. For example one could replace `from oxorder` with `FROM OXORDER` or `from `oxorder``` (using MySQL identifier quoting. All this results in valid SQL and should be considered to make the extension of the sql string more robust. For example, the oxid paypal module replaces the unqoted `from oxorder` with a quoted table name: https://github.com/OXID-eSales/paypal/blob/eb1392f37f34fec5a477d7319cad12aba35722fe/Controller/Admin/OrderList.php#L72 Currently, clicking the admin orders' "sort by payment column" link crashes when the SQL FROM clause is not exactly of the form `from oxorder`. Because in that case, the joins are not applied and the order by statement extensions in `_prepareOrderByQuery()` use columns that can not be found. The provided solution makes the extension more robust against different valid sql strings so that clicking the sorting link does not crash. It works with variants like * `from oxorder` * `FROM OXORDER` * `FROM oxorder` * ``FROM `oxorder``` * ... Further it uses `str_ireplace` as case-insensitive replacement method to be more robust against valid variations.
|
Btw.: In the meantime the oe paypal module has been fixed. (https://github.com/OXID-eSales/paypal/pull/54/files) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Due to a missing query API privided by OXID, SQL strings need to be manipulated directly in admin list controllers.
However, this is very error prone if multiple modules do this. For example one could replace "from oxorder" with "FROM OXORDER" or "from
oxorder" (using MySQL identifier quoting (backticks)). All this results in valid SQL and should be considered to make the extension of the sql string more robust.For example, the oxid paypal module replaces the unqoted
from oxorderwith a quoted table name:https://github.com/OXID-eSales/paypal/blob/eb1392f37f34fec5a477d7319cad12aba35722fe/Controller/Admin/OrderList.php#L72
Currently, clicking the admin orders' "sort by payment column" link crashes when the SQL FROM clause is not exactly of the form
from oxorder.Because in that case, the joins are not applied and the order by statement extensions in
_prepareOrderByQuery()use columns that can not be found.The provided solution makes the extension more robust against different valid sql strings so that clicking the sorting link does not crash.
It works with variants like
oxorderFurther it uses
str_ireplaceas case-insensitive replacement method to be more robust against valid variations.