Navigation Menu

Skip to content

Commit

Permalink
Ensure temporary filter is tracked through view_all_page
Browse files Browse the repository at this point in the history 
Implement correct tracking of the temporary filter id through
view_all_set actions, by passing the filter id as parameter from all
action links and filter form.

Fixes: #8204
  • Loading branch information
@cproensa @atrol
cproensa authored and atrol committed Mar 4, 2018
1 parent 5079c07 commit 1fc514b
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 47 deletions.
73 changes: 61 additions & 12 deletions core/filter_api.php
View file
Expand Up @@ -1000,6 +1000,7 @@ function filter_deserialize( $p_serialized_filter ) {
*/
function filter_serialize( $p_filter_array ) {
$t_cookie_version = FILTER_VERSION;
filter_clean_runtime_properties( $p_filter_array );
$t_settings_serialized = json_encode( $p_filter_array );
$t_settings_string = $t_cookie_version . '#' . $t_settings_serialized;
return $t_settings_string;
Expand Down Expand Up @@ -2454,11 +2455,14 @@ function filter_draw_selection_area( $p_page_number, $p_for_screen = true, $p_ex
<?php # CSRF protection not required here - form does not result in modifications ?>
<input type="hidden" name="type" value="<?php echo $t_view_all_set_type ?>" />
<?php
if( $p_for_screen == false ) {
echo '<input type="hidden" name="print" value="1" />';
echo '<input type="hidden" name="offset" value="0" />';
}
?>
if( filter_is_temporary( $t_filter ) ) {
echo '<input type="hidden" name="filter" value="' . filter_get_temporary_key( $t_filter ) . '" />';
}
if( $p_for_screen == false ) {
echo '<input type="hidden" name="print" value="1" />';
echo '<input type="hidden" name="offset" value="0" />';
}
?>
<input type="hidden" name="page_number" value="<?php echo $t_page_number?>" />
<input type="hidden" name="view_type" value="<?php echo $t_view_type?>" />
<?php
Expand Down Expand Up @@ -2540,7 +2544,7 @@ function filter_draw_selection_area( $p_page_number, $p_for_screen = true, $p_ex
<input id="filter-bar-search-txt" type="text" size="16" class="input-xs"
placeholder="<?php echo lang_get( 'search' ) ?>"
value="<?php echo string_attribute( $t_filter[FILTER_PROPERTY_SEARCH] ); ?>" />
<button id="filter-bar-search-btn" type="submit" name="filter" class="btn btn-primary btn-white btn-round btn-xs"
<button id="filter-bar-search-btn" type="submit" name="filter_submit" class="btn btn-primary btn-white btn-round btn-xs"
title="<?php echo lang_get( 'filter_button' ) ?>">
<i class="ace-icon fa fa-search"></i>
</button>
Expand Down Expand Up @@ -2571,7 +2575,7 @@ function filter_draw_selection_area( $p_page_number, $p_for_screen = true, $p_ex
echo '<input type="text" id="filter-search-txt" class="input-sm" size="16" name="', FILTER_PROPERTY_SEARCH, '"
placeholder="' . lang_get( 'search' ) . '" value="', string_attribute( $t_filter[FILTER_PROPERTY_SEARCH] ), '" />';
?>
<input type="submit" class="btn btn-primary btn-sm btn-white btn-round no-float" name="filter" value="<?php echo lang_get( 'filter_button' )?>" />
<input type="submit" class="btn btn-primary btn-sm btn-white btn-round no-float" name="filter_submit" value="<?php echo lang_get( 'filter_button' )?>" />
</div>
<?php

Expand Down Expand Up @@ -3484,6 +3488,14 @@ function filter_gpc_get( array $p_filter = null ) {
$t_filter_input[FILTER_PROPERTY_NOTE_USER_ID] = $f_note_user_id;
$t_filter_input[FILTER_PROPERTY_MATCH_TYPE] = $f_match_type;

# copy runtime properties, if present
if( isset( $t_filter['_temporary_key'] ) ) {
$t_filter_input['_temporary_key'] = $t_filter['_temporary_key'];
}
if( isset( $t_filter['_filter_id'] ) ) {
$t_filter_input['_filter_id'] = $t_filter['_filter_id'];
}

return filter_ensure_valid_filter( $t_filter_input );
}

Expand Down Expand Up @@ -3745,6 +3757,8 @@ function filter_get( $p_filter_id, array $p_default = null ) {
}
}

$t_filter['_filter_id'] = $p_filter_id;

return $t_filter;
}

Expand All @@ -3769,6 +3783,7 @@ function filter_temporary_get( $p_filter_key, $p_default = null ) {
# setting here the key in the filter array only if the key exists
# this validates against receiving garbage input as XSS attacks
$t_filter = $t_session_filters[$p_filter_key];
$t_filter['_temporary_key'] = $p_filter_key;
return filter_ensure_valid_filter( $t_filter );
} else {
if( $t_trigger_error ) {
Expand Down Expand Up @@ -3800,6 +3815,8 @@ function filter_temporary_set( array $p_filter, $p_filter_key = null ) {
} else {
$t_filter_key = $p_filter_key;
}

filter_clean_runtime_properties( $p_filter );
$t_session_filters = session_get( 'temporary_filters', array() );
$t_session_filters[$t_filter_key] = $p_filter;
session_set( 'temporary_filters', $t_session_filters );
Expand All @@ -3821,17 +3838,49 @@ function filter_get_temporary_key( array $p_filter ) {
}

/**
* Returns a string formatted as GET parameter, suitable for tracking a
* temporary filter by its session key
* If the filter was not originated from a temporary key, returns an empty string
* Returns true if the filter was loaded as temporary filter
* @param array $p_filter Filter array
* @return boolean Whether this filter is temporary
*/
function filter_is_temporary( array $p_filter ) {
return isset( $p_filter['_temporary_key'] );
}

/**
* Returns a string formatted as GET parameter, suitable for tracking a
* temporary filter by its session key.
* The parameter can be ither an existing key, so its used directly,
* or a filter array, which can contain a property with the key
* If a filter is provided that does not contain the key proeprty, an empty
* string is returned.
* @param array|string $p_key_or_filter Either a string key, or a filter array
* @return string Formatted parameter string, or empty
*/
function filter_get_temporary_key_param( array $p_filter ) {
$t_key = filter_get_temporary_key( $p_filter );
function filter_get_temporary_key_param( $p_key_or_filter ) {
if( is_array( $p_key_or_filter ) ) {
$t_key = filter_get_temporary_key( $p_key_or_filter );
} else {
$t_key = $p_key_or_filter;
}
if( $t_key ) {
return 'filter=' . $t_key;
} else {
return '';
}
}

/**
* Removes runtime properties that are should not be saved as part of the filter
* Use this function before saving the filter.
* @param array $p_filter Filter array (passed as reference, it gets modified)
* @return array Modified filter array
*/
function filter_clean_runtime_properties( array &$p_filter ) {
if( isset( $p_filter['_temporary_id'] ) ) {
unset( $p_filter['_temporary_id'] );
}
if( isset( $p_filter['_filter_id'] ) ) {
unset( $p_filter['_filter_id'] );
}
return $p_filter;
}
6 changes: 4 additions & 2 deletions core/filter_form_api.php
View file
Expand Up @@ -2387,8 +2387,10 @@ function filter_form_draw_inputs( $p_filter, $p_for_screen = true, $p_static = f
if( $p_static) {
return $p_label;
} else {
if( $t_source_query_id > 0 ) {
$t_data_filter_id = ' data-filter_id="' . $t_source_query_id . '"';
if( filter_is_temporary( $t_filter ) ) {
$t_data_filter_id = ' data-filter="' . filter_get_temporary_key( $t_filter ) . '"';
} elseif ( isset( $t_filter['_filter_id'] ) ) {
$t_data_filter_id = ' data-filter_id="' . $t_filter['_filter_id'] . '"';
} else {
$t_data_filter_id = '';
}
Expand Down
44 changes: 27 additions & 17 deletions core/print_api.php
View file
Expand Up @@ -1317,6 +1317,12 @@ function print_formatted_severity_string( BugData $p_bug ) {
* @return void
*/
function print_view_bug_sort_link( $p_string, $p_sort_field, $p_sort, $p_dir, $p_columns_target = COLUMNS_TARGET_VIEW_PAGE ) {
# @TODO cproensa, $g_filter is needed to get the temporary id, since the
# actual filter is not providede as parameter. Ideally, we should not
# rely in this global variable, but at the moment is not possible without
# a rewrite of these print functions.
global $g_filter;

switch( $p_columns_target ) {
case COLUMNS_TARGET_PRINT_PAGE:
case COLUMNS_TARGET_VIEW_PAGE:
Expand All @@ -1333,7 +1339,8 @@ function print_view_bug_sort_link( $p_string, $p_sort_field, $p_sort, $p_dir, $p
}
$t_sort_field = rawurlencode( $p_sort_field );
$t_print_parameter = ( $p_columns_target == COLUMNS_TARGET_PRINT_PAGE ) ? '&print=1' : '';
print_link( 'view_all_set.php?sort_add=' . $t_sort_field . '&dir_add=' . $p_dir . '&type=2' . $t_print_parameter, $p_string );
$t_filter_parameter = filter_is_temporary( $g_filter ) ? filter_get_temporary_key_param( $g_filter ) . '&' : '';
print_link( 'view_all_set.php?' . $t_filter_parameter . 'sort_add=' . $t_sort_field . '&dir_add=' . $p_dir . '&type=2' . $t_print_parameter, $p_string );
break;
default:
echo $p_string;
Expand Down Expand Up @@ -1513,19 +1520,19 @@ function print_small_button( $p_link, $p_url_text, $p_new_window = false ) {
* @param string $p_text The displayed text for the link.
* @param integer $p_page_no The page number to link to.
* @param integer $p_page_cur The current page number.
* @param integer $p_temp_filter_id Temporary filter id.
* @param integer $p_temp_filter_key Temporary filter id.
* @return void
*/
function print_page_link( $p_page_url, $p_text = '', $p_page_no = 0, $p_page_cur = 0, $p_temp_filter_id = 0 ) {
function print_page_link( $p_page_url, $p_text = '', $p_page_no = 0, $p_page_cur = 0, $p_temp_filter_key = null ) {
if( is_blank( $p_text ) ) {
$p_text = $p_page_no;
}

if( ( 0 < $p_page_no ) && ( $p_page_no != $p_page_cur ) ) {
echo '<li class="pull-right"> ';
$t_delimiter = ( strpos( $p_page_url, '?' ) ? '&' : '?' );
if( $p_temp_filter_id !== 0 ) {
print_link( $p_page_url . $t_delimiter . 'filter=' . $p_temp_filter_id . '&page_number=' . $p_page_no, $p_text );
if( $p_temp_filter_key ) {
print_link( $p_page_url . $t_delimiter . 'filter=' . $p_temp_filter_key . '&page_number=' . $p_page_no, $p_text );
} else {
print_link( $p_page_url . $t_delimiter . 'page_number=' . $p_page_no, $p_text );
}
Expand All @@ -1541,12 +1548,17 @@ function print_page_link( $p_page_url, $p_text = '', $p_page_no = 0, $p_page_cur
* @param integer $p_start The first page number.
* @param integer $p_end The last page number.
* @param integer $p_current The current page number.
* @param integer $p_temp_filter_id Temporary filter id.
* @param integer $p_temp_filter_key Temporary filter id.
* @return void
*/
function print_page_links( $p_page, $p_start, $p_end, $p_current, $p_temp_filter_id = 0 ) {
function print_page_links( $p_page, $p_start, $p_end, $p_current, $p_temp_filter_key = null ) {
$t_items = array();

# @TODO cproensa
# passing the temporary filter id to build ad-hoc url parameter is weak
# ideally, we should pass a parameters array which is appended to all generated links
# those parameters are provided as needed by the main page calling this functions

# Check if we have more than one page,
# otherwise return without doing anything.

Expand All @@ -1565,11 +1577,11 @@ function print_page_links( $p_page, $p_start, $p_end, $p_current, $p_temp_filter
print( '<ul class="pagination small no-margin"> ' );

# Next and Last links
print_page_link( $p_page, $t_last, $p_end, $p_current, $p_temp_filter_id );
print_page_link( $p_page, $t_last, $p_end, $p_current, $p_temp_filter_key );
if( $p_current < $p_end ) {
print_page_link( $p_page, $t_next, $p_current + 1, $p_current, $p_temp_filter_id );
print_page_link( $p_page, $t_next, $p_current + 1, $p_current, $p_temp_filter_key );
} else {
print_page_link( $p_page, $t_next, null, null, $p_temp_filter_id );
print_page_link( $p_page, $t_next, null, null, $p_temp_filter_key );
}

# Page numbers ...
Expand All @@ -1590,11 +1602,9 @@ function print_page_links( $p_page, $p_start, $p_end, $p_current, $p_temp_filter
array_push( $t_items, '<li class="active pull-right"><a>' . $i . '</a></li>' );
} else {
$t_delimiter = ( strpos( $p_page, '?' ) ? '&' : '?' ) ;
if( $p_temp_filter_id !== 0 ) {
array_push( $t_items, '<li class="pull-right"><a href="' . $p_page . $t_delimiter . 'filter=' . $p_temp_filter_id . '&amp;page_number=' . $i . '">' . $i . '</a></li>' );
} else {
array_push( $t_items, '<li class="pull-right"><a href="' . $p_page . $t_delimiter . 'page_number=' . $i . '">' . $i . '</a></li>' );
}
$t_filter_param = filter_get_temporary_key_param( $p_temp_filter_key );
$t_filter_param .= $t_filter_param === null ? : '&amp;';
array_push( $t_items, '<li class="pull-right"><a href="' . $p_page . $t_delimiter . $t_filter_param . 'page_number=' . $i . '">' . $i . '</a></li>' );
}
}
echo implode( '&#160;', $t_items );
Expand All @@ -1605,8 +1615,8 @@ function print_page_links( $p_page, $p_start, $p_end, $p_current, $p_temp_filter


# First and previous links
print_page_link( $p_page, $t_prev, $p_current - 1, $p_current, $p_temp_filter_id );
print_page_link( $p_page, $t_first, 1, $p_current, $p_temp_filter_id );
print_page_link( $p_page, $t_prev, $p_current - 1, $p_current, $p_temp_filter_key );
print_page_link( $p_page, $t_first, 1, $p_current, $p_temp_filter_key );

print( ' </ul>' );
}
Expand Down
4 changes: 4 additions & 0 deletions js/common.js
View file
Expand Up @@ -122,13 +122,17 @@ $(document).ready( function() {
event.preventDefault();
var fieldID = $(this).attr('id');
var filter_id = $(this).data('filter_id');
var filter_tmp_id = $(this).data('filter');
var targetID = fieldID + '_target';
var viewType = $('#filters_form_open input[name=view_type]').val();
$('#' + targetID).html('<span class="dynamic-filter-loading">' + translations['loading'] + "</span>");
var params = 'view_type=' + viewType + '&filter_target=' + fieldID;
if( undefined !== filter_id ) {
params += '&filter_id=' + filter_id;
}
if( undefined !== filter_tmp_id ) {
params += '&filter=' + filter_tmp_id;
}
$.ajax({
url: 'return_dynamic_filters.php',
data: params,
Expand Down
6 changes: 3 additions & 3 deletions return_dynamic_filters.php
View file
Expand Up @@ -61,13 +61,13 @@
compress_enable();

$f_filter_id = gpc_get( 'filter_id', null );
if( null === $f_filter_id ) {
$t_filter = current_user_get_bug_filter();
} else {
if( null !== $f_filter_id ) {
$t_filter = filter_get( $f_filter_id, null );
if( null === $t_filter ) {
trigger_error( ERROR_ACCESS_DENIED, ERROR );
}
} else {
$t_filter = current_user_get_bug_filter();
}

$f_view_type = gpc_get_string( 'view_type', $t_filter['_view_type'] );
Expand Down
8 changes: 4 additions & 4 deletions view_all_inc.php
View file
Expand Up @@ -135,8 +135,8 @@
</div>
<div class="btn-group pull-right"><?php
# -- Page number links --
$f_tmp_filter = gpc_get_string( 'filter', 0);
print_page_links( 'view_all_bug_page.php', 1, $t_page_count, (int)$f_page_number, $f_tmp_filter );
$t_tmp_filter_key = filter_get_temporary_key( $t_filter );
print_page_links( 'view_all_bug_page.php', 1, $t_page_count, (int)$f_page_number, $t_tmp_filter_key );
?>
</div>
</div>
Expand Down Expand Up @@ -235,8 +235,8 @@ function write_bug_rows( array $p_rows ) {
</div>
<div class="btn-group pull-right">
<?php
$f_tmp_filter = gpc_get_string('filter', 0);
print_page_links('view_all_bug_page.php', 1, $t_page_count, (int)$f_page_number, $f_tmp_filter);
$t_tmp_filter_key = filter_get_temporary_key( $t_filter );
print_page_links('view_all_bug_page.php', 1, $t_page_count, (int)$f_page_number, $t_tmp_filter_key );
?>
</div>
<?php # -- ====================== end of MASS BUG MANIPULATION ========================= -- ?>
Expand Down
22 changes: 13 additions & 9 deletions view_all_set.php
View file
Expand Up @@ -58,11 +58,7 @@
$f_type = gpc_get_int( 'type', -1 );
$f_source_query_id = gpc_get_int( 'source_query_id', -1 );
$f_print = gpc_get_bool( 'print' );
$f_temp_filter = gpc_get_bool( 'temporary' );

if( $f_temp_filter ) {
$f_type = 1;
}
$f_make_temporary = gpc_get_bool( 'temporary' );

if( $f_type < 0 ) {
print_header_redirect( 'view_all_bug_page.php' );
Expand All @@ -75,6 +71,9 @@

# Get the filter in use
$t_setting_arr = current_user_get_bug_filter();
$t_temp_filter = $f_make_temporary || filter_is_temporary( $t_setting_arr );
$t_previous_temporary_key = filter_get_temporary_key( $t_setting_arr );


# Clear the source query id. Since we have entered new filter criteria.
if( isset( $t_setting_arr['_source_query_id'] ) ) {
Expand Down Expand Up @@ -159,7 +158,7 @@
$t_setting_arr = filter_ensure_valid_filter( $t_setting_arr );

# If only using a temporary filter, don't store it in the database
if( !$f_temp_filter ) {
if( !$t_temp_filter ) {
# Store the filter string in the database: its the current filter, so some values won't change
filter_set_project_filter( $t_setting_arr );
}
Expand All @@ -171,8 +170,13 @@
$t_redirect_url = 'view_all_bug_page.php';
}

if( $f_temp_filter ) {
$t_temp_key = filter_temporary_set( $t_setting_arr );
$t_redirect_url = $t_redirect_url . '?filter=' . $t_temp_key;
if( $t_temp_filter ) {
# keeping the $t_previous_temporary_key, and using it to save back the filter
# The key inside the filter array may have been deleted as part of some actions
# Note, if we reset the key here, a new filter will be created after each action.
# This adds a lot of orphaned filters to session store, but would allow consistency
# through browser back button, for example.
$t_temporary_key = filter_temporary_set( $t_setting_arr, $t_previous_temporary_key );
$t_redirect_url = $t_redirect_url . '?' . filter_get_temporary_key_param( $t_temporary_key );
}
print_header_redirect( $t_redirect_url );

0 comments on commit 1fc514b

Please sign in to comment.