You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by ep...@apache.org on 2023/03/07 16:16:19 UTC
[airflow] 12/23: DAG list sorting lost when switching page (#29756)
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 <am...@gmail.com>
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 <ad...@cloudera.com>
(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()