Skip to content

Commit

Permalink
Add 'Close' button for bugs
Browse files Browse the repository at this point in the history 
This commit introduces a new 'Close' button in the View Issue Details
page, making it easier and more intuitive for reporters to close their
own issues (when $g_allow_reporter_close = ON).

It also improves the access level verifications, simplifies the code and
makes it more readable by using standard access check functions.

This is an improvement over the functionality implemented in #11502 not
only in terms of usability, but also for security as in some specific
cases users could actually bypass the workflow.

Fixes #14156
  • Loading branch information
@dregad
dregad committed Apr 14, 2012
1 parent 0630f85 commit 085ab16
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 54 deletions.
52 changes: 31 additions & 21 deletions bug_change_status_page.php
View file
Expand Up @@ -82,14 +82,20 @@
$f_new_status = gpc_get_int( 'new_status' );
$f_reopen_flag = gpc_get_int( 'reopen_flag', OFF );

$t_reopen = config_get( 'bug_reopen_status', null, null, $t_bug->project_id );
$t_resolved = config_get( 'bug_resolved_status_threshold', null, null, $t_bug->project_id );
$t_closed = config_get( 'bug_closed_status_threshold', null, null, $t_bug->project_id );
$t_current_user_id = auth_get_current_user_id();

if ( !( ( access_has_bug_level( access_get_status_threshold( $f_new_status, bug_get_field( $f_bug_id, 'project_id' ) ), $f_bug_id ) ) ||
( ( bug_get_field( $f_bug_id, 'reporter_id' ) == $t_current_user_id ) &&
( ( ON == config_get( 'allow_reporter_reopen' ) ) ||
( ON == config_get( 'allow_reporter_close' ) ) ) ) ||
( ( ON == $f_reopen_flag ) && ( access_has_bug_level( config_get( 'reopen_bug_threshold' ), $f_bug_id ) ) )
) ) {
# Ensure user has proper access level before proceeding
if( $f_new_status == $t_reopen && $f_reopen_flag ) {
access_ensure_can_reopen_bug( $f_bug_id, $t_current_user_id );
}
else if( $f_new_status == $t_closed ) {
access_ensure_can_close_bug( $f_bug_id, $t_current_user_id );
}
else if ( bug_is_readonly( $f_bug_id )
|| !access_has_bug_level( access_get_status_threshold( $f_new_status, $t_bug->project_id ), $f_bug_id, $t_current_user_id ) ) {
access_denied();
}

Expand Down Expand Up @@ -126,10 +132,6 @@
}

$t_status_label = str_replace( " ", "_", MantisEnum::getLabel( config_get( 'status_enum_string' ), $f_new_status ) );
$t_resolved = config_get( 'bug_resolved_status_threshold' );
$t_closed = config_get( 'bug_closed_status_threshold' );

$t_bug = bug_get( $f_bug_id );

html_page_top( bug_format_summary( $f_bug_id, SUMMARY_CAPTION ) );

Expand Down Expand Up @@ -225,13 +227,13 @@
</tr>
<?php } ?>

<!-- Due date -->
<?php if ( $t_can_update_due_date ) {
$t_date_to_display = '';
if ( !date_is_null( $t_bug->due_date ) ) {
$t_date_to_display = date( config_get( 'calendar_date_format' ), $t_bug->due_date );
}
?>
<!-- Due date -->
<tr <?php echo helper_alternate_class() ?>>
<th class="category">
<?php print_documentation_link( 'due_date' ) ?>
Expand Down Expand Up @@ -301,11 +303,11 @@
?>

<?php
if ( ( $t_resolved <= $f_new_status ) ) {
$t_show_product_version = ( ON == config_get( 'show_product_version' ) )
|| ( ( AUTO == config_get( 'show_product_version' ) )
&& ( count( version_get_all_rows( $t_bug->project_id ) ) > 0 ) );
if ( $t_show_product_version ) {
if ( ( $f_new_status >= $t_resolved ) ) {
if ( version_should_show_product_version( $t_bug->project_id )
&& !bug_is_readonly( $f_bug_id )
&& access_has_bug_level( config_get( 'update_bug_threshold' ), $f_bug_id )
) {
?>
<!-- Fixed in Version -->
<tr <?php echo helper_alternate_class() ?>>
Expand All @@ -314,15 +316,23 @@
</th>
<td>
<select name="fixed_in_version">
<?php print_version_option_list( bug_get_field( $f_bug_id, 'fixed_in_version' ),
bug_get_field( $f_bug_id, 'project_id' ), VERSION_ALL ) ?>
<?php print_version_option_list( $t_bug->fixed_in_version, $t_bug->project_id, VERSION_ALL ) ?>
</select>
</td>
</tr>
<?php }
} ?>

<?php
}
}
?>
<?php event_signal( 'EVENT_BUG_CHANGE_STATUS_FORM', array( $f_bug_id ) ); ?>
<?php
if ( ON == $f_reopen_flag ) {
?>
<!-- Bug was re-opened -->
<?php
printf(" <input type=\"hidden\" name=\"resolution\" value=\"%s\" />\n", config_get( 'bug_reopen_resolution' ) );
}
?>

<!-- Bugnote -->
<tr id="bug-change-status-note" <?php echo helper_alternate_class() ?>>
Expand Down
5 changes: 3 additions & 2 deletions bug_update_advanced_page.php
View file
Expand Up @@ -401,8 +401,9 @@

echo '<td class="' . $status_label . '">';
print_status_option_list( 'status', $tpl_bug->status,
( $tpl_bug->reporter_id == auth_get_current_user_id() &&
( ON == config_get( 'allow_reporter_close' ) ) ), $tpl_bug->project_id );
access_can_close_bug( $tpl_bug->id ),
$tpl_bug->project_id
);
echo '</td>';
} else {
$t_spacer += 2;
Expand Down
47 changes: 34 additions & 13 deletions core/access_api.php
View file
Expand Up @@ -490,31 +490,40 @@ function access_ensure_bugnote_level( $p_access_level, $p_bugnote_id, $p_user_id
}

/**
* Check if the current user can close the specified bug
* Check if the specified bug can be closed
* @param int $p_bug_id integer representing bug id to check access against
* @param int|null $p_user_id integer representing user id, defaults to null to use current user
* @return bool whether user has access to close bugs
* @return bool true if user can close the bug
* @access public
*/
function access_can_close_bug( $p_bug_id, $p_user_id = null ) {
if( null === $p_user_id ) {
$p_user_id = auth_get_current_user_id();
if( bug_is_closed( $p_bug_id ) ) {
# Can't close a bug that's already closed
return false;
}

# If allow_reporter_close is enabled, then reporters can always close their own bugs
if( ON == config_get( 'allow_reporter_close' ) && bug_is_user_reporter( $p_bug_id, $p_user_id ) ) {
return true;
if( null === $p_user_id ) {
$p_user_id = auth_get_current_user_id();
}

$t_bug = bug_get( $p_bug_id );

$t_closed_status_threshold = access_get_status_threshold( config_get( 'bug_closed_status_threshold' ), $t_bug->project_id );
# If allow_reporter_close is enabled, then reporters can close their own bugs
# if they are in resolved status
if( ON == config_get( 'allow_reporter_close', null, null, $t_bug->project_id )
&& bug_is_user_reporter( $p_bug_id, $p_user_id )
&& bug_is_resolved( $p_bug_id )
) {
return true;
}

$t_closed_status = config_get( 'bug_closed_status_threshold', null, null, $t_bug->project_id );
$t_closed_status_threshold = access_get_status_threshold( $t_closed_status, $t_bug->project_id );
return access_has_bug_level( $t_closed_status_threshold, $p_bug_id, $p_user_id );
}

/**
* Make sure that the current user can close the specified bug
* Make sure that the user can close the specified bug
* @see access_can_close_bug
* @param int $p_bug_id integer representing bug id to check access against
* @param int|null $p_user_id integer representing user id, defaults to null to use current user
Expand All @@ -527,27 +536,39 @@ function access_ensure_can_close_bug( $p_bug_id, $p_user_id = null ) {
}

/**
* Check if the current user can reopen the specified bug
* Check if the specified bug can be reopened
* @param int $p_bug_id integer representing bug id to check access against
* @param int|null $p_user_id integer representing user id, defaults to null to use current user
* @return bool whether user has access to reopen bugs
* @access public
*/
function access_can_reopen_bug( $p_bug_id, $p_user_id = null ) {
if( !bug_is_resolved( $p_bug_id ) ) {
# Can't reopen a bug that's not resolved
return false;
}

if( $p_user_id === null ) {
$p_user_id = auth_get_current_user_id();
}

$t_bug = bug_get( $p_bug_id );

# If allow_reporter_reopen is enabled, then reporters can always reopen their own bugs
if( ON == config_get( 'allow_reporter_reopen' ) && bug_is_user_reporter( $p_bug_id, $p_user_id ) ) {
if( ON == config_get( 'allow_reporter_reopen', null, null, $t_bug->project_id )
&& bug_is_user_reporter( $p_bug_id, $p_user_id )
) {
return true;
}

return access_has_bug_level( config_get( 'reopen_bug_threshold' ), $p_bug_id, $p_user_id );
$t_reopen_status = config_get( 'reopen_bug_threshold', null, null, $t_bug->project_id );
$t_reopen_status_threshold = access_get_status_threshold( $t_reopen_status, $t_bug->project_id );

return access_has_bug_level( $t_reopen_status_threshold, $p_bug_id, $p_user_id );
}

/**
* Make sure that the current user can reopen the specified bug.
* Make sure that the user can reopen the specified bug.
* Calls access_denied if user has no access to terminate script
* @see access_can_reopen_bug
* @param int $p_bug_id integer representing bug id to check access against
Expand Down
16 changes: 14 additions & 2 deletions core/bug_api.php
View file
Expand Up @@ -846,8 +846,20 @@ function bug_is_readonly( $p_bug_id ) {
* @uses config_api.php
*/
function bug_is_resolved( $p_bug_id ) {
$t_status = bug_get_field( $p_bug_id, 'status' );
return( $t_status >= config_get( 'bug_resolved_status_threshold' ) );
$t_bug = bug_get( $p_bug_id );
return( $t_bug->status >= config_get( 'bug_resolved_status_threshold', null, null, $t_bug->project_id ) );
}

/**
* Check if a given bug is closed
* @param int p_bug_id integer representing bug id
* @return bool true if bug is closed, false otherwise
* @access public
* @uses config_api.php
*/
function bug_is_closed( $p_bug_id ) {
$t_bug = bug_get( $p_bug_id );
return( $t_bug->status >= config_get( 'bug_closed_status_threshold', null, null, $t_bug->project_id ) );
}

/**
Expand Down
56 changes: 40 additions & 16 deletions core/html_api.php
View file
Expand Up @@ -1339,7 +1339,7 @@ function html_status_percentage_legend() {
/**
* Print an html button inside a form
* @param string $p_action
* @param string $p_buttion_text
* @param string $p_button_text
* @param array $p_fields
* @param string $p_method
* @return null
Expand Down Expand Up @@ -1401,7 +1401,12 @@ function html_button_bug_change_status( $p_bug_id ) {
$t_bug_current_state = bug_get_field( $p_bug_id, 'status' );
$t_current_access = access_get_project_level( $t_bug_project_id );

$t_enum_list = get_status_option_list( $t_current_access, $t_bug_current_state, false, ( bug_get_field( $p_bug_id, 'reporter_id' ) == auth_get_current_user_id() && ( ON == config_get( 'allow_reporter_close' ) ) ), $t_bug_project_id );
$t_enum_list = get_status_option_list(
$t_current_access,
$t_bug_current_state,
false,
bug_is_user_reporter( $p_bug_id, auth_get_current_user_id() ) && ( ON == config_get( 'allow_reporter_close' ) ),
$t_bug_project_id );

if( count( $t_enum_list ) > 0 ) {

Expand Down Expand Up @@ -1561,13 +1566,29 @@ function html_button_bug_create_child( $p_bug_id ) {
* @return null
*/
function html_button_bug_reopen( $p_bug_id ) {
$t_status = bug_get_field( $p_bug_id, 'status' );
$t_project = bug_get_field( $p_bug_id, 'project_id' );
if( access_can_reopen_bug( $p_bug_id ) ) {
$t_reopen_status = config_get( 'bug_reopen_status', null, null, $t_project );
html_button(
'bug_change_status_page.php',
lang_get( 'reopen_bug_button' ),
array( 'id' => $p_bug_id, 'new_status' => $t_reopen_status, 'reopen_flag' => ON )
);
}
}

if( access_has_bug_level( config_get( 'reopen_bug_threshold', null, null, $t_project ), $p_bug_id ) ||
(( bug_get_field( $p_bug_id, 'reporter_id' ) == auth_get_current_user_id() ) && ( ON == config_get( 'allow_reporter_reopen', null, null, $t_project ) ) ) ) {
html_button( 'bug_change_status_page.php', lang_get( 'reopen_bug_button' ), array( 'id' => $p_bug_id, 'new_status' => $t_reopen_status, 'reopen_flag' => ON ) );
/**
* Print a button to close the given bug
* @param int $p_bug_id
* @return null
*/
function html_button_bug_close( $p_bug_id ) {
if( access_can_close_bug( $p_bug_id ) ) {
$t_closed_status = config_get( 'bug_closed_status_threshold', null, null, $t_project );
html_button(
'bug_change_status_page.php',
lang_get( 'close_bug_button' ),
array( 'id' => $p_bug_id, 'new_status' => $t_closed_status )
);
}
}

Expand Down Expand Up @@ -1645,6 +1666,7 @@ function html_button_wiki( $p_bug_id ) {
*/
function html_buttons_view_bug_page( $p_bug_id ) {
$t_resolved = config_get( 'bug_resolved_status_threshold' );
$t_closed = config_get( 'bug_closed_status_threshold' );
$t_status = bug_get_field( $p_bug_id, 'status' );
$t_readonly = bug_is_readonly( $p_bug_id );
$t_sticky = config_get( 'set_bug_sticky_threshold' );
Expand All @@ -1663,7 +1685,7 @@ function html_buttons_view_bug_page( $p_bug_id ) {
}

# Change status button/dropdown
if ( !$t_readonly || config_get( 'allow_reporter_close' ) ) {
if ( !$t_readonly ) {
echo '<td class="center">';
html_button_bug_change_status( $p_bug_id );
echo '</td>';
Expand Down Expand Up @@ -1691,21 +1713,23 @@ function html_buttons_view_bug_page( $p_bug_id ) {
echo '</td>';
}

# CLONE button
if( !$t_readonly ) {
# CREATE CHILD button
echo '<td class="center">';
html_button_bug_create_child( $p_bug_id );
echo '</td>';
}

if( $t_resolved <= $t_status ) {
# resolved is not the same as readonly
echo '<td class="center">';
# REOPEN button
echo '<td class="center">';
html_button_bug_reopen( $p_bug_id );
echo '</td>';

# CLOSE button
echo '<td class="center">';
html_button_bug_close( $p_bug_id );
echo '</td>';

# REOPEN button
html_button_bug_reopen( $p_bug_id );
echo '</td>';
}

# MOVE button
echo '<td class="center">';
Expand Down
1 change: 1 addition & 0 deletions lang/strings_english.txt
View file
Expand Up @@ -1177,6 +1177,7 @@ If you requested this verification, visit the following URL to change your passw
'bug_assign_to_button' => 'Assign To:',
'bug_status_to_button' => 'Change Status To:',
'reopen_bug_button' => 'Reopen',
'close_bug_button' => 'Close',
'move_bug_button' => 'Move',
'attached_files' => 'Attached Files',
'publish' => 'Publish',
Expand Down

0 comments on commit 085ab16

Please sign in to comment.