Further update, we are getting the same "Invalid value" error on issue edits where that field currently has no value. I'm going to SQL a fix for that (replace "" with "0000"), but this elevates it in my book to an honest problem, and not just a nuisance.

What RDBMS are you using ?

All pertinent data (and apologies for not including it originally Damien):-

Host OS: Ubuntu 10.04 LTS
Apache: 2.2.14
MySQL: 5.1.63 (Percona MySQL build 14.14)

I can reproduce this behavior on my dev box using the custom field definition you provided in the attached screenshot.

The commit which introduced the problem is 9c71d4f8, released in 1.2.9: fix for 0012029, modifying the custom field validation function

I believe the root cause is that the validation should not fail when the field is empty.

Please test the attached patch and let me know how it goes

Applied your patch, changed the custom field definition back to no default value and don't display when reporting, then added a new case to the project the field applies to. Issue submitted without validation error.

Also, edited an existing issue removing the previous "0000" dummy value from the custom field. Again, issue submitted without validation error.

Many thanks for (a) finding and (b) fixing this one Damien; I consider this issue resolved.

(In reply to comment 0014516:0032399)

Can you kindly have a look at this [1] as well ?

I am specifically interested in your feedback because of the call to
custom_field_validate() in api/soap/mc_issue_api.php,
mci_issue_set_custom_fields() function, which does not seem to contain any logic
related to checking for required custom fields (unless this is done elsewhere in
the soap api).

[1] https://github.com/dregad/mantisbt/tree/fix-14516

Thanks for the heads-up, Damien.

Doesn't the call to custom_field_validate on line 173 cover the validation requirements.

Doesn't the call to custom_field_validate on line 173 cover the validation requirements.

Only partially, in that the function does not check the 'required' status of a custom field, only that its contents are valid.

If you look at the code, the error checking for required field is tested separately, e.g. in bug_report.php line 119

foreach( $t_related_custom_field_ids as $t_id ) {

$t_def = custom_field_get_definition( $t_id );

    # Produce an error if the field is required but wasn't posted
    if ( !gpc_isset_custom_field( $t_id, $t_def['type'] ) &&
        ( $t_def['require_report'] ) ) {
        error_parameters( lang_get_defaulted( custom_field_get_field( $t_id, 'name' ) ) );
        trigger_error( ERROR_EMPTY_FIELD, ERROR );
    }

    if ( !custom_field_validate( $t_id, gpc_get_custom_field( "custom_field_$t_id", $t_def['type'], NULL ) ) ) {
        error_parameters( lang_get_defaulted( custom_field_get_field( $t_id, 'name' ) ) );
        trigger_error( ERROR_CUSTOM_FIELD_INVALID_VALUE, ERROR );
    }
}

Based on PantsManUK's feedback in 0014516:0032401 I will push my fix to core.

@rombert if you think, as I do, that SOAP should be amended to check for required custom field, I let you create a separate issue to track it.

Thanks for testing, I'll have a look

@atrol That would seem (to me, at least) to be desired behaviour; right up to the point that it becomes a requirement for the field to have a value, it can be blank.

A direct example, using our own data (since it's displayed above): the "Tested in revision" field clearly has a minimum and maximum length requirement, but it can be blank until it's required (when the issue is marked Resolved or Closed in our case), at which point the minimum/maximum requirements need to be enforced.

@dregad Unfortunately, in my (admittedly limited) testing, despite "Tested in revision" being "Display" and "Required" for Resolve, it neither displayed nor warned that it was empty when I resolved a test issue. It did appear when I Closed the case (and failed validation when left blank).

The custom field configuration would need some redesign to handle all use cases in a consistent way. Probably nothing to implement in 1.2.x.

@PantsManUK, you are right, I am able to enforce the desired behaviour by checking "Required On Update".

(In reply to comment 0014516:0032408)

Doesn't the call to custom_field_validate on line 173 cover the validation
requirements.

Only partially, in that the function does not check the 'required' status of a
custom field, only that its contents are valid.

If you look at the code, the error checking for required field is tested
separately, e.g. in bug_report.php line 119

foreach( $t_related_custom_field_ids as $t_id ) {

$t_def = custom_field_get_definition( $t_id );

Produce an error if the field is required but wasn't posted
if ( !gpc_isset_custom_field( $t_id, $t_def['type'] ) &&

( $t_def['require_report'] ) ) {

error_parameters( lang_get_defaulted( custom_field_get_field( $t_id, 'name' )

) );

trigger_error( ERROR_EMPTY_FIELD, ERROR );

}
if ( !custom_field_validate( $t_id, gpc_get_custom_field(

"customfield$t_id", $t_def['type'], NULL ) ) ) {

error_parameters( lang_get_defaulted( custom_field_get_field( $t_id, 'name' )

) );

trigger_error( ERROR_CUSTOM_FIELD_INVALID_VALUE, ERROR );

}

}

I guess it's done this way in order to return a differentiated error message to
the user.

I've added bug 0014533 to track this for the SOAP API.

@PantsManUK re 0014516:0032420:

despite "Tested in revision" being "Display" and "Required" for Resolve, it
neither displayed nor warned that it was empty when I resolved a test issue. It
did appear when I Closed the case (and failed validation when left blank).

I am not able to reproduce this here (i.e. custom field is both displayed and required when resolving issue), and I wonder whether your problem is caused by your using a non-standard mantis, as you mention in the bug's description

with some additional local changes to the code for our workflow.

Please try again with a standard MantisBT codebase and let me know if problem persists, if possible with detailed steps to reproduce.

@PantsManUK - as you have not provided any feedback on my earlier note, I am resolving this again.

Feel free to reopen if you can provide steps to reproduce the issue.

Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch