From: <gi...@ma...> - 2010-02-25 04:05:38
|
The branch, master has been updated via 2191aa1bf91c9483d936efef31b5790516c29b69 (commit) from c3dedbbe950b3a4d1fef432d49e96ef4fbc39409 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- commit 2191aa1bf91c9483d936efef31b5790516c29b69 Author: David Hicks <hic...@op...> Date: Fri Feb 26 01:55:01 2010 +1100 Fix #11552: No errors shown when actiongroup tag attaching fails When attacking tags to multiple issues via the action group feature (the dropdown in the bottom left of the view all issues page), errors are silently ignored. The user should instead be informed if any of the tags could not be applied to the issues selected. This patch also changes the return status from the actiongroup validation and processing functions to be an error string or null in the event that an error didn't occur. In the future it'd be much better if the validation and processing functions worked on the entire set of selected issues instead of each issue one-by-one. This gives the actiongroup validation/processing functions more flexibility (especially in the case of the tag attaching action). ----------------------------------------------------------------------- Summary of changes: bug_actiongroup_add_note_inc.php | 15 ++--- bug_actiongroup_attach_tags_inc.php | 74 +++++++++++--------------- bug_actiongroup_ext.php | 18 +++---- bug_actiongroup_update_product_build_inc.php | 16 ++---- bug_actiongroup_update_severity_inc.php | 18 ++---- lang/strings_english.txt | 1 + 6 files changed, 59 insertions(+), 83 deletions(-) ----------------------------------------------------------------------- commit 2191aa1bf91c9483d936efef31b5790516c29b69 Author: David Hicks <hic...@op...> Date: Fri Feb 26 01:55:01 2010 +1100 Fix #11552: No errors shown when actiongroup tag attaching fails When attacking tags to multiple issues via the action group feature (the dropdown in the bottom left of the view all issues page), errors are silently ignored. The user should instead be informed if any of the tags could not be applied to the issues selected. This patch also changes the return status from the actiongroup validation and processing functions to be an error string or null in the event that an error didn't occur. In the future it'd be much better if the validation and processing functions worked on the entire set of selected issues instead of each issue one-by-one. This gives the actiongroup validation/processing functions more flexibility (especially in the case of the tag attaching action). diff --git a/bug_actiongroup_add_note_inc.php b/bug_actiongroup_add_note_inc.php index 4520f4c..b625a0f 100644 --- a/bug_actiongroup_add_note_inc.php +++ b/bug_actiongroup_add_note_inc.php @@ -93,7 +93,7 @@ function action_add_note_print_fields() { /** * Validates the action on the specified bug id. * - * @returns true|array Action can be applied., ( bug_id => reason for failure ) + * @return string|null On failure: the reason why the action could not be validated. On success: null. */ function action_add_note_validate( $p_bug_id ) { $f_bugnote_text = gpc_get_string( 'bugnote_text' ); @@ -103,32 +103,29 @@ function action_add_note_validate( $p_bug_id ) { trigger_error( ERROR_EMPTY_FIELD, ERROR ); } - $t_failed_validation_ids = array(); $t_add_bugnote_threshold = config_get( 'add_bugnote_threshold' ); $t_bug_id = $p_bug_id; if ( bug_is_readonly( $t_bug_id ) ) { - $t_failed_validation_ids[$t_bug_id] = lang_get( 'actiongroup_error_issue_is_readonly' ); - return $t_failed_validation_ids; + return lang_get( 'actiongroup_error_issue_is_readonly' ); } if ( !access_has_bug_level( $t_add_bugnote_threshold, $t_bug_id ) ) { - $t_failed_validation_ids[$t_bug_id] = lang_get( 'access_denied' ); - return $t_failed_validation_ids; + return lang_get( 'access_denied' ); } - return true; + return null; } /** * Executes the custom action on the specified bug id. * * @param $p_bug_id The bug id to execute the custom action on. - * @returns true|array Action executed successfully., ( bug_id => reason for failure ) + * @return null Previous validation ensures that this function doesn't fail. Therefore we can always return null to indicate no errors occurred. */ function action_add_note_process( $p_bug_id ) { $f_bugnote_text = gpc_get_string( 'bugnote_text' ); $f_view_state = gpc_get_int( 'view_state' ); bugnote_add ( $p_bug_id, $f_bugnote_text, '0:00', /* $p_private = */ $f_view_state != VS_PUBLIC ); - return true; + return null; } diff --git a/bug_actiongroup_attach_tags_inc.php b/bug_actiongroup_attach_tags_inc.php index e3c473c..90b94fd 100644 --- a/bug_actiongroup_attach_tags_inc.php +++ b/bug_actiongroup_attach_tags_inc.php @@ -64,62 +64,52 @@ function action_attach_tags_print_fields() { /** * Validates the Attach Tags group action. - * Gets called for every bug, but performs the real tag validation only - * the first time. Any invalid tags will be skipped, as there is no simple - * or clean method of presenting these errors to the user. + * Checks if a user can attach the requested tags to a given bug. * @param integer Bug ID - * @return boolean True + * @return string|null On failure: the reason for tags failing validation for the given bug. On success: null. */ function action_attach_tags_validate( $p_bug_id ) { - global $g_action_attach_tags_valid; - if ( !isset( $g_action_attach_tags_valid ) ) { - $f_tag_string = gpc_get_string( 'tag_string' ); - $f_tag_select = gpc_get_string( 'tag_select' ); - - global $g_action_attach_tags_attach, $g_action_attach_tags_create, $g_action_attach_tags_failed; - $g_action_attach_tags_attach = array(); - $g_action_attach_tags_create = array(); - $g_action_attach_tags_failed = array(); - - $t_tags = tag_parse_string( $f_tag_string ); - $t_can_attach = access_has_bug_level( config_get( 'tag_attach_threshold' ), $p_bug_id ); - $t_can_create = access_has_global_level( config_get( 'tag_create_threshold' ) ); - - foreach ( $t_tags as $t_tag_row ) { - if ( -1 == $t_tag_row['id'] ) { - if ( $t_can_create && $t_can_attach ) { - $g_action_attach_tags_create[] = $t_tag_row; - } else { - $g_action_attach_tags_failed[] = $t_tag_row; - } - } else if ( -2 == $t_tag_row['id'] ) { - $g_action_attach_tags_failed[] = $t_tag_row; - } else if ( $t_can_attach ) { - $g_action_attach_tags_attach[] = $t_tag_row; - } else { - $g_action_attach_tags_failed[] = $t_tag_row; - } - } + global $g_action_attach_tags_tags; + global $g_action_attach_tags_attach; + global $g_action_attach_tags_create; + + $t_can_attach = access_has_bug_level( config_get( 'tag_attach_threshold' ), $p_bug_id ); + if( !$t_can_attach ) { + return lang_get( 'tag_attach_denied' ); + } - if ( 0 < $f_tag_select && tag_exists( $f_tag_select ) ) { - if ( $t_can_attach ) { - $g_action_attach_tags_attach[] = tag_get( $f_tag_select ); - } else { - $g_action_attach_tags_failed[] = tag_get( $f_tag_select ); + if( !isset( $g_action_attach_tags_tags ) ) { + if( !isset( $g_action_attach_tags_attach ) ) { + $g_action_attach_tags_attach = array(); + $g_action_attach_tags_create = array(); + } + $g_action_attach_tags_tags = tag_parse_string( gpc_get_string( 'tag_string' ) ); + foreach ( $g_action_attach_tags_tags as $t_tag_row ) { + if ( $t_tag_row['id'] == -1 ) { + $g_action_attach_tags_create[$t_tag_row['name']] = $t_tag_row; + } else if( $t_tag_row['id'] >= 0 ) { + $g_action_attach_tags_attach[$t_tag_row['name']] = $t_tag_row; } } + } + $t_can_create = access_has_bug_level( config_get( 'tag_create_threshold' ), $p_bug_id ); + if( count( $g_action_attach_tags_create ) > 0 && !$t_can_create ) { + return lang_get( 'tag_create_denied' ); } - global $g_action_attach_tags_attach, $g_action_attach_tags_create, $g_action_attach_tags_failed; + if( count( $g_action_attach_tags_create ) == 0 && + count( $g_action_attach_tags_attach ) == 0 ) { + return lang_get( 'tag_none_attached' ); + } - return true; + return null; } /** * Attaches all the tags to each bug in the group action. * @param integer Bug ID - * @return boolean True if all tags attach properly + * @return null Previous validation ensures that this function doesn't fail. Therefore we can always return null to indicate no errors occurred. */ function action_attach_tags_process( $p_bug_id ) { global $g_action_attach_tags_attach, $g_action_attach_tags_create; @@ -138,5 +128,5 @@ function action_attach_tags_process( $p_bug_id ) { } } - return true; + return null; } diff --git a/bug_actiongroup_ext.php b/bug_actiongroup_ext.php index 18a347e..d02f464 100644 --- a/bug_actiongroup_ext.php +++ b/bug_actiongroup_ext.php @@ -79,16 +79,14 @@ $t_failed_ids = array(); foreach( $t_projects_bugs as $t_project_id => $t_bugs ) { $g_project_override = $t_project_id; foreach( $t_bugs as $t_bug_id ) { - $t_result = bug_group_action_validate( $f_action, $t_bug_id ); - if( $t_result !== true ) { - foreach( $t_result as $t_key => $t_value ) { - $t_failed_ids[$t_key] = $t_value; - } + $t_fail_reason = bug_group_action_validate( $f_action, $t_bug_id ); + if( $t_fail_reason !== null ) { + $t_failed_ids[$t_bug_id] = $t_fail_reason; } if( !isset( $t_failed_ids[$t_bug_id] ) ) { - $t_result = bug_group_action_process( $f_action, $t_bug_id ); - if( $t_result !== true ) { - $t_failed_ids[] = $t_result; + $t_fail_reason = bug_group_action_process( $f_action, $t_bug_id ); + if( $t_fail_reason !== null ) { + $t_failed_ids[$t_bug_id] = $t_fail_reason; } } } @@ -103,9 +101,9 @@ if ( count( $t_failed_ids ) > 0 ) { echo '<div align="center">'; - $separator = lang_get( 'word_separator' ); + $t_word_separator = lang_get( 'word_separator' ); foreach( $t_failed_ids as $t_id => $t_reason ) { - $label = sprintf( lang_get( 'label' ), string_get_bug_view_link( $t_id ) ) . $sepatator; + $label = sprintf( lang_get( 'label' ), string_get_bug_view_link( $t_id ) ) . $t_word_separator; printf("<p>%s%s</p>\n", $label, $t_reason ); } diff --git a/bug_actiongroup_update_product_build_inc.php b/bug_actiongroup_update_product_build_inc.php index 6a11a13..9e02da4 100644 --- a/bug_actiongroup_update_product_build_inc.php +++ b/bug_actiongroup_update_product_build_inc.php @@ -62,36 +62,32 @@ function action_update_product_build_print_fields() { * Validates the action on the specified bug id. * * @param $p_bug_id Bug ID - * @return true|array Action can be applied., bug_id => reason for failure + * @return string|null On failure: the reason why the action could not be validated. On success: null. */ function action_update_product_build_validate( $p_bug_id ) { $t_bug_id = (int)$p_bug_id; if ( bug_is_readonly( $t_bug_id ) ) { - $t_failed_validation_ids = array(); - $t_failed_validation_ids[$t_bug_id] = lang_get( 'actiongroup_error_issue_is_readonly' ); - return $t_failed_validation_ids; + return lang_get( 'actiongroup_error_issue_is_readonly' ); } if ( !access_has_bug_level( config_get( 'update_bug_threshold' ), $t_bug_id ) ) { - $t_failed_validation_ids = array(); - $t_failed_validation_ids[$t_bug_id] = lang_get( 'access_denied' ); - return $t_failed_validation_ids; + return lang_get( 'access_denied' ); } - return true; + return null; } /** * Executes the custom action on the specified bug id. * * @param $p_bug_id The bug id to execute the custom action on. - * @returns true|array Action executed successfully., ( bug_id => reason for failure ) + * @return null Previous validation ensures that this function doesn't fail. Therefore we can always return null to indicate no errors occurred. */ function action_update_product_build_process( $p_bug_id ) { $f_build = gpc_get_string( 'build' ); $t_build = trim( $f_build ); bug_set_field( $p_bug_id, 'build', $t_build ); - return true; + return null; } diff --git a/bug_actiongroup_update_severity_inc.php b/bug_actiongroup_update_severity_inc.php index 18a755c..d275be9 100644 --- a/bug_actiongroup_update_severity_inc.php +++ b/bug_actiongroup_update_severity_inc.php @@ -67,28 +67,23 @@ function action_update_severity_print_fields() { /** * Validates the action on the specified bug id. * - * @returns true Action can be applied. - * @returns array( bug_id => reason for failure ) + * @return string|null On failure: the reason why the action could not be validated. On success: null. */ function action_update_severity_validate( $p_bug_id ) { $f_severity = gpc_get_string( 'severity' ); - $t_failed_validation_ids = array(); - $t_update_severity_threshold = config_get( 'update_bug_threshold' ); $t_bug_id = $p_bug_id; if ( bug_is_readonly( $t_bug_id ) ) { - $t_failed_validation_ids[$t_bug_id] = lang_get( 'actiongroup_error_issue_is_readonly' ); - return $t_failed_validation_ids; + return lang_get( 'actiongroup_error_issue_is_readonly' ); } if ( !access_has_bug_level( $t_update_severity_threshold, $t_bug_id ) ) { - $t_failed_validation_ids[$t_bug_id] = lang_get( 'access_denied' ); - return $t_failed_validation_ids; + return lang_get( 'access_denied' ); } - return true; + return null; } /** @@ -96,11 +91,10 @@ function action_update_severity_validate( $p_bug_id ) { * * @param $p_bug_id The bug id to execute the custom action on. * - * @returns true Action executed successfully. - * @returns array( bug_id => reason for failure ) + * @return null Previous validation ensures that this function doesn't fail. Therefore we can always return null to indicate no errors occurred. */ function action_update_severity_process( $p_bug_id ) { $f_severity = gpc_get_string( 'severity' ); bug_set_field( $p_bug_id, 'severity', $f_severity ); - return true; + return null; } diff --git a/lang/strings_english.txt b/lang/strings_english.txt index a5b3ce6..2803885 100644 --- a/lang/strings_english.txt +++ b/lang/strings_english.txt @@ -1537,6 +1537,7 @@ $s_tag_detach = 'Detach "%1$s"'; $s_tag_separate_by = '(Separate by "%1$s")'; $s_tag_invalid_name = 'Invalid tag name.'; $s_tag_create_denied = 'Create permission denied.'; +$s_tag_attach_denied = 'Attach permission denied.'; $s_tag_filter_default = 'Attached Issues (%1$s)'; $s_tag_history_attached = 'Tag Attached'; $s_tag_history_detached = 'Tag Detached'; ----------------------------------------------------------------------- -- Mantis Bug Tracker |