Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Do not use strict type comparison unless necessary
Fixes #20478
  • Loading branch information
dregad committed Jan 6, 2016
1 parent d6ea7fb commit 101b931
Showing 1 changed file with 27 additions and 27 deletions.
54 changes: 27 additions & 27 deletions bug_update.php
Expand Up @@ -69,7 +69,7 @@

$t_current_user_id = auth_get_current_user_id();

if( helper_get_current_project() !== $t_existing_bug->project_id ) {
if( helper_get_current_project() != $t_existing_bug->project_id ) {
$g_project_override = $t_existing_bug->project_id;
}

Expand Down Expand Up @@ -113,7 +113,7 @@
$t_bug_note->view_state = gpc_get_bool( 'private', config_get( 'default_bugnote_view_status' ) == VS_PRIVATE ) ? VS_PRIVATE : VS_PUBLIC;
$t_bug_note->time_tracking = gpc_get_string( 'time_tracking', '0:00' );

if( $t_existing_bug->last_updated !== $t_updated_bug->last_updated ) {
if( $t_existing_bug->last_updated != $t_updated_bug->last_updated ) {
trigger_error( ERROR_BUG_CONFLICTING_EDIT, ERROR );
}

Expand Down Expand Up @@ -169,7 +169,7 @@
}

# Validate any change to the status of the issue.
if( $t_existing_bug->status !== $t_updated_bug->status ) {
if( $t_existing_bug->status != $t_updated_bug->status ) {
if( !bug_check_workflow( $t_existing_bug->status, $t_updated_bug->status ) ) {
error_parameters( lang_get( 'status' ) );
trigger_error( ERROR_CUSTOM_FIELD_INVALID_VALUE, ERROR );
Expand All @@ -179,13 +179,13 @@
$t_can_bypass_status_access_thresholds = false;
if( $t_close_issue &&
$t_existing_bug->status >= $t_resolved_status &&
$t_existing_bug->reporter_id === $t_current_user_id &&
$t_existing_bug->reporter_id == $t_current_user_id &&
config_get( 'allow_reporter_close' ) ) {
$t_can_bypass_status_access_thresholds = true;
} else if( $t_reopen_issue &&
$t_existing_bug->status >= $t_resolved_status &&
$t_existing_bug->status <= $t_closed_status &&
$t_existing_bug->reporter_id === $t_current_user_id &&
$t_existing_bug->reporter_id == $t_current_user_id &&
config_get( 'allow_reporter_reopen' ) ) {
$t_can_bypass_status_access_thresholds = true;
}
Expand All @@ -201,12 +201,12 @@

# Validate any change to the handler of an issue.
$t_issue_is_sponsored = sponsorship_get_amount( sponsorship_get_all_ids( $f_bug_id ) ) > 0;
if( $t_existing_bug->handler_id !== $t_updated_bug->handler_id ) {
if( $t_existing_bug->handler_id != $t_updated_bug->handler_id ) {
access_ensure_bug_level( config_get( 'update_bug_assign_threshold' ), $f_bug_id );
if( $t_issue_is_sponsored && !access_has_bug_level( config_get( 'handle_sponsored_bugs_threshold' ), $f_bug_id ) ) {
trigger_error( ERROR_SPONSORSHIP_HANDLER_ACCESS_LEVEL_TOO_LOW, ERROR );
}
if( $t_updated_bug->handler_id !== NO_USER ) {
if( $t_updated_bug->handler_id != NO_USER ) {
if( !access_has_bug_level( config_get( 'handle_bug_threshold' ), $f_bug_id, $t_updated_bug->handler_id ) ) {
trigger_error( ERROR_HANDLER_ACCESS_TOO_LOW, ERROR );
}
Expand All @@ -217,8 +217,8 @@
}

# Check whether the category has been undefined when it's compulsory.
if( $t_existing_bug->category_id !== $t_updated_bug->category_id ) {
if( $t_updated_bug->category_id === 0 &&
if( $t_existing_bug->category_id != $t_updated_bug->category_id ) {
if( $t_updated_bug->category_id == 0 &&
!config_get( 'allow_no_category' ) ) {
error_parameters( lang_get( 'category' ) );
trigger_error( ERROR_EMPTY_FIELD, ERROR );
Expand Down Expand Up @@ -252,12 +252,12 @@
}

# Ensure that the user has permission to change the target version of the issue.
if( $t_existing_bug->target_version !== $t_updated_bug->target_version ) {
if( $t_existing_bug->target_version != $t_updated_bug->target_version ) {

This comment has been minimized.

Copy link
@atrol

atrol Jan 6, 2016

Member

Please revert changing this line.
Keep in mind that the following code will output EQUAL (did I mention that I don't like PHP that much because of this?)

<?php
$t_existing_bug_target_version = 1;
$t_updated_bug_target_version = 1.0;

if( $t_existing_bug_target_version != $t_updated_bug_target_version ) {
    echo "NOT EQUAL";
} else {
    echo "EQUAL";
}

This comment has been minimized.

Copy link
@dregad

dregad Jan 6, 2016

Author Member

You're right, didn't consider this case where it converts to float (I was only thinking about SemVer where we have x.y.z so it's always a string compare).

This comment has been minimized.

Copy link
@dregad

dregad Jan 6, 2016

Author Member

Fixed in bddd139

access_ensure_bug_level( config_get( 'roadmap_update_threshold' ), $f_bug_id );
}

# Ensure that the user has permission to change the view status of the issue.
if( (int)$t_existing_bug->view_state !== $t_updated_bug->view_state ) {
if( $t_existing_bug->view_state != $t_updated_bug->view_state ) {
access_ensure_bug_level( config_get( 'change_view_status_threshold' ), $f_bug_id );
}

Expand Down Expand Up @@ -316,8 +316,8 @@
}

# Perform validation of the duplicate ID of the bug.
if( $t_updated_bug->duplicate_id !== 0 ) {
if( $t_updated_bug->duplicate_id === $f_bug_id ) {
if( $t_updated_bug->duplicate_id != 0 ) {
if( $t_updated_bug->duplicate_id == $f_bug_id ) {
trigger_error( ERROR_BUG_DUPLICATE_SELF, ERROR );
}
bug_ensure_exists( $t_updated_bug->duplicate_id );
Expand All @@ -339,7 +339,7 @@
error_parameters( lang_get( 'bugnote' ) );
trigger_error( ERROR_EMPTY_FIELD, ERROR );
}
if( $t_bug_note->view_state !== config_get( 'default_bugnote_view_status' ) ) {
if( $t_bug_note->view_state != config_get( 'default_bugnote_view_status' ) ) {
access_ensure_bug_level( config_get( 'set_view_status_threshold' ), $f_bug_id );
}
}
Expand All @@ -351,20 +351,20 @@
# have one feedback, assigned and submitted status.
if( $t_bug_note->note &&
config_get( 'reassign_on_feedback' ) &&
$t_existing_bug->status === config_get( 'bug_feedback_status' ) &&
$t_updated_bug->status !== $t_existing_bug->status &&
$t_updated_bug->handler_id !== $t_current_user_id &&
$t_updated_bug->reporter_id === $t_current_user_id ) {
if( $t_updated_bug->handler_id !== NO_USER ) {
$t_existing_bug->status == config_get( 'bug_feedback_status' ) &&
$t_updated_bug->status != $t_existing_bug->status &&
$t_updated_bug->handler_id != $t_current_user_id &&
$t_updated_bug->reporter_id == $t_current_user_id ) {
if( $t_updated_bug->handler_id != NO_USER ) {
$t_updated_bug->status = config_get( 'bug_assigned_status' );
} else {
$t_updated_bug->status = config_get( 'bug_submit_status' );
}
}

# Handle automatic assignment of issues.
if( $t_existing_bug->handler_id === NO_USER &&
$t_updated_bug->handler_id !== NO_USER &&
if( $t_existing_bug->handler_id == NO_USER &&
$t_updated_bug->handler_id != NO_USER &&
$t_updated_bug->status == $t_existing_bug->status &&
$t_updated_bug->status < config_get( 'bug_assigned_status' ) &&
config_get( 'auto_set_status_to_assigned' ) ) {
Expand All @@ -380,9 +380,9 @@
$t_updated_bug = event_signal( 'EVENT_UPDATE_BUG_DATA', $t_updated_bug, $t_existing_bug );

# Commit the bug updates to the database.
$t_text_field_update_required = ( $t_existing_bug->description !== $t_updated_bug->description ) ||
( $t_existing_bug->additional_information !== $t_updated_bug->additional_information ) ||
( $t_existing_bug->steps_to_reproduce !== $t_updated_bug->steps_to_reproduce );
$t_text_field_update_required = ( $t_existing_bug->description != $t_updated_bug->description ) ||
( $t_existing_bug->additional_information != $t_updated_bug->additional_information ) ||
( $t_existing_bug->steps_to_reproduce != $t_updated_bug->steps_to_reproduce );
$t_updated_bug->update( $t_text_field_update_required, true );

# Update custom field values.
Expand All @@ -396,7 +396,7 @@
}

# Add a duplicate relationship if requested.
if( $t_updated_bug->duplicate_id !== 0 ) {
if( $t_updated_bug->duplicate_id != 0 ) {
relationship_add( $f_bug_id, $t_updated_bug->duplicate_id, BUG_DUPLICATE );
history_log_event_special( $f_bug_id, BUG_ADD_RELATIONSHIP, BUG_DUPLICATE, $t_updated_bug->duplicate_id );
history_log_event_special( $t_updated_bug->duplicate_id, BUG_ADD_RELATIONSHIP, BUG_HAS_DUPLICATE, $f_bug_id );
Expand Down Expand Up @@ -425,9 +425,9 @@
email_relationship_child_closed( $f_bug_id );
} else if( $t_reopen_issue ) {
email_bug_reopened( $f_bug_id );
} else if( $t_existing_bug->handler_id !== $t_updated_bug->handler_id ) {
} else if( $t_existing_bug->handler_id != $t_updated_bug->handler_id ) {
email_owner_changed( $f_bug_id, $t_existing_bug->handler_id, $t_updated_bug->handler_id );
} else if( $t_existing_bug->status !== $t_updated_bug->status ) {
} else if( $t_existing_bug->status != $t_updated_bug->status ) {
$t_new_status_label = MantisEnum::getLabel( config_get( 'status_enum_string' ), $t_updated_bug->status );
$t_new_status_label = str_replace( ' ', '_', $t_new_status_label );
email_bug_status_changed( $f_bug_id, $t_new_status_label );
Expand Down

0 comments on commit 101b931

Please sign in to comment.