Bug #13804 for Maypole-FormBuilder: possible generic solution to choosing subsets of columns intelligently

Wed Jul 20 17:01:38 2005 Guest - Ticket created

Subject:

possible generic solution to choosing subsets of columns intelligently

In your normal CRUD database application, columns fall into at least three discrete groups: -- displayable, those that can be displayed in the interface (presumably most columns but not all) -- editable, those that can be specified in adds and edits -- required, those that must be specified in adds and edits (not necessarily the same as "not null" columns, because some of those may be primary keys that are auto-generated but not editable, although they may still be displayable) This model no doubt still a simplification of what is necessary in "real" situations, but is still an improvement on the current Maypole and Maypole + FormBuilder model. It's actually not very difficult to implement consistently. I was able to do so in about five minutes by making use of Class::DBI's column grouper (incidentally, I have no idea why Maypole bothered with the display_columns sub, which seems rather ugly given CDBI's column grouper is sitting right there). Here's an example: __PACKAGE__->columns(Display => qw/ table_id item_id ref description num_values /); __PACKAGE__->columns(Required => qw/ ref description /); __PACKAGE__->columns(Editable => qw/ ref description compound /); I wrote a small thing to automatically create a display_columns sub returning the Display group of columns. It's then quite a simple matter making the Maypole::FB templates aware of this. For addnew, you add these args: fields => [ $request->model_class->columns('Editable') ], required => [ $request->model_class->columns('Required') ], For search, you add: fields => [ $request->model_class->display_columns ], Ditto for editlist. Unfortunately, there's not yet a way to combine forms with uneditable fields in a table row, but I'm working on that. :-) I suppose one argument for the display_columns sub over the CBDI column grouper is that the former supports dynamic changes in column display. Perhaps, then, it would be preferably to add editable_columns and required_columns subs. Now, I understand that this is arguably simply a template-level issue, but it seems to me that given the tedium involved in this sort of thing, and the frequency with which it's bound to come up, it would be worthwhile to add some generic functionality of this sort to Maypole and/or Maypole::FB. Perhaps I will submit a proposal to the Maypole mailing list. In the meantime, these ramblings will have to do.

Thu Jul 21 03:36:59 2005 davekam [...] pobox.com - Correspondence added

From:	David Kamholz <davekam [...] pobox.com>
Subject:	[cpan #13804] Re: possible generic solution to choosing subsets of columns intelligently
Date:	Thu, 21 Jul 2005 09:36:44 +0200
To:	bug-Maypole-FormBuilder [...] rt.cpan.org
RT-Send-Cc:

Hmmm.... well, I suppose I made things a bit over-complicated in my last proposal. All that's really necessary is support for a sub editable_columns which an improved tabulate() will be able to read in order to render the listedit mode properly. All the rest can probably done just by passing the necessary fields args to FB from templates. There's also a more marginal case to be made for a sub required_columns -- it could just go in form_builder_defaults, but you don't necessary want it everywhere (mainly not in search forms, I imagine). Beyond this, there's unlikely to be too many things that are widely generalizable. I think the main point is just that, if there's a certain consistent set of arguments or properties you want to associate with each table, it's much easier to do so in the model classes than creating separate templates table by table. To the extent that these properties may interact with the internals of functions (like tabulate), and may also be widely applicable, there's a case for including a standard way of specifying them in the distribution. Anyway, apologies for my over-exuberance last time. :-)

Thu Jul 21 04:38:44 2005 cpan.zerofive [...] googlemail.com - Correspondence added

[davekam@pobox.com - Thu Jul 21 03:36:59 2005]: Show quoted text

> Hmmm.... well, I suppose I made things a bit over-complicated in my > last proposal. All that's really necessary is support for a sub > editable_columns which an improved tabulate() will be able to read in > order to render the listedit mode properly. All the rest can probably > done just by passing the necessary fields args to FB from templates. > There's also a more marginal case to be made for a sub > required_columns -- it could just go in form_builder_defaults, but > you don't necessary want it everywhere (mainly not in search forms, I > imagine). Beyond this, there's unlikely to be too many things that > are widely generalizable. I think the main point is just that, if > there's a certain consistent set of arguments or properties you want > to associate with each table, it's much easier to do so in the model > classes than creating separate templates table by table. To the > extent that these properties may interact with the internals of > functions (like tabulate), and may also be widely applicable, there's > a case for including a standard way of specifying them in the > distribution.

Yes, ATM there's quite a mix: - display_columns() - is used to decide what columns go in forms - CDBI::FB sets the 'required' flag on has_a columns - the Required column group, if present, also gets the 'required' flag set (although this is deprecated) - or you can list required fields in form_builder_defaults, or in the call to as_form() - fields can be made displayable but not editable by setting the readonly HTML attribute (not sure if you can do this via form_builder_defaults, but you could either post-process the form, or use the DISABLED preprocessor, and it would be easy to add a READONLY preprocessor, or the inverse, an EDITABLE preprocessor). But disabled doesn't seem to be working, or is not what I thought it was. The editable_columns() method makes sense to me. I'd be interested in what other users thought, maybe you could raise it on the dev list. It would in principle be nice to keep the API for FB as close as possible to the standard model interface, and therefore it'd be nice if they agreed to add this to the standard model. In practice, there's no reason not to go ahead in FB. I'm a bit uncomfortable slapping lots of new column groups on people's CDBI classes. So for instance, I've deprecated using the Required group because I imagine that could be quite a common keyword. Same would go for Editable. I also think it's nicer to have one place to set form rendering attributes - in form_builder_defaults. We could add an 'editable' attribute there. Column groups have the potential to alter the behaviour of CDBI objects (if someone is already using a label for something else), whereas form_builder_defaults doesn't. On the other hand, Maypole already uses a Stringify column group. tabulate() is certainly a bottleneck. I think we need a documentation section that lists all the different ways a form field can be rendered (required, editable, hidden, with fields from a related class [eventually], etc), and then we can look at tabulate and see how it copes with each. d.

Tue Aug 09 05:21:01 2005 cpan.zerofive [...] googlemail.com - Correspondence added

I've gone in the direction of adding (lots) more column group methods, which all default to one of the standard Maypole methods. So currently there's edit_columns and edit_fields etc. Most *_columns methods need a matching *_fields method to list has_many fields. This isn't complete yet, and they aren't yet used consistently throughout the templates and modules, but they will be in time. In general, you might want a dedicated pair of methods for each major template (edit, view, list etc.), so as you add your own custom templates, you may want to add more of these methods. Or just use column groups, but you'd still need an appropriate *_fields method. The nice thing about using methods is that you can define a sensible default in the base class, and then override that method in specific subclasses, whereas with column groups, you need to define them separately in each subclass. d.

Tue Aug 09 05:21:02 2005 cpan.zerofive [...] googlemail.com - Status changed from 'new' to 'resolved'