[JBS] Call for Testers: Custom Fields for 3.7

[Niels Braczek]

Hello guys, we are close to 3.7 alpha 1. There are some easy PRs
available to improve custom fields. Testing instructions are clear and
should take not more than 5 minutes to test, some even do have already a
test:

https://github.com/joomla/joomla-cms/pull/12968
https://github.com/joomla/joomla-cms/pull/13069
https://github.com/joomla/joomla-cms/pull/13103
https://github.com/joomla/joomla-cms/pull/13175

Regards,
Niels

--
| New Stars on the Horizon: GreenCape · nibralab · laJoom |
| http://www.bsds.de · BSDS Braczek Software- und DatenSysteme |
| Webdesign · Webhosting · e-Commerce · Joomla! Content Management |
------------------------------------------------------------------

[Hannes Papenberg]

Hello,
I've brought this up internally several weeks ago, but since nothing has
happened, I'm bringing this up here too.

Please improve the code quality of the custom fields feature. I only
looked through JFormField and see loads of problems. So far, Joomla has
always tried to use understandable and sane method names and where
ambigious, we have extensive comments. We also normally have unittests.
This is currently not the case for at least JFormField for the fields
feature and I've seen equally poor code in other PRs.

To give you an example what is wrong with JFormField:

The last method of the class is named "getFormParameters", however that
seems to return an XML for this field. So at least name it
getFieldParameters and I would actually call it getFieldForm, since that
is what it does. The code itself uses some horrible reflection stuff to
get the directory of the original class file, which means that there is
no way to override that field form file completely. Why can't we use
something like the JFormHelper class with all the paths to lookup in?
Also, why can't we simply use the lowercase JFormField::$type of the
field to find the filename? Then it uses JFile::exists(), which our own
documentation says to use file_exists() instead, just to then use
JFile::read(), which the same documentation says to use
file_get_contents() instead.

The next method is named postProcessDomNode and looking at that, I have
no idea at all what that method is supposed to do. So I read the doc
block ... and am no wiser. The doc block is completely useless. The
method otherwise is empty.

The next method has an equally non-sensical name (appendXMLFieldTag) and
again the doc block has VERY little information. At least a little bit
more than the other method. Looking through this code, I first stumble
over a call to JFactory::getApplication() and checks against some
parameter and isSite() and isAdmin(). I don't think that a class like
JformField should be tied to JApplicationCms like this. Anyway... Then
there is some more code that I don't really want to review and at the
end we have a call to postProcessDomNode(). And I'm asking myself why we
have to have this postProcessDomNode() method at all. What is wrong with
overriding appendXMLFieldTag() and doing something like
public function appendXMLFieldTag() {
$node = parent::appendXMLFieldTag();
/** Do magic **/
return $node;
}
So we could simply drop the postProcessDomNode() method, since it is
completely useless.

This code does not raise confidence in this feature from me. We are
introducing a new feature here and with the active opposition of moving
to Joomla 4.0 soon AND the REALLY bad press we would get if we
introduced this here now and then completely rewrote it in 4.0 again,
breaking everyones code, we should get this right the first time. We are
stuck with the method names the moment we release this as a stable
release. Considering that we are still trying to get rid of methodnames
and attributes introduced in Joomla 1.5 (and been doing so since Joomla
1.6), this is extremely important! As I wrote a few weeks ago, I also
expect a feature like this to be covered by unittests and I refuse to
accept the explanation "Oh, we don't require unittests here, because
people don't know how to write them." If that is the case, then don't
put this code in the /libraries folder. Then you can also just rewrite
it whenever you want and actually simply provide it as a non-core
extension until it is ready for core. Writing unittests is no magic and
can be learned. But simply adding new code without tests is a sure way
for us to mess this up even more. (I would have used a different word
than "mess" but apparently there are very sensitive souls on this
mailinglist...) If unittests are such a big issue, we should rather
simply drop our testsuite and and not require any tests at all, since if
half the code can be added without tests, I don't see why I have to
waste my time writing such tests.

If we accept new features and new code into Joomla, it has to be done
properly and the code has to be usable AND serviceable by other people.
More fiaskos like with Tags or Finder are unacceptable.

Regards,
Hannes

[Allon Moritz]

@Hannes, can we please have hangout together to find a way how to solve this situation. I just don't have time to argument with you per Mail.

--
You received this message because you are subscribed to the Google Groups "Joomla! CMS Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to joomla-dev-cms+unsubscribe@googlegroups.com.
To post to this group, send an email to joomla-...@googlegroups.com.
Visit this group at https://groups.google.com/group/joomla-dev-cms.
For more options, visit https://groups.google.com/d/optout.

[Hannes Papenberg]

I wont be able to promise a time and date for a chat, but feel free to
contact me via Skype and I will get back to you as soon as possible. If
you need my Skype contact, please write me personally.

Hannes

Am 14.12.2016 um 13:53 schrieb Allon Moritz:
> @Hannes, can we please have hangout together to find a way how to solve
> this situation. I just don't have time to argument with you per Mail.
>
> On Wed, Dec 14, 2016 at 1:41 PM, 'Hannes Papenberg' via Joomla! CMS
> Development <joomla-...@googlegroups.com

> <https://github.com/joomla/joomla-cms/pull/12968>
> https://github.com/joomla/joomla-cms/pull/13069
> <https://github.com/joomla/joomla-cms/pull/13069>
> https://github.com/joomla/joomla-cms/pull/13103
> <https://github.com/joomla/joomla-cms/pull/13103>
> https://github.com/joomla/joomla-cms/pull/13175

> <https://github.com/joomla/joomla-cms/pull/13175>
>
> Regards,
> Niels
>
>
> --
> You received this message because you are subscribed to the Google
> Groups "Joomla! CMS Development" group.
> To unsubscribe from this group and stop receiving emails from it,

> send an email to joomla-dev-cm...@googlegroups.com
> <mailto:joomla-dev-cms%2Bunsu...@googlegroups.com>.

> To post to this group, send an email to
> joomla-...@googlegroups.com

> <mailto:joomla-...@googlegroups.com>.

> Visit this group at https://groups.google.com/group/joomla-dev-cms

> <https://groups.google.com/group/joomla-dev-cms>.

> For more options, visit https://groups.google.com/d/optout

> <https://groups.google.com/d/optout>.

>
>
> --
> You received this message because you are subscribed to the Google
> Groups "Joomla! CMS Development" group.
> To unsubscribe from this group and stop receiving emails from it, send

> an email to joomla-dev-cm...@googlegroups.com
> <mailto:joomla-dev-cm...@googlegroups.com>.
> To post to this group, send email to joomla-...@googlegroups.com
> <mailto:joomla-...@googlegroups.com>.

[Bakual]

Please also open an issue so it can be tracked down. I partly agree with the method name issue and the reflection stuff sure isn't ideal either. However just writing it in this Google Group it will get lost.

> <mailto:joomla-dev-cms+unsub...@googlegroups.com>.

[Angie Radtke]

Hi Guys,
looking forward having customs field .-). Thanks for your work.
I think this tool is very helpful.

Here my quick test results:

*Backend: *
Fieldtype 'number' missing -> Postal code etc.
Fieldtype textarea needs an Editor.
After saving field, fieldtype couldn't be changed -> Is there any
reason for that I dont't understand?


*Frontend:*
It seems that field->values are not searchable?
Template-Overides

var_dump($this->item->fields);

I wonder, returned array starts counting with 2 ?
I guess the reason for that is that I already deleated 2 items.

Merry Christmas and a happy new year

Angie

[Angie Radtke]

> Fieldtype textarea needs an Editor.

Sorry , haven't seen the editor field type

Angie

[René Alain Erichsen]

On Wednesday, 21 December 2016 10:28:06 UTC+1, a.radtke wrote:

Hi Guys,
looking forward having customs field .-). Thanks for your work.
I think this tool is very helpful.

Here my quick  test results:

*Backend: *
Fieldtype 'number' missing  -> Postal code etc.

Indeed, there are several missing field types. Filelist is the one I miss the most. Can't create a field to select a PDF file, for example. For now I'm resorting to a Text field and manual typing. :(

 

Fieldtype textarea needs an Editor.
After saving field, fieldtype couldn't  be changed -> Is there any
reason for that I dont't understand?

This may have something to do with the varying data structures required to support the different field types, making it not trivial.

  

*Frontend:*
It seems that field->values are not searchable?

I think they are supposed to be searchable with the old Search plugin, not sure about Smart Search at the moment.

 

Template-Overides

var_dump($this->item->fields);

I wonder, returned array starts counting with 2 ?
I guess the reason for that is  that I already deleated 2 items.

The keys are the field ids, so you can get the value that was entered in a specific field with $this->item->fields[id]->rawvalue; and use it in your override if you like.

/R

[Bakual]

Hi Angie

Best is to open issues on GitHub, so they don't get lost.

> Fieldtype 'number' missing  -> Postal code etc. 

If Allons PR to rewrite the fields to plugins gets merged then it will be very easy to create custom filetypes or add others. PR is https://github.com/joomla/joomla-cms/pull/13319 but will be expanded with the other types.
You can open an issue and request the number fieldtype to be added. This way we don't forget it. Maybe there was a reason it is missing but I don't know it.

> After saving field, fieldtype couldn't  be changed -> Is there any reason for that I dont't understand? 

The filetype also defines various additional parameters that get loaded. When changing the fieldtype the whole form gets reloaded to get the new parameters.
That means if you're changing the type, you likely would loose existing data. For that it makes sense that you can't change it afterwards.

> It seems that field->values are not searchable? 

Smart Seach has a bug. It doesn't update the field values always and you need to save the item twice to get it indexed. Chris already wrote a partial solution for it.
Conventional Search should work, but has some issues as well. There is a PR from me to fix that. However with conventional search it will not be possible to search for values from "list" types (eg selects, checkboxes) since only the value is stored in the database and not the text representation of it. So search for the "Yes" option in a "Yes"/"No" select will not work, but text fields should work.

> I wonder, returned array starts counting with 2 ? 

The key of the array is the ID of the field.