User tests: Successful: Unsuccessful:
Pull Request for Issue #15004.
Fixes PHP Notice: Undefined property: JTableLanguage::$id in ROOT/plugins/system/fields/fields.php on line 149
.
Code review.
Status | New | ⇒ | Pending |
Category | ⇒ | Front End Plugins |
I have tested this item
OK, but I still worry about this plugin impact.
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.
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.
Fieldshelper::extract()
is already checking validateSection
. So there is no reason to do that twice.
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
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.
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.
Category | Front End Plugins | ⇒ | com_fields Front End Plugins |
Status | Pending | ⇒ | Discussion |
I have tested this item
Status | Discussion | ⇒ | Ready to Commit |
RTC after two successful testes.
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:
?
|
Category | Front End Plugins com_fields | ⇒ | Front End Plugins |
Thanks!
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 ?