Skip to content

Commit

Permalink
Fix #11552: No errors shown when actiongroup tag attaching fails
Browse files Browse the repository at this point in the history 
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).
  • Loading branch information
@davidhicks
davidhicks committed Feb 25, 2010
1 parent c3dedbb commit 2191aa1
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 83 deletions.
15 changes: 6 additions & 9 deletions bug_actiongroup_add_note_inc.php
View file
Expand Up @@ -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' );
Expand All @@ -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;
}
74 changes: 32 additions & 42 deletions bug_actiongroup_attach_tags_inc.php
View file
Expand Up @@ -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;
Expand All @@ -138,5 +128,5 @@ function action_attach_tags_process( $p_bug_id ) {
}
}

return true;
return null;
}
18 changes: 8 additions & 10 deletions bug_actiongroup_ext.php
View file
Expand Up @@ -79,16 +79,14 @@
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;
}
}
}
Expand All @@ -103,9 +101,9 @@

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

Expand Down
16 changes: 6 additions & 10 deletions bug_actiongroup_update_product_build_inc.php
View file
Expand Up @@ -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;
}
18 changes: 6 additions & 12 deletions bug_actiongroup_update_severity_inc.php
View file
Expand Up @@ -67,40 +67,34 @@ 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;
}

/**
* Executes the custom action on the specified 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;
}
1 change: 1 addition & 0 deletions lang/strings_english.txt
View file
Expand Up @@ -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';
Expand Down

0 comments on commit 2191aa1

Please sign in to comment.