1001846 - When editing cc_accessible using Bug.update, the method sometimes returns is_cc_accessible and sometimes cc_accessible as being changed

Status: RESOLVED FIXED

(Bugzilla :: WebService, defect)

Description

[Frédéric Buclin]

Run this script several times. Before each call, make sure cc_accessible = 1:

    $call = rpc_call('Bug.update', {ids => [$bug_id], is_cc_accessible => 0});
    say Dumper($call->result->{bugs}->[0]->{changes});

Sometimes, I get:

$VAR1 = {
          'cc_accessible' => {
                             'removed' => '1',
                             'added' => '0'
                           }
        };

and sometimes:

$VAR1 = {
          'is_cc_accessible' => {
                                'added' => '0',
                                'removed' => '1'
                              }
        };

Note that the field name is not the same: cc_accessible vs is_cc_accessible. This is inconsistent and makes QA tests to fail randomly, because the name of the field varies from run to run. This happens with Perl 5.18.1. I suspect this is due to the new randomization of hashes in Perl 5.18, see bug 997281 for details.

Comment 1

[Frédéric Buclin]

The problem is here, in WebService/Bug.pm:

  my %api_name = reverse %{ Bugzilla::Bug::FIELD_MAP() };


It uses reverse, but FIELD_MAP contains:

use constant FIELD_MAP => {
    cc_accessible    => 'cclist_accessible',
    is_cc_accessible => 'cclist_accessible',
    ....
};

So cc_accessible and is_cc_accessible will overwrite each other, randomly. Per bug 578513, we should return is_cc_accessible, always.

Comment 2

[Frédéric Buclin]

Attachment: patch, v1

When mkanat added is_cc_accessible into FIELD_MAP, he forgot to remove the obsolete cc_accessible.

Comment 3

[David Lawrence [:dkl]]

Comment on attachment 8413195 [details] [diff] [review]
patch, v1

Review of attachment 8413195 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

Comment 4

[Frédéric Buclin]

To ssh://[e-mail]/bugzilla/bugzilla.git
   4ad3e58..7505025  master -> master

To ssh://[e-mail]/bugzilla/bugzilla.git
   dc47bf8..14208d5  4.4 -> 4.4

Comment 5

[Dave Miller [:justdave]]

Approved for 4.2, failing tests is bad on a release branch.

Comment 6

[Frédéric Buclin]

To ssh://[e-mail]/bugzilla/bugzilla.git
   feca0d8..9d068ba  4.2 -> 4.2