Sounds fine to me. One small suggestion: to be more consistent with
the existing "extends" attribute, maybe we should have something like
extends-actions.
-David
On Aug 31, 2009, at 9:47 PM, Harmeet Bedi wrote:
To me main main motivation for raising this is that the rules of
ofbiz are sometimes hidden for non expert users and current override
may be a pitfall.
How about this:
Have 2 attributes to a form with default values.
1. parent-actions with default override and options append(super
semantics), override and ignore(for completion). Details documented
below
2. parent-row-actions. Similar to (1) for rows.
Since default is override, the behavior will be backwards
compatible, defaults ensure no need to change any existing forms,
rules are exposed xsd documentation so easier for users to know and
system is more flexible.
widget-form.xsd
<xs:attributeGroup name="attlist.form">
...
<xs:attribute name="parent-actions" default="override">
<xs:annotation>
<xs:documentation>If form derives from parent, form actions may
override existing parent form actions, append to parent form
actions or ignore
parent form actions</xs:documentation>
</xs:annotation>
<xs:simpleType>
<xs:restriction base="xs:token">
<xs:enumeration value="append">
<xs:annotation>
<xs:documentation>append form actions to list of inherited
parent form actions</xs:documentation>
</xs:annotation>
</xs:enumeration>
<xs:enumeration value="override">
<xs:annotation>
<xs:documentation>If action block exists, ignore parent
action list.
If action block does not exist use the
parent action list
</xs:documentation>
</xs:annotation>
</xs:enumeration>
<xs:enumeration value="ignore">
<xs:annotation>
<xs:documentation>Ignore parent form actions.
Same as override with no actions
specified in actions block.
</xs:documentation>
</xs:annotation>
</xs:enumeration>
</xs:restriction>
</xs:simpleType>
</xs:attribute>
...
</xs:attributeGroup>
if ok, will put together patch.
Harmeet
----- Original Message -----
From: "David E Jones" <d...@me.com>
To: dev@ofbiz.apache.org
Sent: Monday, August 31, 2009 8:18:00 PM GMT -05:00 US/Canada Eastern
Subject: Re: action list in form inheritance
Yes, that would be another way... ie don't call parent form actions
unless it is explicitly specific as opposed to calling it unless
something says not to call it.
-David
On Aug 31, 2009, at 5:23 PM, Adrian Crum wrote:
Maybe what the actions section needs is an element that would
duplicate super().
-Adrian
David E Jones wrote:
Harmeet,
The original intent of this design was to be kind of like a method
in a class in that if you specify a method with the same name in a
sub-class then it will override the method in the parent class so
that none of that method is run.
The basic problem is what if you didn't want one or more of the
actions in the parent form to run, how would you do it? Because
there is no way to specify which if the parent actions you want or
don't want (unless we did some sort of action blocks with names and
you could include/exclude them explicitly), the idea is that you
either use the parent actions (like a parent class method), or you
override the action block and specify the ones you want to run.
As for my opinion on it, as long as we have a way to get it to not
run the parent actions (ie for cases when they are not desired or
they conflict with other things that need to be done in the sub-
form) then some variation on what it currently does would be fine.
-David
On Aug 30, 2009, at 9:28 PM, Harmeet Bedi wrote:
Actions seem to modeled part of form construction. e.g. when i
construct a
form at runtime it first runs all the actions and then creates
fields.
Sometimes it may create fields based on variables set in action
block and
use-when conditions.
Now this construction step breaks down when i have a form
inheriting from
another form. If the form does not define any actions, it gets the
action
list from it's parent and everything is ok.
But if a form has any actions, even a 'set' for a field it masks
all the
actions of the parent form.
In ModelForm
public void initForm(Element formElement) {
...
if ( parent != null ) {
...
this.actions = parent.actions;
...
}
...
Element actionsElement = UtilXml.firstChildElement(formElement,
"actions");
if (actionsElement != null) {
this.actions = ModelFormAction.readSubActions(this,
actionsElement);
}
...
}
I was thinking of form actions as a list of steps that constitute
runtime
form construction. It is instead done as an override. This is
problematic
and a potential pitfall to me.
I would expect a user of ofbiz would have this interaction
- They see a form, like it fields and behaviour and then decide to
inherit
and extend it to add some value.
- User decides to alter form actions to add some fields and alter
some
actions. Suddenly ancestor form breaks unless all the form
actions in each
ancestor are copied or somehow called from leaf form. Now often
users will
not know this so will be left scratching their head for hours on
why the
form that they liked does not behave as they expected.
To me it would be better to append actions as in:
in ModelForm
public void initForm(Element formElement) {
...
if ( parent != null ) {
...
this.actions.appendAll(parent.actions);
...
}
...
Element actionsElement = UtilXml.firstChildElement(formElement,
"actions");
if (actionsElement != null) {
this.actions.appendAll(ModelFormAction.readSubActions(this,
actionsElement));
}
...
}
Reasons:
It would be more consistent with inheritance semantics in
construction -
that is when constructing a derived object - if constructor in
base class
has functions and constructor in derived class has functions, the
functions
in base class are run followed by functions in derived class.
Users will inherit and add to form fields and actions. Users would
ideally
want to add limited value to base forms and want to keep it's
behaviour
unchanged.
It could make form inheritance simpler to get for new users.
I did a walk through of existing ofbiz code. This change does not
seem to
adversely impact.
Does this seem sensible. thoughts ?
Harmeet
