setTemplate creates invalid template record by weeblr · Pull Request #32710 · joomla/joomla-cms

weeblr · 2021-03-17T09:35:19Z

See issue #32709

Pull Request for Issue # .

Summary of Changes

Template record in application object now also has inheritable and parent properties which are not set by SiteApplication::setTemplate. This causes large numbers of PHP notices to be logged in various places.

Changes made is B/C, allowing either to pass in a full template definition object (with custom inheritable, params and parent props) or just the template name as a string.

Testing Instructions

Factory::getApplication()->setTemplate('new_template');

Actual result BEFORE applying this Pull Request

[17-Mar-2021 09:05:41 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:41 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 179
[17-Mar-2021 09:05:41 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Plugin\PluginHelper.php on line 61
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Plugin\PluginHelper.php on line 61
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Plugin\PluginHelper.php on line 61
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Error\Renderer\HtmlRenderer.php on line 76
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Document\Renderer\Html\MetasRenderer.php on line 130
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Document\Renderer\Html\MetasRenderer.php on line 130
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Document\Renderer\Html\MetasRenderer.php on line 134
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Document\Renderer\Html\MetasRenderer.php on line 130
[17-Mar-2021 09:05:42 UTC] PHP Notice:  Undefined property: stdClass::$parent in V:\dev\workspaces\products\src\j4\libraries\src\Application\SiteApplication.php on line 402

Expected result AFTER applying this Pull Request

No PHP notices.

Documentation Changes Required

See issue joomla#32709

joeforjoomla · 2021-03-17T09:46:34Z

@weeblr this is already related to this PR #30360
Nobody seems to care about fixing it

rdeutz · 2021-03-17T09:50:46Z

@joeforjoomla as long as testing doesn't run sucessfully we are not going to merge, that's different from not caring

joeforjoomla · 2021-03-17T10:00:53Z

@rdeutz as i see tests are successfully since months

rdeutz · 2021-03-17T10:23:50Z

@rdeutz as i see tests are successfully since months

currently not

weeblr · 2021-03-17T10:28:02Z

@rdeutz I fixed the 3 spaces codestyle in this PR.

In addition, this PR should address the comment by @SharkyKZ in #30360 (comment). It allows passing either a string (B/C) as template name, in which case parent and inheritable have default values (0 and ''). Or if $template is an object, it expects it to have inheritable and parent properties which are then assigned to the template property itself.

Side note: the failing test in the #30360 don't seem related to the PR itself, but rather the test system (composer issue).

Yannick

dgrammatiko · 2021-03-17T10:55:18Z

I have tested this item ✅ successfully on b828804

Thank you Yannick

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32710.}

richard67 · 2021-03-17T11:38:12Z

@weeblr Does this PR here replace #30360 by fixing the same issue in a better way? Or is that PR still needed when this one here will be merged? Am asking just to be sure I understand right.

dgrammatiko · 2021-03-17T11:41:10Z

@richard67 yes, this replaces the other PR and also adds the missing points that @SharkyKZ was pointed in there. So this is complete compared to the other PR

weeblr · 2021-03-17T11:42:40Z

@richard67 I was not aware of #30360 until @joeforjoomla mentioned it. It does solve the same problem, changing the same method, in a differrent way.

I feel my approach is more flexible but I'd leave up that to reviewers to decide.

richard67 · 2021-03-17T11:57:15Z

@dgrammatiko @weeblr Thanks for your feedback. I'll close the other PR in favour of this here.

dgrammatiko · 2021-05-12T16:39:35Z

@richard67 can you add a release blocker here?

richard67 · 2021-05-12T16:43:21Z

@richard67 can you add a release blocker here?

@dgrammatiko Why does that suddenly become a release blocker?

@wilsonge What's your opinion? Release blocker or not?

dgrammatiko · 2021-05-12T16:46:45Z

Why does that suddenly become a release blocker?

This PR replaced #30360 which was (or should have been RB). Anyways if this PR isn't merged is kinda hard to programmatically set a template (eg the API is broken)

richard67 · 2021-05-12T16:49:32Z

#30360 never had the release blocker label.

richard67 · 2021-05-12T16:56:52Z

I've asked for opinions in Glip in maintainers and JBS channel. Will see what they say.

joomdonation · 2021-05-13T05:23:36Z

I have tested this item ✅ successfully on b828804

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32710.}

richard67 · 2021-05-13T10:41:54Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32710.}

setTemplate creates invalid template record

a8df297

See issue joomla#32709

joomla-cms-bot added the PR-4.0-dev label Mar 17, 2021

richard67 mentioned this pull request Mar 17, 2021

[4.0] setTemplate does not create a valid template record #32709

Closed

Fixed whitespaces in docblock and empty line

b828804

richard67 mentioned this pull request Mar 17, 2021

Update SiteApplication.php to initialize properly the template object caused by #30192 #30360

Closed

rdeutz added this to the Joomla 4.0 milestone Mar 22, 2021

dgrammatiko mentioned this pull request Apr 11, 2021

[4.0] change template style #33098

Closed

joomla-cms-bot removed this from the Joomla 4.0 milestone May 13, 2021

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 13, 2021

richard67 added this to the Joomla 4.0 milestone May 13, 2021

rdeutz merged commit dad70fa into joomla:4.0-dev May 14, 2021

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 14, 2021

Uh oh!

Conversation

weeblr commented Mar 17, 2021

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

Uh oh!

joeforjoomla commented Mar 17, 2021

Uh oh!

rdeutz commented Mar 17, 2021

Uh oh!

joeforjoomla commented Mar 17, 2021

Uh oh!

rdeutz commented Mar 17, 2021

Uh oh!

weeblr commented Mar 17, 2021

Uh oh!

dgrammatiko commented Mar 17, 2021

Uh oh!

richard67 commented Mar 17, 2021

Uh oh!

dgrammatiko commented Mar 17, 2021

Uh oh!

weeblr commented Mar 17, 2021

Uh oh!

richard67 commented Mar 17, 2021

Uh oh!

dgrammatiko commented May 12, 2021

Uh oh!

richard67 commented May 12, 2021

Uh oh!

dgrammatiko commented May 12, 2021

Uh oh!

richard67 commented May 12, 2021

Uh oh!

richard67 commented May 12, 2021

Uh oh!

joomdonation commented May 13, 2021

Uh oh!

richard67 commented May 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants