View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0014558 | mantisbt | api soap | public | 2012-08-07 02:31 | 2013-04-06 08:15 |
Reporter | schreibm | Assigned To | rombert | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Product Version | 1.2.5 | ||||
Target Version | 1.2.12 | Fixed in Version | 1.2.12 | ||
Summary | 0014558: Monitors get lost when calling mc_issue_update via SOAP | ||||
Description | When I want to update a field of an issue via SOAP, the monitors of that issue get lost. The function "mc_issue_update" is calling "mci_issue_set_monitors" (line 829). The "bug_monitor" function is checking if the user that should be added as monitor is already monitoring the bug and does not add it in that case. IMHO the issue here is, that first all monitors from the SOAP call are added (in fact they are not added if they already are monitoring the issue) and after that all existing monitors are removed. I guess it should be the other way around: First remove all existing monitors and than add all that are passed with the SOAP Body. Find attached a patch that should fix the issue. | ||||
Tags | No tags attached. | ||||
Attached Files | monitors.patch (1,654 bytes)
Index: api/soap/mc_issue_api.php =================================================================== --- api/soap/mc_issue_api.php (revision 173162) +++ api/soap/mc_issue_api.php (working copy) @@ -342,7 +342,23 @@ $t_monitors = array(); foreach ( $p_monitors as $t_monitor ) $t_monitors[] = $t_monitor['id']; - + + foreach ( $t_existing_monitors as $t_user_id ) { + + if ( $p_user_id == $t_user_id ) { + if ( ! access_has_bug_level( config_get( 'monitor_bug_threshold' ), $p_issue_id ) ) + continue; + } else { + if ( !access_has_bug_level( config_get( 'monitor_delete_others_bug_threshold' ), $p_issue_id ) ) + continue; + } + + if ( in_array( $p_user_id, $t_monitors) ) + continue; + + bug_unmonitor( $p_issue_id, $t_user_id); + } + foreach ( $t_monitors as $t_user_id ) { if ( $p_user_id == $t_user_id ) { @@ -359,21 +375,7 @@ bug_monitor( $p_issue_id, $t_user_id); } - foreach ( $t_existing_monitors as $t_user_id ) { - if ( $p_user_id == $t_user_id ) { - if ( ! access_has_bug_level( config_get( 'monitor_bug_threshold' ), $p_issue_id ) ) - continue; - } else { - if ( !access_has_bug_level( config_get( 'monitor_delete_others_bug_threshold' ), $p_issue_id ) ) - continue; - } - - if ( in_array( $p_user_id, $t_monitors) ) - continue; - - bug_unmonitor( $p_issue_id, $t_user_id); - } } 14558.patch (1,793 bytes)
diff --git a/api/soap/mc_issue_api.php b/api/soap/mc_issue_api.php index 57aa6ca..c40a0bb 100644 --- a/api/soap/mc_issue_api.php +++ b/api/soap/mc_issue_api.php @@ -352,38 +352,42 @@ function mci_issue_set_monitors( $p_issue_id , $p_user_id, $p_monitors ) { foreach ( $p_monitors as $t_monitor ) $t_monitors[] = $t_monitor['id']; - foreach ( $t_monitors as $t_user_id ) { + // unmonitor existing monitors + foreach ( $t_existing_monitors as $t_user_id ) { if ( $p_user_id == $t_user_id ) { if ( ! access_has_bug_level( config_get( 'monitor_bug_threshold' ), $p_issue_id ) ) continue; } else { - if ( !access_has_bug_level( config_get( 'monitor_add_others_bug_threshold' ), $p_issue_id ) ) + if ( !access_has_bug_level( config_get( 'monitor_delete_others_bug_threshold' ), $p_issue_id ) ) continue; } - if ( in_array( $p_user_id, $t_existing_monitors) ) + if ( in_array( $p_user_id, $t_monitors) ) continue; - bug_monitor( $p_issue_id, $t_user_id); + bug_unmonitor( $p_issue_id, $t_user_id); } - - foreach ( $t_existing_monitors as $t_user_id ) { + + // monitor monitors based on the passed in user ids + foreach ( $t_monitors as $t_user_id ) { if ( $p_user_id == $t_user_id ) { if ( ! access_has_bug_level( config_get( 'monitor_bug_threshold' ), $p_issue_id ) ) continue; } else { - if ( !access_has_bug_level( config_get( 'monitor_delete_others_bug_threshold' ), $p_issue_id ) ) + if ( !access_has_bug_level( config_get( 'monitor_add_others_bug_threshold' ), $p_issue_id ) ) continue; } - if ( in_array( $p_user_id, $t_monitors) ) + if ( in_array( $p_user_id, $t_existing_monitors) ) continue; - bug_unmonitor( $p_issue_id, $t_user_id); + bug_monitor( $p_issue_id, $t_user_id); } + + } /** | ||||
Thanks for raising this issue. I had hit the problem earlier but I was unable to reproduce. I'll look into your patch in detail. In the meantime you can consider supplying a git-formatted patch, for proper attribution. |
|
I am not a git expert, hope 14558.patch is well formatted |
|
Thanks for the investigation and patch, I appreciate your digging into the code. I've done some digging as well and it seems that the error is that I was checking the wrong user id for monitoring/unmonitoring. I've attached an updated patch which applies on top of master-1.2.x which works for me (tm) and which also uses clearer variable names. I'd appreciate it if you could verify this before I will commit it. Fix-14558-Monitors-get-lost-when-calling-mcissueupda.patch (2,525 bytes)
From 9258fc19fa903987584cad2e76800db79e93252c Thu, 9 Aug 2012 23:19:23 +0300 From: Robert Munteanu <robert@lmn.ro> Date: Thu, 9 Aug 2012 23:19:07 +0300 Subject: [PATCH] Fix #14558: Monitors get lost when calling mc_issue_update via SOAP diff --git a/api/soap/mc_issue_api.php b/api/soap/mc_issue_api.php index 51d8668..a575b43 100644 --- a/api/soap/mc_issue_api.php +++ b/api/soap/mc_issue_api.php @@ -341,20 +341,23 @@ * @param array $p_monitors An array of arrays with the <em>id</em> field set to the id * of the users which should monitor this issue. */ -function mci_issue_set_monitors( $p_issue_id , $p_user_id, $p_monitors ) { +function mci_issue_set_monitors( $p_issue_id , $p_requesting_user_id, $p_monitors ) { if ( bug_is_readonly( $p_issue_id ) ) { - return mci_soap_fault_access_denied( $p_user_id, "Issue '$p_issue_id' is readonly" ); + return mci_soap_fault_access_denied( $p_requesting_user_id, "Issue '$p_issue_id' is readonly" ); } - $t_existing_monitors = bug_get_monitors( $p_issue_id ); + // 1. get existing monitor ids + $t_existing_monitor_ids = bug_get_monitors( $p_issue_id ); - $t_monitors = array(); + // 2. build new monitors ids + $t_new_monitor_ids = array(); foreach ( $p_monitors as $t_monitor ) - $t_monitors[] = $t_monitor['id']; + $t_new_monitor_ids[] = $t_monitor['id']; - foreach ( $t_monitors as $t_user_id ) { + // 3. for each of the new monitor ids, add it if it does not already exist + foreach ( $t_new_monitor_ids as $t_user_id ) { - if ( $p_user_id == $t_user_id ) { + if ( $p_requesting_user_id == $t_user_id ) { if ( ! access_has_bug_level( config_get( 'monitor_bug_threshold' ), $p_issue_id ) ) continue; } else { @@ -362,15 +365,16 @@ continue; } - if ( in_array( $p_user_id, $t_existing_monitors) ) + if ( in_array( $t_user_id, $t_existing_monitor_ids) ) continue; bug_monitor( $p_issue_id, $t_user_id); } - foreach ( $t_existing_monitors as $t_user_id ) { + // 4. for each of the existing monitor ids, remove it if it is not found in the new monitor ids + foreach ( $t_existing_monitor_ids as $t_user_id ) { - if ( $p_user_id == $t_user_id ) { + if ( $p_requesting_user_id == $t_user_id ) { if ( ! access_has_bug_level( config_get( 'monitor_bug_threshold' ), $p_issue_id ) ) continue; } else { @@ -378,7 +382,7 @@ continue; } - if ( in_array( $p_user_id, $t_monitors) ) + if ( in_array( $t_user_id, $t_new_monitor_ids) ) continue; bug_unmonitor( $p_issue_id, $t_user_id); |
|
I verified your patch. You can commit it, it works fine! |
|
Thanks for the confirmation. |
|
Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch |
|
MantisBT: master-1.2.x 05f7c33c 2012-08-10 10:41 Details Diff |
Fix 0014558: Monitors get lost when calling mc_issue_update via SOAP |
Affected Issues 0014558 |
|
mod - api/soap/mc_issue_api.php | Diff File | ||
mod - tests/soap/IssueUpdateTest.php | Diff File | ||
MantisBT: master 9a8d41dd 2012-08-10 10:41 Details Diff |
Fix 0014558: Monitors get lost when calling mc_issue_update via SOAP |
Affected Issues 0014558 |
|
mod - api/soap/mc_issue_api.php | Diff File | ||
mod - tests/soap/IssueUpdateTest.php | Diff File |