Fix build failures for the cake 5 upgrade #326
Conversation
…with CakePHP 5, and fixing the PHPUnit tests afterwards
…Cake, so no need to have special logic for it
…Info() might return null on errors leading to a possible NPE. Static code analysis B does not know that $fileInfo->getPathInfo() might return null and therefore complains that the comparison is always true. Fix one of them, ignore the other
… so let's update to psalm 5
…cake\cache\cacheengineinterface" error, so let's upgrade to a more current version
…elease to support psr log 3
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## cake5 #326 +/- ##
========================================
Coverage ? 53.72%
Complexity ? 213
========================================
Files ? 10
Lines ? 590
Branches ? 0
========================================
Hits ? 317
Misses ? 273
Partials ? 0 ☔ View full report in Codecov by Sentry. |
src/View/PdfView.php
Outdated
| /** | ||
| * @inheritDoc | ||
| */ | ||
| public function getConfig(?string $key = null, mixed $default = null): mixed |
There was a problem hiding this comment.
Cake 5 makes the base method protected, but it's used in the unit tests. I opened it again, but i'm also open to other solutions.
There was a problem hiding this comment.
The protected visibility is a mistake in the core. It will be fixed in 5.0.1.
There was a problem hiding this comment.
I guess we'll wait for te release of 5.0.1 here, then.
There was a problem hiding this comment.
This class also needs
/**
* Mime-type this view class renders as.
*
* @return string Either the content type or '' which means no type.
*/
public static function contentType(): string {
return '...;
}
It seems.
There was a problem hiding this comment.
With this fragment, the testRenderErrorTemplate fails and i'm wondering, if that test actually makes sense. I think i do not know enough about Cakes Error handling to judge that.
src/View/PdfView.php
Outdated
| /** | ||
| * @inheritDoc | ||
| */ | ||
| public function getConfig(?string $key = null, mixed $default = null): mixed |
There was a problem hiding this comment.
The protected visibility is a mistake in the core. It will be fixed in 5.0.1.
|
Do you have any plans to split the Get/Set methods in CakePdf in getters and setters? That way, you could use proper typing for the return types instead of mixed. A Major release might be a good opportunity to introduce such a breaking change |
I personally don't have the time to make non essential changes but I am fine with it if someone wants to submit a PR. |
…rkaround for a php 5.0.0 bug again
I tried to give speaking commit messages. The code is tested with MPDF and Cake and PHP 8.2 and it works fine.