? Pending

User tests: Successful: Unsuccessful:

avatar laoneo
laoneo
30 Mar 2017

Pull Request for Issue #15004.
Fixes PHP Notice: Undefined property: JTableLanguage::$id in ROOT/plugins/system/fields/fields.php on line 149.

Code review.

avatar laoneo laoneo - open - 30 Mar 2017
avatar laoneo laoneo - change - 30 Mar 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 30 Mar 2017
Category Front End Plugins
avatar infograf768
infograf768 - comment - 30 Mar 2017

This works.
But I am concerned that this system plugin has impacts on totally unrelated stuff like Content Languages.
Basically, where else could it impact ?

avatar infograf768 infograf768 - test_item - 30 Mar 2017 - Tested successfully
avatar infograf768
infograf768 - comment - 30 Mar 2017

I have tested this item successfully on db0d277

OK, but I still worry about this plugin impact.


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

avatar Bakual
Bakual - comment - 30 Mar 2017

But I am concerned that this system plugin has impacts on totally unrelated stuff like Content Languages.
Basically, where else could it impact ?

It impacts all extensions of course. That's what system plugins are supposed to do. But it will not break anything since it will only try to delete field values. Since no fields are present nothing will happen beside a useless query.

Another way to fix it would be to use the presence of the "validateSection" method (used in Fieldshelper::extract) in an extension to determine if the following could should run or not.

avatar joomdonation
joomdonation - comment - 31 Mar 2017

Another way to fix it would be to use the presence of the "validateSection" method (used in Fieldshelper::extract) in an extension to determine if the following could should run or not.
Hide all checks

That would be a better / preferred fix than this one.

avatar laoneo
laoneo - comment - 31 Mar 2017

Fieldshelper::extract()is already checking validateSection. So there is no reason to do that twice.

avatar Bakual
Bakual - comment - 31 Mar 2017

Fieldshelper::extract()is already checking validateSection. So there is no reason to do that twice.

It's an optional check in Fieldshelper::extract(), that's what I mean. It will only be checked if the method exists.
Com_languages doesn't have that method (because it doesn't support fields) and thus the context is returned as is and the code is processed further.
We could change it so that method is a requirement for fields to work. Basically a "context supports fields" type of check. Then the code would stop right there because the returned context from Fieldshelper::extract() would be false.

Just an idea, it may have other unwanted side effects ?

avatar laoneo
laoneo - comment - 31 Mar 2017

A side effect would be, when an extension developer will integrate com_fields, it will just not work. Till she/he figures out that it needs a validateSection function in a helper class the component probably doesn't even have.

avatar Bakual
Bakual - comment - 31 Mar 2017

Yep, that needs to be documented of course. Tobias started the doc already (see https://docs.joomla.org/User:Zero24#Implement_fields_into_your_component for his wip) and that one would have to be added there as well.

avatar franz-wohlkoenig franz-wohlkoenig - change - 31 Mar 2017
Category Front End Plugins com_fields Front End Plugins
avatar franz-wohlkoenig franz-wohlkoenig - change - 31 Mar 2017
Status Pending Discussion
avatar franz-wohlkoenig franz-wohlkoenig - test_item - 1 Apr 2017 - Tested successfully
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 1 Apr 2017

I have tested this item successfully on db0d277


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

avatar franz-wohlkoenig franz-wohlkoenig - change - 1 Apr 2017
Status Discussion Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 1 Apr 2017

RTC after two successful testes.

avatar wilsonge wilsonge - change - 1 Apr 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-04-01 10:56:37
Closed_By wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 1 Apr 2017
avatar wilsonge wilsonge - merge - 1 Apr 2017
avatar joomla-cms-bot joomla-cms-bot - change - 1 Apr 2017
Category Front End Plugins com_fields Front End Plugins
avatar laoneo
laoneo - comment - 1 Apr 2017

Thanks!

Add a Comment

Login with GitHub to post a comment