? ? Pending

User tests: Successful: Unsuccessful:

avatar joeforjoomla
joeforjoomla
13 Aug 2020

Related to #30192

Summary of Changes

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

Testing Instructions

Call the 'setTemplate' method from a system plugin

Actual result BEFORE applying this Pull Request

Notice for undefined property 'parent'
image

Expected result AFTER applying this Pull Request

No PHP notice

Documentation Changes Required

No

avatar joeforjoomla joeforjoomla - open - 13 Aug 2020
avatar joeforjoomla joeforjoomla - change - 13 Aug 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 13 Aug 2020
Category Libraries
avatar joeforjoomla
joeforjoomla - comment - 13 Aug 2020

@dgrammatiko you are here

avatar dgrammatiko
dgrammatiko - comment - 13 Aug 2020

@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

avatar joeforjoomla
joeforjoomla - comment - 13 Aug 2020

Yes @dgrammatiko actually the setTemplate should have new parameters to allow setting both new properties. That would be ideal.

avatar joeforjoomla joeforjoomla - change - 13 Aug 2020
Labels Added: ?
avatar joeforjoomla
joeforjoomla - comment - 13 Aug 2020

@joeforjoomla i've updated this PR accordingly

avatar dgrammatiko
dgrammatiko - comment - 13 Aug 2020

@joeforjoomla @wilsonge I was wrong here, we don't need to change the signature of the function, check joeforjoomla#1

avatar richard67
richard67 - comment - 13 Aug 2020

Testing Instructions

Call the 'setTemplate' method from a system template

@joeforjoomla From a system template? Or from a system plugin?

avatar joeforjoomla joeforjoomla - change - 13 Aug 2020
The description was changed
avatar joeforjoomla joeforjoomla - edited - 13 Aug 2020
avatar joeforjoomla
joeforjoomla - comment - 13 Aug 2020

@richard67 ops sorry.. obviously system plugin

avatar richard67
richard67 - comment - 13 Aug 2020

No problem, can happen.

avatar richard67
richard67 - comment - 14 Aug 2020

Unit test failure is not related to this PR.

avatar joeforjoomla joeforjoomla - test_item - 21 Aug 2020 - Tested successfully
avatar joeforjoomla
joeforjoomla - comment - 21 Aug 2020

I have tested this item successfully on 578166f


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30360.

avatar richard67
richard67 - comment - 21 Aug 2020

@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.

avatar richard67 richard67 - alter_testresult - 21 Aug 2020 - joeforjoomla: Not tested
avatar dgrammatiko dgrammatiko - test_item - 21 Aug 2020 - Tested successfully
avatar dgrammatiko
dgrammatiko - comment - 21 Aug 2020

I have tested this item successfully on 578166f


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30360.

avatar JoeJoomlaCoder JoeJoomlaCoder - test_item - 26 Aug 2020 - Tested successfully
avatar JoeJoomlaCoder
JoeJoomlaCoder - comment - 26 Aug 2020

I have tested this item successfully on 578166f


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30360.

avatar joeforjoomla
joeforjoomla - comment - 26 Aug 2020

RTC

avatar Quy Quy - change - 4 Sep 2020
Status Pending Ready to Commit
avatar Quy
Quy - comment - 4 Sep 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30360.

avatar infograf768 infograf768 - change - 5 Sep 2020
Status Ready to Commit Pending
avatar infograf768
infograf768 - comment - 5 Sep 2020

Back to pending as there are unaddressed comments.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30360.

avatar richard67
richard67 - comment - 30 Sep 2020

@joeforjoomla Is @JoeJoomlaCoder also an account from you?

avatar rmittl rmittl - test_item - 17 Oct 2020 - Tested successfully
avatar rmittl
rmittl - comment - 17 Oct 2020

I have tested this item successfully on 578166f

Called setTemplate() method in a System Plugin -> PHP Notice
After applied Patch -> no PHP Notice Warning


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30360.

avatar rmittl
rmittl - comment - 17 Oct 2020

Called setTemplate() method in a System Plugin -> PHP Notice
After applied Patch -> no PHP Notice Warning
Tested successfully


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30360.

avatar richard67
richard67 - comment - 17 Oct 2020

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.

avatar rmittl
rmittl - comment - 17 Oct 2020

Sorry, my fault. The issues Joomla Site update not correctly after posting "Test this". So I post it again and it was double.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30360.

avatar joeforjoomla joeforjoomla - change - 17 Oct 2020
Labels Added: ?
avatar joeforjoomla joeforjoomla - change - 17 Oct 2020
Labels Added: ?
Removed: ?
avatar joeforjoomla
joeforjoomla - comment - 19 Jan 2021

@richard67 any update on this? Can we get it merged quickly? What is still missing?

avatar dgrammatiko
dgrammatiko - comment - 19 Jan 2021

@joeforjoomla please address this comment #30360 (comment)

avatar joeforjoomla
joeforjoomla - comment - 19 Jan 2021

@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.

avatar dgrammatiko
dgrammatiko - comment - 19 Jan 2021

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

avatar joeforjoomla
joeforjoomla - comment - 19 Jan 2021

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.

avatar joeforjoomla
joeforjoomla - comment - 14 Mar 2021

This is still an issue. What to do to merge a fix?

avatar brianteeman
brianteeman - comment - 14 Mar 2021

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

avatar joeforjoomla
joeforjoomla - comment - 14 Mar 2021

@brianteeman this is an open bug, nobody is interested to create fake accounts but just fix the Joomla code

avatar rdeutz rdeutz - change - 17 Mar 2021
Labels Added: ?
Removed: ?
avatar richard67
richard67 - comment - 17 Mar 2021

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.

avatar richard67 richard67 - close - 17 Mar 2021
avatar richard67 richard67 - change - 17 Mar 2021
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2021-03-17 11:58:59
Closed_By richard67
Labels Added: ?
Removed: ?

Add a Comment

Login with GitHub to post a comment