Re: Patch: Required fields on inital PR creation

[Mel Hatzis]

On 10/27/02 11:36, Yngve Svendsen wrote:
> At 16:42 15.10.2002 -0700, Mel Hatzis wrote:
>
> > Please review (and hopefully accept) the following patch that fixes a bug
> > in the 'on-change require' functionality and also expands the Gnats 
> > grammar
> > to allow for "required input fields" upon initial PR creation.
> >
> > 1/ The "require { fields }" functionality that's part of the on-change 
> > edit
> >    options for a field is only doing a check for field_value == NULL 
> > right now
> >    (rather than also checking for empty strings). Since field values are
> >    generally initialized to an empty string (upon creation), the 
> > 'require'
> >    clause is basically useless. The patch adds a check for field_value 
> > == "".
> >
> > 2/ It is extremely useful to allow the Gnats administrator to configure
> >    Gnats so that a predefined set of fields must be assigned a value 
> > upon initial
> >    PR creation. Together with the field's own 'on-change require' 
> > clause, this
> >    allows for fields that can never have a null (blank) value.
> >    Allowing the Gnats administrator to enforce non-null fields this 
> > way provides
> >    a powerful mechanism toward improved data integrity.
>
>
> This doesn't work quite as it should with multitext fields. If a field 
> is in the initial-entry fields list, both send-pr and Gnatsweb will 
> include it when they submit PRs, even if it is empty. In the submitted 
> PR text, these fields will contain newline character but nothing else. 
> This slips past the require functionality that this patch implements.

Good catch...your patch looks entirely reasonable.

It'd be nice if the Gnats code didn't add newlines for otherwise
blank values - while this may make the display_pr functionality
a little easier, I think it's not very intuitive. Perhaps something
we can improve on for 4.1.

-Mel

> I suggest we also check for space characters, as defined by isspace(), 
> by making the following trivial change:
>
> Index: edit.c
> ===================================================================
> RCS file: /cvsroot/gnats/gnats/gnats/edit.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 edit.c
> --- edit.c      24 Oct 2002 12:45:09 -0000      1.58
> +++ edit.c      27 Oct 2002 19:29:43 -0000
> @@ -1105,7 +1105,7 @@ applyChangeAction (ChangeActions action,
>    while (fields != NULL)
>      {
>        const char *fldval = get_field_value (pr, oldPR, fields->ent, 
> NULL, NULL);
> -      if (fldval == NULL || *fldval == '\0')
> +      if (value_is_empty (fldval))
>         {
>           setError (err, CODE_INVALID_PR_CONTENTS,
>                     "Required field %s missing from PR %s.",
> Index: file-pr.c
> ===================================================================
> RCS file: /cvsroot/gnats/gnats/gnats/file-pr.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 file-pr.c
> --- file-pr.c   24 Oct 2002 12:45:09 -0000      1.48
> +++ file-pr.c   27 Oct 2002 19:29:43 -0000
> @@ -446,7 +446,7 @@ missing_required_fields (const DatabaseI
>    while (fields != NULL)
>      {
>        const char *fldval = get_field_value (pr, NULL, fields->ent, 
> NULL, NULL);
> -      if (fldval == NULL || *fldval == '\0')
> +      if (value_is_empty (fldval))
>         {
>            setError (err, CODE_INVALID_PR_CONTENTS,
>                      "Required field %s missing from new PR - cannot 
> submit.",
>
>
>
>
>
> - Yngve