I just checked Mantis 2.1.0 and it happens there as well.

I just checked Mantis 2.1.0 and it happens there as well.

Right.
The same applies to check box fields

The problem is caused by the function that get executed when storing/restoring the values to/from database
'#function_value_to_database' => 'cfdef_prepare_list_value_to_database', '#function_database_to_value' => 'cfdef_prepare_list_database_to_value',

No time to have a deeper look at the moment.

The root cause is that bug_update.php does not compare old vs new value when building the list of custom fields to update [1].

The problem can be fixed there or in custom field API [2], by adding a comparison between old and new values; the latter is less efficient (we can avoid calling custom_field_set_value() completely, since we already know the values), but would ensure consistent behavior from SOAP API, so maybe we should do both.

[1] https://github.com/mantisbt/mantisbt/blob/release-2.1.0/bug_update.php#L307-L308
[2] https://github.com/mantisbt/mantisbt/blob/release-2.1.0/core/custom_field_api.php#L1308

PR https://github.com/mantisbt/mantisbt/pull/1035

@dregad, not sure if you are aware that it works for other custom field types because there is a comparison of old and new value in function history_log_event_direct [1]

The comparison fails in this case as we store the values by adding | at the begin and the end)

function cfdef_prepare_list_value_to_database( $p_value ) {
    if( '' == $p_value ) {
        return '';
    } else {
        return '|' . $p_value . '|';
    }
}

but compare after removing the |

function cfdef_prepare_list_database_to_value( $p_value ) {
    return rtrim( ltrim( $p_value, '|' ), '|' );
}

[1] https://github.com/mantisbt/mantisbt/blob/release-2.1.0/core/history_api.php#L78

EDIT (dregad) use Markdown `` instead of for proper formatting of the code

@atrol I did not realize that, thanks for pointing me to it. I'll have a look at the logic there.

That being said, I think it makes sense to merge the proposed change anyway, as there is no point in executing an UPDATE query when it's not needed.

I think the logic in the call to history_log_event_direct() in custom_field_set_value() is wrong.

1. $t_value contains the new field value, in DB format
2. old field value is converted to display value format

So history_log_event_direct() is effectively comparing apples with oranges.

@atrol your feedback on 0022408:0055747 would be appreciated. Do you agree with my analysis ?

I don't have time to have a deeper look and for tests.
So you think that call of history_log_event_direct has to be changed in function custom_field_set_value to

        history_log_event_direct( $p_bug_id, $t_name, $t_row[$t_value_field], $t_value );

I didn't test either, but yes this is what I think

@vboctor, what's your take on 0022408:0055747 ?

Hi,
This fix is OK for me further to testing in version 2.0.0.
Thanks a lot !

Hi,

i think the correct fix is in the function custom_field_set_value :

history_log_event_direct( $c_bug_id, $t_name, custom_field_database_to_value( $row['value'], $t_type ), $p_value );

So was this line in Mantis 1.2.10. So instead of $t_value it has to be $p_value, which is the new not formatted value.

@dregad seems reasonable since it seems to fix the issue :)

Hi! Any update on this issue? from the above history it looks like the PR is there but we're still seeing this issue so are there plans to roll it out?

@sandyj there is a good reason that the PR is not merged, see my comment

In file custom_field_api.php, line 1352:

history_log_event_direct( $p_bug_id, $t_name, custom_field_database_to_value( $t_row[$t_value_field], $t_type ), $t_value );

can we call a db_affected_rows before it to see if a history is needed to be inserted?

created a pull request: https://github.com/mantisbt/mantisbt/pull/1514

still happening in 2.23.0

and still happening in 2.25.2
@dregad will there be a fix for this 5 year old bug?

PR: https://github.com/mantisbt/mantisbt/pull/1930