View Issue Details

IDProjectCategoryView StatusLast Update
0022408mantisbtcustom fieldspublic2017-12-04 02:25
ReporterjensberkeAssigned Todregad 
PrioritynormalSeverityminorReproducibilityalways
Status assignedResolutionopen 
Product Version1.3.5 
Target Version2.10.0Fixed in Version 
Summary0022408: Custom field's value logged as changed in history, when it wasn't changed
Description

If a custom field of type "multiselection list" in an issue already has a value and you change another value in the ticket, the custom field's value also gets logged in the issue history though it hasn't changed.

Steps To Reproduce
  1. Assume a custom field of type "multiseletion list" with "possible values" set to "not relevant|relevant", and that this field is assigned to a project.
  2. Create an issue and set this custom field to "not relevant".
  3. Change the issue by editing any other field value but leave the custom field unchanged.
  4. Though the custom field hasn't changed, the issue history shows an entry for it with the custom field's name in column "field" and the value as "not relevant => not relevant" in column "change"
TagsNo tags attached.

Relationships

has duplicate 0022421 closedatrol On Issue Update, history always lists the multiselection custom fields though they have not been updated 
related to 0022464 assigneddregad Loose type comparison can prevent custom field update 

Activities

jensberke

jensberke

2017-02-22 07:35

reporter   ~0055738

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

atrol

atrol

2017-02-22 08:14

developer   ~0055741

Last edited: 2017-02-22 08:14

View 3 revisions

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.

dregad

dregad

2017-02-22 10:50

developer   ~0055742

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

dregad

dregad

2017-02-22 11:05

developer   ~0055743

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

atrol

atrol

2017-02-22 11:21

developer   ~0055744

Last edited: 2017-02-22 11:49

View 3 revisions

@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

dregad

dregad

2017-02-22 11:38

developer   ~0055746

@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.

dregad

dregad

2017-02-22 11:43

developer   ~0055747

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.

dregad

dregad

2017-02-23 05:48

developer   ~0055761

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

atrol

atrol

2017-02-23 06:13

developer   ~0055763

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 );
dregad

dregad

2017-02-23 06:18

developer   ~0055764

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

dregad

dregad

2017-02-24 09:25

developer   ~0055776

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

mantisiator

mantisiator

2017-02-26 11:30

reporter   ~0055785

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

MarcoW

MarcoW

2017-03-02 06:55

reporter   ~0055863

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.

vboctor

vboctor

2017-04-01 17:22

manager   ~0056315

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

Issue History

Date Modified Username Field Change
2017-02-22 06:38 jensberke New Issue
2017-02-22 07:23 atrol Status new => confirmed
2017-02-22 07:35 jensberke Note Added: 0055738
2017-02-22 08:14 atrol Note Added: 0055741
2017-02-22 08:14 atrol Note Edited: 0055741 View Revisions
2017-02-22 08:14 atrol Note Edited: 0055741 View Revisions
2017-02-22 10:50 dregad Note Added: 0055742
2017-02-22 11:00 dregad Summary A "multiseletion list" custom field's value gets logged in the issue history as changed when it wasn't changed => Custom field's value logged as changed in history, when it wasn't changed
2017-02-22 11:00 dregad Description Updated View Revisions
2017-02-22 11:05 dregad Assigned To => dregad
2017-02-22 11:05 dregad Status confirmed => assigned
2017-02-22 11:05 dregad Note Added: 0055743
2017-02-22 11:05 dregad Target Version => 2.2.0
2017-02-22 11:21 atrol Note Added: 0055744
2017-02-22 11:22 atrol Note Edited: 0055744 View Revisions
2017-02-22 11:38 dregad Note Added: 0055746
2017-02-22 11:43 dregad Note Added: 0055747
2017-02-22 11:49 dregad Note Edited: 0055744 View Revisions
2017-02-23 05:48 dregad Note Added: 0055761
2017-02-23 06:13 atrol Note Added: 0055763
2017-02-23 06:18 dregad Note Added: 0055764
2017-02-24 09:25 dregad Note Added: 0055776
2017-02-26 04:47 atrol Relationship added has duplicate 0022421
2017-02-26 11:30 mantisiator Note Added: 0055785
2017-02-26 21:19 vboctor Target Version 2.2.0 => 2.3.0
2017-03-02 06:55 MarcoW Note Added: 0055863
2017-03-04 05:00 dregad Relationship added related to 0022464
2017-04-01 00:20 vboctor Target Version 2.3.0 => 2.4.0
2017-04-01 17:22 vboctor Note Added: 0056315
2017-04-30 14:53 vboctoradmin Target Version 2.4.0 => 2.5.0
2017-06-04 16:19 atrol Target Version 2.5.0 => 2.6.0
2017-09-03 18:49 vboctor Target Version 2.6.0 => 2.7.0
2017-10-08 23:55 vboctor Target Version 2.7.0 => 2.8.0
2017-10-28 19:14 vboctor Target Version 2.8.0 => 2.9.0
2017-12-04 02:25 vboctor Target Version 2.9.0 => 2.10.0