feat(hitl): update url generating utility

providers/standard/src/airflow/providers/standard/operators/hitl.py

@@ -98,14 +98,10 @@ def validate_options(self) -> None:
 
         Raises:
             ValueError: If `options` is empty.
-            ValueError: If any option contains a comma (`,`), which is not allowed.
         """
         if not self.options:
             raise ValueError('"options" cannot be empty.')
 
-        if any("," in option for option in self.options):
-            raise ValueError('"," is not allowed in option')
-
     def validate_params(self) -> None:
         """
         Validate the `params` attribute of the instance.
@@ -235,7 +231,7 @@ def generate_link_to_ui(
         if options:
             if diff := set(options) - set(self.options):
                 raise ValueError(f"options {diff} are not valid options")
-            query_param["_options"] = ",".join(options)
+            query_param["_options"] = options
 
         if params_input:
             if diff := set(params_input.keys()) - set(self.params.keys()):

providers/standard/tests/unit/standard/operators/test_hitl.py

@@ -26,6 +26,7 @@
 import datetime
 from typing import TYPE_CHECKING, Any
 from unittest.mock import MagicMock
+from urllib.parse import parse_qs, urlparse
 
 import pytest
 from sqlalchemy import select
@@ -86,21 +87,13 @@ def test_validate_options(self) -> None:
         )
         hitl_op.validate_options()
 
-    @pytest.mark.parametrize(
-        "options, expected_err_msg",
-        [
-            ([], '"options" cannot be empty.'),
-            (["1,1", "1", "2"], '"," is not allowed in option'),
-        ],
-        ids=["empty", "comma"],
-    )
-    def test_validate_options_with_empty_options(self, options: list[str], expected_err_msg: str) -> None:
+    def test_validate_options_with_empty_options(self) -> None:
         # validate_options is called during initialization
-        with pytest.raises(ValueError, match=expected_err_msg):
+        with pytest.raises(ValueError, match='"options" cannot be empty.'):
             HITLOperator(
                 task_id="hitl_test",
                 subject="This is subject",
-                options=options,
+                options=[],
                 body="This is body",
                 defaults=["1"],
                 multiple=False,
@@ -281,16 +274,21 @@ def test_validate_params_input_with_invalid_input(self) -> None:
             )
 
     @pytest.mark.parametrize(
-        "options, params_input, expected_query_string",
+        "options, params_input, expected_parsed_query",
         [
-            (None, None, "?map_index=-1"),
-            ("1", None, "?_options=1&map_index=-1"),
-            (["1", "2"], None, "?_options=1%2C2&map_index=-1"),
-            (None, {"input_1": 123}, "?input_1=123&map_index=-1"),
+            (None, None, {"map_index": ["-1"]}),
+            ("1", None, {"_options": ["['1']"], "map_index": ["-1"]}),
+            (["1", "2"], None, {"_options": ["['1', '2']"], "map_index": ["-1"]}),
+            (None, {"input_1": "123"}, {"input_1": ["123"], "map_index": ["-1"]}),
             (
                 ["3", "4", "5"],
-                {"input_1": 123123, "input_2": 345345},
-                "?_options=3%2C4%2C5&input_1=123123&input_2=345345&map_index=-1",
+                {"input_1": "123123", "input_2": "345345"},
+                {
+                    "_options": ["['3', '4', '5']"],
+                    "input_1": ["123123"],
+                    "input_2": ["345345"],
+                    "map_index": ["-1"],
+                },
             ),
         ],
         ids=[
@@ -301,65 +299,43 @@ def test_validate_params_input_with_invalid_input(self) -> None:
             "multiple-options-and-param-inputs",
         ],
     )
-    @pytest.mark.parametrize("base_url", ["http://test", "http://test_2:8080"])
-    def test_generate_link_to_ui(
-        self,
-        base_url: str,
-        options: list[str] | None,
-        params_input: dict[str, Any] | None,
-        expected_query_string: str,
-        hitl_task_and_ti_for_generating_link: tuple[HITLOperator, TaskInstance],
-    ) -> None:
-        expected_url = (
-            f"{base_url}/dags/test_dag/runs/test/tasks/hitl_test/required_actions{expected_query_string}"
-        )
-        task, ti = hitl_task_and_ti_for_generating_link
-        url = task.generate_link_to_ui(
-            task_instance=ti,
-            base_url=base_url,
-            options=options,
-            params_input=params_input,
-        )
-        assert url == expected_url
-
     @pytest.mark.parametrize(
-        "options, params_input, expected_query_string",
-        [
-            (None, None, "?map_index=-1"),
-            ("1", None, "?_options=1&map_index=-1"),
-            (["1", "2"], None, "?_options=1%2C2&map_index=-1"),
-            (None, {"input_1": 123}, "?input_1=123&map_index=-1"),
-            (
-                ["3", "4", "5"],
-                {"input_1": 123123, "input_2": 345345},
-                "?_options=3%2C4%2C5&input_1=123123&input_2=345345&map_index=-1",
-            ),
-        ],
-        ids=[
-            "empty",
-            "single-option",
-            "multiple-options",
-            "single-param-input",
-            "multiple-options-and-param-inputs",
-        ],
+        "conf_base_url",
+        [None, "http://localhost:8080/"],
+        ids=["no_conf_url", "with_conf_url"],
+    )
+    @pytest.mark.parametrize(
+        "base_url",
+        ["http://test", "http://test_2:8080"],
+        ids=["url_1", "url_2"],
     )
-    @conf_vars({("api", "base_url"): "http://localhost:8080/"})
-    def test_generate_link_fall_back_to_conf_api_base_url(
+    def test_generate_link_to_ui(
         self,
+        base_url: str,
+        conf_base_url: str,
         options: list[str] | None,
         params_input: dict[str, Any] | None,
-        expected_query_string: str,
+        expected_parsed_query: dict[str, list[str]],
         hitl_task_and_ti_for_generating_link: tuple[HITLOperator, TaskInstance],
     ) -> None:
-        task, ti = hitl_task_and_ti_for_generating_link
-        expected_url = f"http://localhost:8080/dags/test_dag/runs/test/tasks/hitl_test/required_actions{expected_query_string}"
+        with conf_vars({("api", "base_url"): conf_base_url}):
+            if conf_base_url:
+                base_url = conf_base_url
+            task, ti = hitl_task_and_ti_for_generating_link
+            url = task.generate_link_to_ui(
+                task_instance=ti,
+                base_url=base_url,
+                options=options,
+                params_input=params_input,
+            )
 
-        url = task.generate_link_to_ui(
-            task_instance=ti,
-            options=options,
-            params_input=params_input,
-        )
-        assert url == expected_url
+            base_url_parsed_result = urlparse(base_url)
+            parse_result = urlparse(url)
+            assert parse_result.scheme == base_url_parsed_result.scheme
+            assert parse_result.netloc == base_url_parsed_result.netloc
+            assert parse_result.path == "/dags/test_dag/runs/test/tasks/hitl_test/required_actions"
+            assert parse_result.params == ""
+            assert parse_qs(parse_result.query) == expected_parsed_query
 
     @pytest.mark.parametrize(
         "options, params_input, expected_err_msg",