User tests: Successful: Unsuccessful:
Related to #30192
Add the new properties 'parent' and 'inheritable' to the object initialized in the 'setTemplate' method so that there are no 'Notice' if a plugin calls the 'setTemplate' method to override the template used
Call the 'setTemplate' method from a system plugin
Notice for undefined property 'parent'
No PHP notice
No
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
@joeforjoomla sure something I overlooked, but I guess hardcoding the params to null and 0 will only work for the legacy mode. The function should be like:
public function setTemplate($template, $styleParams = null, $inheritable = 0, $parent = '')
{...}
So it can work for all cases
Yes @dgrammatiko actually the setTemplate should have new parameters to allow setting both new properties. That would be ideal.
Labels |
Added:
?
|
@joeforjoomla i've updated this PR accordingly
@joeforjoomla @wilsonge I was wrong here, we don't need to change the signature of the function, check joeforjoomla#1
Testing Instructions
Call the 'setTemplate' method from a system template
@joeforjoomla From a system template? Or from a system plugin?
@richard67 ops sorry.. obviously system plugin
No problem, can happen.
Unit test failure is not related to this PR.
I have tested this item
@joeforjoomla It doesn't make sense to test your own PR since the rules say that when you propose a PR, you have to test it yourself anyway.
I have tested this item
I have tested this item
RTC
Status | Pending | ⇒ | Ready to Commit |
RTC
Status | Ready to Commit | ⇒ | Pending |
Back to pending as there are unaddressed comments.
@joeforjoomla Is @JoeJoomlaCoder also an account from you?
I have tested this item
Called setTemplate() method in a System Plugin -> PHP Notice
After applied Patch -> no PHP Notice Warning
Called setTemplate() method in a System Plugin -> PHP Notice
After applied Patch -> no PHP Notice Warning
Tested successfully
This PR still has open review comments and also fail the PHP code style test by drone.
For fixing the latter I'll suggest a correction in a minute.
But as long as the review comments are not addressed I can't really set RTC.
Sorry, my fault. The issues Joomla Site update not correctly after posting "Test this". So I post it again and it was double.
Labels |
Added:
?
|
Labels |
Added:
?
Removed: ? |
@richard67 any update on this? Can we get it merged quickly? What is still missing?
@joeforjoomla please address this comment #30360 (comment)
@dgrammatiko i don't think that #30360 is a good idea, having a string or an object assigned to the property $this->template->template is not an option, indeed extension have always assumed that this is a string matching the template name. However #30360 is quite unrelated to this PR.
having a string or an object assigned to the property $this->template->template is not an option
I guess that was an example code and they meant $this->template = $template
. The $this->template->template
is the value of the db - > table extensions -> column element and it's a string. Other than that the suggestion is solid and tbh expected behaviour
Ok but honestly i don't understand what i should do here in relation to this PR. Eventually that can be merged as a separate PR.
This is still an issue. What to do to merge a fix?
Creating a fake account in order to mark that you had tested something doesnt really inspire people to accept that test result.
If you want people to test something that is not trivial then you need to give test instructions that are easy to follow
@brianteeman this is an open bug, nobody is interested to create fake accounts but just fix the Joomla code
Labels |
Added:
?
Removed: ? |
Closing in favour of #32710 . @joeforjoomla Sorry, I hope you are not too angry or disappointed. Feel free to re-open here if you think it is a mistake that I closed it.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-03-17 11:58:59 |
Closed_By | ⇒ | richard67 | |
Labels |
Added:
?
Removed: ? |
@dgrammatiko you are here