From 1fc514b5cf1a8d0007aa4b04692275029663af08 Mon Sep 17 00:00:00 2001 From: Carlos Proensa Date: Wed, 12 Apr 2017 19:46:26 +0200 Subject: [PATCH] Ensure temporary filter is tracked through view_all_page 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 --- core/filter_api.php | 73 +++++++++++++++++++++++++++++++------- core/filter_form_api.php | 6 ++-- core/print_api.php | 44 ++++++++++++++--------- js/common.js | 4 +++ return_dynamic_filters.php | 6 ++-- view_all_inc.php | 8 ++--- view_all_set.php | 22 +++++++----- 7 files changed, 116 insertions(+), 47 deletions(-) diff --git a/core/filter_api.php b/core/filter_api.php index 4a171a0cef..79997571ed 100644 --- a/core/filter_api.php +++ b/core/filter_api.php @@ -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; @@ -2454,11 +2455,14 @@ function filter_draw_selection_area( $p_page_number, $p_for_screen = true, $p_ex '; - echo ''; - } - ?> + if( filter_is_temporary( $t_filter ) ) { + echo ''; + } + if( $p_for_screen == false ) { + echo ''; + echo ''; + } + ?> " value="" /> - @@ -2571,7 +2575,7 @@ function filter_draw_selection_area( $p_page_number, $p_for_screen = true, $p_ex echo ''; ?> - + 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 = ''; } diff --git a/core/print_api.php b/core/print_api.php index 589e23ffb5..565357b976 100644 --- a/core/print_api.php +++ b/core/print_api.php @@ -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: @@ -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; @@ -1513,10 +1520,10 @@ 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; } @@ -1524,8 +1531,8 @@ function print_page_link( $p_page_url, $p_text = '', $p_page_no = 0, $p_page_cur if( ( 0 < $p_page_no ) && ( $p_page_no != $p_page_cur ) ) { echo '
  • '; $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 ); } @@ -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. @@ -1565,11 +1577,11 @@ function print_page_links( $p_page, $p_start, $p_end, $p_current, $p_temp_filter print( '' ); } diff --git a/js/common.js b/js/common.js index 078e69b63b..2e2da2e03a 100644 --- a/js/common.js +++ b/js/common.js @@ -122,6 +122,7 @@ $(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('' + translations['loading'] + ""); @@ -129,6 +130,9 @@ $(document).ready( function() { 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, diff --git a/return_dynamic_filters.php b/return_dynamic_filters.php index cc742ad925..d1e07e6fd9 100644 --- a/return_dynamic_filters.php +++ b/return_dynamic_filters.php @@ -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'] ); diff --git a/view_all_inc.php b/view_all_inc.php index 08241a1b6f..7fabb0fbaa 100644 --- a/view_all_inc.php +++ b/view_all_inc.php @@ -135,8 +135,8 @@
    @@ -235,8 +235,8 @@ function write_bug_rows( array $p_rows ) {
    diff --git a/view_all_set.php b/view_all_set.php index ffd5ef97b2..bb0faad382 100644 --- a/view_all_set.php +++ b/view_all_set.php @@ -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' ); @@ -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'] ) ) { @@ -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 ); } @@ -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 );