Re: action list in form inheritance

[Adrian Crum]

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