This is an automated email from the ASF dual-hosted git repository. ephraimanierobi pushed a commit to branch v2-5-test in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 521fb2c090fc0d26509d66959dbac385b284d3f6 Author: Amogh Desai <[email protected]> AuthorDate: Thu Mar 2 20:29:43 2023 +0530 DAG list sorting lost when switching page (#29756) * Adding sorting keys to generate_pages * Handling review comments and using the new keys * Fixing Brent review comments * Adding docstring --------- Co-authored-by: Amogh <[email protected]> (cherry picked from commit c8cd90fa92c1597300dbbad4366c2bef49ef6390) --- airflow/www/utils.py | 70 ++++++++++++++++++++++++++++++++++++++----------- airflow/www/views.py | 5 ++-- tests/www/test_utils.py | 50 +++++++++++++++++++++++++++++++++-- 3 files changed, 106 insertions(+), 19 deletions(-) diff --git a/airflow/www/utils.py b/airflow/www/utils.py index 5381709e35..bd5b84cb5e 100644 --- a/airflow/www/utils.py +++ b/airflow/www/utils.py @@ -211,7 +211,16 @@ def get_params(**kwargs): return urlencode({d: v for d, v in kwargs.items() if v is not None}, True) -def generate_pages(current_page, num_of_pages, search=None, status=None, tags=None, window=7): +def generate_pages( + current_page, + num_of_pages, + search=None, + status=None, + tags=None, + window=7, + sorting_key=None, + sorting_direction=None, +): """ Generates the HTML for a paging component using a similar logic to the paging auto-generated by Flask managed views. The paging component defines a number of @@ -230,6 +239,9 @@ def generate_pages(current_page, num_of_pages, search=None, status=None, tags=No :param status: 'all', 'active', or 'paused' :param tags: array of strings of the current filtered tags :param window: the number of pages to be shown in the paging component (7 default) + :param sorting_key: the sorting key selected for dags, None indicates that sorting is not needed/provided + :param sorting_direction: direction of sorting, 'asc' or 'desc', + None indicates that sorting is not needed/provided :return: the HTML string of the paging component """ void_link = "javascript:void(0)" @@ -267,9 +279,15 @@ def generate_pages(current_page, num_of_pages, search=None, status=None, tags=No is_disabled = "disabled" if current_page <= 0 else "" - first_node_link = ( - void_link if is_disabled else f"?{get_params(page=0, search=search, status=status, tags=tags)}" + qs = get_params( + page=0, + search=search, + status=status, + tags=tags, + sorting_key=sorting_key, + sorting_direction=sorting_direction, ) + first_node_link = void_link if is_disabled else f"?{qs}" output.append( first_node.format( href_link=first_node_link, @@ -279,7 +297,15 @@ def generate_pages(current_page, num_of_pages, search=None, status=None, tags=No page_link = void_link if current_page > 0: - page_link = f"?{get_params(page=current_page - 1, search=search, status=status, tags=tags)}" + qs = get_params( + page=current_page - 1, + search=search, + status=status, + tags=tags, + sorting_key=sorting_key, + sorting_direction=sorting_direction, + ) + page_link = f"?{qs}" output.append(previous_node.format(href_link=page_link, disabled=is_disabled)) @@ -297,30 +323,44 @@ def generate_pages(current_page, num_of_pages, search=None, status=None, tags=No return page == current for page in pages: + qs = get_params( + page=page, + search=search, + status=status, + tags=tags, + sorting_key=sorting_key, + sorting_direction=sorting_direction, + ) vals = { "is_active": "active" if is_current(current_page, page) else "", - "href_link": void_link - if is_current(current_page, page) - else f"?{get_params(page=page, search=search, status=status, tags=tags)}", + "href_link": void_link if is_current(current_page, page) else f"?{qs}", "page_num": page + 1, } output.append(page_node.format(**vals)) is_disabled = "disabled" if current_page >= num_of_pages - 1 else "" - page_link = ( - void_link - if current_page >= num_of_pages - 1 - else f"?{get_params(page=current_page + 1, search=search, status=status, tags=tags)}" + qs = get_params( + page=current_page + 1, + search=search, + status=status, + tags=tags, + sorting_key=sorting_key, + sorting_direction=sorting_direction, ) + page_link = void_link if current_page >= num_of_pages - 1 else f"?{qs}" output.append(next_node.format(href_link=page_link, disabled=is_disabled)) - last_node_link = ( - void_link - if is_disabled - else f"?{get_params(page=last_page, search=search, status=status, tags=tags)}" + qs = get_params( + page=last_page, + search=search, + status=status, + tags=tags, + sorting_key=sorting_key, + sorting_direction=sorting_direction, ) + last_node_link = void_link if is_disabled else f"?{qs}" output.append( last_node.format( href_link=last_node_link, diff --git a/airflow/www/views.py b/airflow/www/views.py index 15ab7fc327..880d447699 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -827,6 +827,8 @@ class Airflow(AirflowBaseView): search=escape(arg_search_query) if arg_search_query else None, status=arg_status_filter if arg_status_filter else None, tags=arg_tags_filter if arg_tags_filter else None, + sorting_key=arg_sorting_key if arg_sorting_key else None, + sorting_direction=arg_sorting_direction if arg_sorting_direction else None, ), num_runs=num_runs, tags=tags, @@ -3784,8 +3786,7 @@ class Airflow(AirflowBaseView): audit_logs_count=audit_logs_count, page_size=PAGE_SIZE, paging=wwwutils.generate_pages( - current_page, - num_of_pages, + current_page, num_of_pages, arg_sorting_key, arg_sorting_direction ), sorting_key=arg_sorting_key, sorting_direction=arg_sorting_direction, diff --git a/tests/www/test_utils.py b/tests/www/test_utils.py index 33b0cc6487..c6f48ee360 100644 --- a/tests/www/test_utils.py +++ b/tests/www/test_utils.py @@ -28,10 +28,27 @@ from airflow.www.utils import wrapped_markdown class TestUtils: - def check_generate_pages_html(self, current_page, total_pages, window=7, check_middle=False): + def check_generate_pages_html( + self, + current_page, + total_pages, + window=7, + check_middle=False, + sorting_key=None, + sorting_direction=None, + ): extra_links = 4 # first, prev, next, last search = "'>\"/><img src=x onerror=alert(1)>" - html_str = utils.generate_pages(current_page, total_pages, search=search) + if sorting_key and sorting_direction: + html_str = utils.generate_pages( + current_page, + total_pages, + search=search, + sorting_key=sorting_key, + sorting_direction=sorting_direction, + ) + else: + html_str = utils.generate_pages(current_page, total_pages, search=search) assert search not in html_str, "The raw search string shouldn't appear in the output" assert "search=%27%3E%22%2F%3E%3Cimg+src%3Dx+onerror%3Dalert%281%29%3E" in html_str @@ -47,10 +64,27 @@ class TestUtils: page_items = ulist_items[2:-2] mid = int(len(page_items) / 2) + all_nodes = [] + pages = [] + + if sorting_key and sorting_direction: + last_page = total_pages - 1 + + if current_page <= mid or total_pages < window: + pages = list(range(0, min(total_pages, window))) + elif mid < current_page < last_page - mid: + pages = list(range(current_page - mid, current_page + mid + 1)) + else: + pages = list(range(total_pages - window, last_page + 1)) + + pages.append(last_page + 1) + pages.sort(reverse=True if sorting_direction == "desc" else False) + for i, item in enumerate(page_items): a_node = item.a href_link = a_node["href"] node_text = a_node.string + all_nodes.append(node_text) if node_text == str(current_page + 1): if check_middle: assert mid == i @@ -62,6 +96,13 @@ class TestUtils: assert query["page"] == [str(int(node_text) - 1)] assert query["search"] == [search] + if sorting_key and sorting_direction: + if pages[0] == 0: + pages = pages[1:] + pages = list(map(lambda x: str(x), pages)) + + assert pages == all_nodes + def test_generate_pager_current_start(self): self.check_generate_pages_html(current_page=0, total_pages=6) @@ -71,6 +112,11 @@ class TestUtils: def test_generate_pager_current_end(self): self.check_generate_pages_html(current_page=38, total_pages=39) + def test_generate_pager_current_start_with_sorting(self): + self.check_generate_pages_html( + current_page=0, total_pages=4, sorting_key="dag_id", sorting_direction="asc" + ) + def test_params_no_values(self): """Should return an empty string if no params are passed""" assert "" == utils.get_params()
