email_enabled value inverted in User.update RPC call

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
WebService
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Jeff Fearn, Assigned: Jeff Fearn)

Tracking

4.4.8
Bugzilla 4.4
Bug Flags:
approval +
approval5.0 +
blocking5.0 +
approval4.4 +

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

2 years ago
Created attachment 8602437 [details] [diff] [review]
patch to remove mapping and invert value in DB

email_enabled should be the inverse of disable_mail, however mail_enabled is mapped directly to disable_mail and thus you have to invert the boolean value to get the expected behaviour.

e.g.

User.update({'email_enabled': 1}) should enable email, but it disables it.
User.update({'email_enabled': 0}) should disable email, but it enables it.

Attached is a patch to remove the mapping and add a sub to invert the value supplied in the DB.
(Assignee)

Updated

2 years ago
Attachment #8602437 - Flags: review?(glob)
confirming.
Assignee: webservice → Jeff.Fearn
Flags: blocking5.0+
Target Milestone: --- → Bugzilla 4.4
Comment on attachment 8602437 [details] [diff] [review]
patch to remove mapping and invert value in DB

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

the changes look good, but don't pass tests:

#   Failed test 'Bugzilla/User.pm POD coverage is 99%. Undocumented methods: set_email_enabled'
#   at t/011pod.t line 108.

you'll also need to update Bugzilla/API/1_0/Resource/User.pm (trunk only)

::: Bugzilla/User.pm
@@ +269,1 @@
>  sub set_extern_id    { $_[0]->set('extern_id', $_[1]); }

realign the {s please
Attachment #8602437 - Flags: review?(glob) → review-
(Assignee)

Updated

2 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 3

2 years ago
Created attachment 8604413 [details] [diff] [review]
fix issues noted in patch 1

Wasn't sure where to add pod so I just added it to teh bottom of the (hopefully) relevant section.
Attachment #8602437 - Attachment is obsolete: true
Attachment #8604413 - Flags: review?(glob)
Comment on attachment 8604413 [details] [diff] [review]
fix issues noted in patch 1

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

> Wasn't sure where to add pod so I just added it to teh bottom of the (hopefully) relevant section.
the location looks good to me.

please ensure you run tests prior to attaching a patch:

t/011pod.t ........... 76/377
#   Failed test 'Bugzilla/User.pm has incorrect POD syntax --ERROR'
#   at t/011pod.t line 80.

pointing perldoc at the file shows the issue (at the end of the output):

POD ERRORS
    Hey! The above document had some coding errors, which are explained
    below:

    Around line 3104:
        '=item' outside of any '=over'
Attachment #8604413 - Flags: review?(glob) → review-
(Assignee)

Comment 5

2 years ago
Created attachment 8604478 [details] [diff] [review]
Fix POD error
Attachment #8604413 - Attachment is obsolete: true
Attachment #8604478 - Flags: review?(glob)
Comment on attachment 8604478 [details] [diff] [review]
Fix POD error

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

r=glob
Attachment #8604478 - Flags: review?(glob) → review+
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   bd4bfe3..3ff9b3e  master -> master

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   e191f86..3d2b724  5.0 -> 5.0

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   3a21f12..42ae6d0  4.4 -> 4.4
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Flags: approval5.0+
Flags: approval4.4+
Flags: approval+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.