Fixing issue with how the check is done for determining if templatePa… #11620
Conversation
…th already terminates with subDir
Codecov Report
@@ Coverage Diff @@
## master #11620 +/- ##
=========================================
Coverage 93.36% 93.36%
- Complexity 13014 13015 +1
=========================================
Files 436 436
Lines 32831 32831
=========================================
Hits 30652 30652
Misses 2179 2179
Continue to review full report at Codecov.
|
| @@ -1249,7 +1249,7 @@ protected function _getViewFileName($name = null) | |||
| if (strlen($this->subDir)) { | |||
| $subDir = $this->subDir . DIRECTORY_SEPARATOR; | |||
| // Check if templatePath already terminates with subDir | |||
| if (strrpos($templatePath, $subDir) == strlen($templatePath) - strlen($subDir)) { | |||
| if ($templatePath != $subDir && substr($templatePath, -(strlen($subDir))) == $subDir) { | |||
There was a problem hiding this comment.
You can use strict !== and === when you are comparing strings now.
There was a problem hiding this comment.
Shouldn't $templatePath and $subDir be equal sometimes ?
There was a problem hiding this comment.
When would that happen? Extension directories are generally lower case, like csv while template paths usually end with the controller name Csvs.
There was a problem hiding this comment.
@dereuromark - yes, maybe === should be used.
@saeideng - I agree with mark that is shouldn't happen because $templatePath is using the controller name and the first letter is uppercase, whereas the $subDir is typically all lowercase.
@markstory - Wouldn't the test you added during this commit (115a354) cover this case as well?
There was a problem hiding this comment.
It does to some extent. But you have a new scenario that requires different code. With only the existing test, this change could be reverted and there wouldn't be a failing test to let us know there was a regression.
Ensure that controller names with length equal to the view subDir still have the subdirectory applied.
…th already terminates with subDir
The problem the with current check:
strrpos($templatePath, $subDir) == strlen($templatePath) - strlen($subDir)Lets say your have a templatePath that is equal to
Jobsfor example and you have a subDir equal to sayxlsxboth have astrlenof4,strrposreturns false and 4-4 = 0 thereforefalse == 0istruewhich is incorrect.