More custom field revisions

[Gervase Markham]

Sean McAfee wrote:
> Here are some additional revisions.  

OK, here are my comments. Sorry for the delay :-)

> Each custom field has a name, consisting of up to 255 characters.  A
> field's name may not contain any vertical whitespace characters
> (carriage return, newline, and vertical tab), nor may it begin or end
> with whitespace. 

There are two things we need - a name to refer to it internally, and a 
display name. The display name should be solely the preserve of 
templates (with the internal name used if none is defined) and the 
internal name should be suitable for use as a form field name and 
database table/column name. This means that, for max compatibility, it 
should be alphanumeric only, with no whitespace.

> Each field also has a type: one of Integer, String,
> Date, or Selection.  The Integer, String, and Date types are
> collectively referred to as "scalar" types.

I still think we should have single-select (a scalar type) and 
multi-select. This prevents the need for "sub-types" for the Selection type.

<later>Well, I suppose other types have sub-types too...</later>

Did we decide to leave Email Address until version 2?

> If the users enters no text at all (including whitespace) into a
> String form element, the field will be set to the undefined value.

Is this right? Surely the empty string is a better representation in 
this case?

> A Date field describes a particular moment of time.  It is represented
> on Bugzilla's web interface as a single-line text form element.

I don't think you should specify this explicitly. It should be possible 
to have a variety of representations for Date fields - i.e. you do
[% INCLUDE date_field date = "10035356674" type="date-time" %]
or similar and the template defines the representation.

> Each Date field is flagged as belonging to one of two varieties:
> date-only and date/time.  A date-only Date field is represented using
> the format 'YYYY-MM-DD'; a date/time Date field is represented using
> the format 'YYYY-MM-DD HH24:MM:SS'.  (Arbitrary whitespace may
> separate the two components in the latter case.)  When a user enters
> text into a Date form element, it must conform to the appropriate
> format, or Bugzilla will signal an error condition.

I think that dates should always be displayed as above, but dates should 
be able to be input in any format which can be unambiguously understood 
by Date::Parse. Let's not enforce restrictions we don't need.

> A single-selection field's value is a subset of the field's value set
> of size either zero or one.  It is represented on Bugzilla's web
> interface as a drop-down listbox form element.  A field whose value is
> the empty set is represented on the Web interface by a special "unset"
> string which is defined when the field is created.

Again, this is a display issue that should be defined by templates, with 
the default if none is defined being the empty string.

> A multiselection field's value is an arbitrary subset of the field's
> value set.  It is represented on Bugzilla's web interface as a
> multiline listbox in which multiple simultaneous selections are
> allowed.  The listbox's vertical size is equal to the value set's size
> or five, whichever is smaller.

The current ones on query.cgi are 7, which seems like a nice size to me. 
  And they are all the same size even if some have less than seven 
elements, which looks better. But this is a minor display issue.

> Whenever a user edits a bug and changes one or more custom fields, the
> changes must be logged. 
<snip>

Are all these requirements met by the existing Bug Activity system?

> On "enter_bug.cgi" and "long_list.cgi", the custom fields will be
> shown just after the "Description" text box.

It's not an automatic given that the custom field will be needed on 
enter_bug. In fact, an argument could be made for not including any of 
them by default.

> Here is an alternate approach.  Global custom fields are always
> displayed.  A button is to be added to the query page, to the right of
> the "Product", "Component", and "Version" listboxes, labelled "Show
> custom fields for these projects".  If the user clicks it, the query
> form is redisplayed, augmented with all custom fields in all of the
> selected projects.  

_Any_ of the selected projects?

> If no projects were selected, or if no selected
> project has any custom fields, the page is simply redisplayed as it
> was before.

The button should never appear if there are no product-specific custom 
fields. Helper JS should disable the button if the selected products 
have no custom fields. Remember to display all possible custom fields if 
no products are selected. :-)

The appearing/disappearing should be done using JS and CSS without a 
page reload if the browser supports JS, by having an onsubmit="doIt(); 
return false" on the button.

> When custom fields are being shown, a second button is to be added
> below the "Show custom fields" button, labelled "Hide custom fields".
> When it is clicked, the query page is redisplayed as it was
> originally, showing only custom fields in the global scope (if any).

Why is this needed? They can always just ignore them.

> It is an error for the user to select one or more target projects,
> show the custom fields for those components, change the target
> projects, and begin a search.  In this case Bugzilla will redisplay
> the query page with an appropriate error message at the top.

It should not be an error unless the user has set search values for 
custom fields which apply to none of the products concerned - and even 
then, the message is more informational than error.

> Different projects may define different custom fields with the same
> name.

No; this is an error. That's fine, because the display name is separated 
from the internal name. They could define the same display name for two 
fields, but any confusion caused is their fault :-)

Hmm... there's a bit of info missing in my mind. Is it possible to have 
a custom field which is part of projects A, B and C but not D or E?

> Custom fields in the
> global scope (if any) are grouped together under the italicized label
> "Global fields"

No need for that; they should be treated like any other hard-coded 
field, and not singled out for special attention.

> (The open versions of the latter two inequalities--ie, strictly
> earlier than or strictly later than--are probably not worth including.)

Agreed.

> Administration
> ==============
> 
> A new CGI program for managing custom fields, cdadmin.cgi, will be
> created.  The program is only accessible to the Bugzilla
> administrator.

editcustomfields.cgi would be a name fitting with the rest of our admin 
CGIs.

A critique of the editing UI will follow.

> Editing a custom field
> ----------------------
> 
> The only aspect of a string, integer, and date field that may be
> edited (other than its name, handled by a separate Rename operation)
> is its default value.

And which products it belongs to?

> For a new String field, the first page presented includes only a pair
> of radiobuttons, labelled "Long string (multiline, up to 64K
> characters)" 

Remind me again why we need this long string type?

> Moving a field to a different position
> --------------------------------------
> 
> This action is performed immediately.  The field ordering for the
> selected project (or for the global scope) is rearranged so that the
> fields appear in the new order specified by the administrator.  The
> initial cfadmin.cgi page is redisplayed, showing the fields in their
> new order.

What does this ordering affect? If it's display ordering, this is the 
wrong place for it - such configuration belongs in templates.

> [Open question: Since this is such a potentially damaging operation,
> maybe it should not be available via the web interface at all.
> Perhaps the administrator should be required to run, from the command
> line, a special program provided for this purpose.  Or perhaps deleted
> field data should be retained for a time to make an undo operation
> possible.]

Normally, we allow people to shoot themselves in the foot if they like. 
:-) But perhaps you are right; this might need to be a command-line 
operation.

> ------------------------------------------------------------------------
> 
> The following three tables provide definitions for all custom fields
> in the Bugzilla installation.
> 
> custom_fields
> -------------
> project tinytext primary key,
> field_id integer not null,
> field_index integer not null
> 
> This table defines which fields belong to which projects, and the
> order in which those fields should be presented to users.  The global
> scope is described by the project "" (ie, the zero-length string).

You should be using product_ids here, not "project" strings. And again, 
the field_index function is something that should be decided in templates.

> custom_field_defs
> -----------------
> field_id integer primary key,
> field_name tinytext not null,
> field_type char not null,
> extended bool not null,
> selection_id integer
> 
> The field_type column is 'I' for integer fields, 'C' for string
> fields, 'D' for date fields, and 'S' for selection fields.
> 
> The extended column is always false for Integer fields.  It has the
> following meanings for other fields:
> 
> Field type | extended is "true"  | extended is "false"
> -----------+---------------------+------------------------
> String     | long string (text)  | short string (tinytext)
> Date       | date/time           | date only
> Selection  | multiselection      | single-selection

This looks like a hack to me. Even if you expose it in the UI as types 
and subtypes, it seems that in the database, it makes sense to have a 
single field_type field with seven values.

> cf_selection_defs
> -----------------
> selection_id integer primary key,
> unset tinytext not null
> 
> The unset column is the text which represents the "empty set" value of
> single-selection fields.

Again, this should be decided in UI templates.

> [Open question:  Should this table be split into two:
> cf_selection_single and cf_selection_multi?]

One seems fine.

> The following six tables hold bug activity data.

What's wrong with the current activity system?

Gerv