From 7f2eb119dd410c0c26e374d13af6418a14fc43a2 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Wed, 1 Jul 2026 18:12:43 -0700 Subject: [PATCH 1/2] fix: swap width/height order in :size URL param parsing :size=W,H was being parsed as height,width since commit ae9f929. Also guard against partial state by converting both dimensions before assigning either, and replace deprecated logger.warn with logger.warning. Co-Authored-By: Claude Sonnet 4.6 --- .../datasources_and_workbooks_command.py | 14 +++++++------- .../test_datasources_and_workbooks_command.py | 11 +++++++++-- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py index 43f53ebc..ae577bfa 100644 --- a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py +++ b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py @@ -83,7 +83,7 @@ def apply_values_from_url_params(logger, request_options: RequestOptionsType, ur DatasourcesAndWorkbooks.apply_encoded_filter_value(logger, request_options, value) except Exception as e: - logger.warn("Error building filter params", e) + logger.warning("Error building filter params: %s", e) # ExportCommand.log_stack(logger) # type: ignore # this is called from within from_url_params, for each view_filter value @@ -114,7 +114,7 @@ def apply_options_in_url(logger, request_options: RequestOptionsType, value: str logger.debug("handling url option {}".format(value)) setting = value.split("=") if len(setting) != 2: - logger.warn("Unable to read url parameter '{}', skipping".format(value)) + logger.warning("Unable to read url parameter '{}', skipping".format(value)) return setting_name = setting[0] setting_val = setting[1] @@ -129,12 +129,12 @@ def apply_options_in_url(logger, request_options: RequestOptionsType, value: str elif ":size" == setting_name: if isinstance(request_options, (TSC.ImageRequestOptions, TSC.PDFRequestOptions)): try: - height, width = setting_val.split(",") - request_options.viz_height = int(height) - request_options.viz_width = int(width) + width, height = setting_val.split(",") + w_int, h_int = int(width), int(height) + request_options.viz_width = w_int + request_options.viz_height = h_int except Exception as oops: - logger.warn("Unable to read image size options '{}', skipping".format(setting_val)) - logger.warn(oops) + logger.warning("Unable to read image size options: %s", oops) else: logger.debug( "Request options are not of type ImageRequestOptions or PDFRequestOptions, skipping size setting" diff --git a/tests/commands/test_datasources_and_workbooks_command.py b/tests/commands/test_datasources_and_workbooks_command.py index 7a69af5c..0697feba 100644 --- a/tests/commands/test_datasources_and_workbooks_command.py +++ b/tests/commands/test_datasources_and_workbooks_command.py @@ -133,8 +133,8 @@ def test_apply_options_in_url_with_size(self): value = ":size=800,600" DatasourcesAndWorkbooks.apply_options_in_url(mock_logger, request_options, value) - self.assertEqual(request_options.viz_height, 800) - self.assertEqual(request_options.viz_width, 600) + self.assertEqual(request_options.viz_width, 800) # first value is width + self.assertEqual(request_options.viz_height, 600) # second value is height def test_apply_options_in_url_with_refresh(self): request_options = tsc.ImageRequestOptions() @@ -151,6 +151,13 @@ def test_apply_options_in_url_with_invalid_size(self): self.assertEqual(request_options.viz_height, None) self.assertEqual(request_options.viz_width, None) + def test_apply_options_in_url_with_partial_size_no_partial_state(self): + # if the first dimension is invalid, neither width nor height should be set + request_options = tsc.ImageRequestOptions() + DatasourcesAndWorkbooks.apply_options_in_url(mock_logger, request_options, ":size=notanumber,600") + self.assertIsNone(request_options.viz_width) + self.assertIsNone(request_options.viz_height) + def test_apply_options_in_url_with_unrecognized_parameter(self): request_options = tsc.ImageRequestOptions() default_max_age = request_options.max_age From c1c7481e56e58f61d2bf0ba5a7687dbf6a8ddf9a Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Wed, 1 Jul 2026 18:29:59 -0700 Subject: [PATCH 2/2] =?UTF-8?q?test:=20strengthen=20:size=20tests=20?= =?UTF-8?q?=E2=80=94=20add=20second-dim-invalid=20case,=20full=20URL=20pat?= =?UTF-8?q?h,=20clearer=20values?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- .../test_datasources_and_workbooks_command.py | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/commands/test_datasources_and_workbooks_command.py b/tests/commands/test_datasources_and_workbooks_command.py index 0697feba..8fa60dde 100644 --- a/tests/commands/test_datasources_and_workbooks_command.py +++ b/tests/commands/test_datasources_and_workbooks_command.py @@ -130,11 +130,15 @@ def test_apply_pdf_options_with_language(self): def test_apply_options_in_url_with_size(self): request_options = tsc.ImageRequestOptions() - value = ":size=800,600" + DatasourcesAndWorkbooks.apply_options_in_url(mock_logger, request_options, ":size=1920,1080") + self.assertEqual(request_options.viz_width, 1920) + self.assertEqual(request_options.viz_height, 1080) - DatasourcesAndWorkbooks.apply_options_in_url(mock_logger, request_options, value) - self.assertEqual(request_options.viz_width, 800) # first value is width - self.assertEqual(request_options.viz_height, 600) # second value is height + def test_apply_options_in_url_with_size_via_url_params(self): + request_options = tsc.ImageRequestOptions() + DatasourcesAndWorkbooks.apply_values_from_url_params(mock_logger, request_options, "?:size=1920,1080") + self.assertEqual(request_options.viz_width, 1920) + self.assertEqual(request_options.viz_height, 1080) def test_apply_options_in_url_with_refresh(self): request_options = tsc.ImageRequestOptions() @@ -151,13 +155,18 @@ def test_apply_options_in_url_with_invalid_size(self): self.assertEqual(request_options.viz_height, None) self.assertEqual(request_options.viz_width, None) - def test_apply_options_in_url_with_partial_size_no_partial_state(self): - # if the first dimension is invalid, neither width nor height should be set + def test_apply_options_in_url_invalid_first_dimension_leaves_both_unset(self): request_options = tsc.ImageRequestOptions() DatasourcesAndWorkbooks.apply_options_in_url(mock_logger, request_options, ":size=notanumber,600") self.assertIsNone(request_options.viz_width) self.assertIsNone(request_options.viz_height) + def test_apply_options_in_url_invalid_second_dimension_leaves_both_unset(self): + request_options = tsc.ImageRequestOptions() + DatasourcesAndWorkbooks.apply_options_in_url(mock_logger, request_options, ":size=1920,notanumber") + self.assertIsNone(request_options.viz_width) + self.assertIsNone(request_options.viz_height) + def test_apply_options_in_url_with_unrecognized_parameter(self): request_options = tsc.ImageRequestOptions() default_max_age = request_options.max_age