From d3913c7cc9be93ef4779dc60df4aefd73934674f Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Thu, 9 Jan 2025 15:55:26 -0800 Subject: [PATCH 1/7] Fix: filters were being skipped for workbooks --- tabcmd/commands/datasources_and_workbooks/export_command.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tabcmd/commands/datasources_and_workbooks/export_command.py b/tabcmd/commands/datasources_and_workbooks/export_command.py index 5781c137..762bfba8 100644 --- a/tabcmd/commands/datasources_and_workbooks/export_command.py +++ b/tabcmd/commands/datasources_and_workbooks/export_command.py @@ -140,14 +140,12 @@ def apply_filters_from_args(request_options: TSC.PDFRequestOptions, args, logger for value in params: ExportCommand.apply_filter_value(logger, request_options, value) - # filtering isn't actually implemented for workbooks in REST @staticmethod def download_wb_pdf(server, workbook_item, args, logger): logger.debug(args.url) pdf_options = TSC.PDFRequestOptions(maxage=1) - if args.filter or args.url.find("?") > 0: - logger.info("warning: Filter values will not be applied when exporting a complete workbook") ExportCommand.apply_values_from_url_params(logger, pdf_options, args.url) + ExportCommand.apply_filters_from_args(pdf_options, args, logger) ExportCommand.apply_pdf_options(logger, pdf_options, args) logger.debug(pdf_options.get_query_params()) server.workbooks.populate_pdf(workbook_item, pdf_options) From 1180703b4b7a15d43de57be81107b6fd5faf3ef1 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Thu, 9 Jan 2025 16:03:23 -0800 Subject: [PATCH 2/7] Add test for wb filters This will verify that the command can run, but to confirm the filtering would need a visual check --- tests/e2e/online_tests.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/e2e/online_tests.py b/tests/e2e/online_tests.py index 78ecbe84..3eb08192 100644 --- a/tests/e2e/online_tests.py +++ b/tests/e2e/online_tests.py @@ -188,7 +188,7 @@ def _create_extract(self, type, wb_name): # variation: url def _refresh_extract(self, type, wb_name): command = "refreshextracts" - arguments = [command, wb_name, "--project", default_project_name, "-w",] # bug: should not need -w + arguments = [command, "-w", wb_name, "--project", default_project_name] # bug: should not need -w try: _test_command(arguments) except Exception as e: @@ -447,6 +447,15 @@ def test__delete_ds_live(self): name_on_server = OnlineCommandTest.TDS_FILE_LIVE_NAME self._delete_ds(name_on_server) + @pytest.mark.order(19) + def test_export_wb_filters(self): + wb_name_on_server = OnlineCommandTest.TWBX_WITH_EXTRACT_NAME + sheet_name = OnlineCommandTest.TWBX_WITH_EXTRACT_SHEET + friendly_name = wb_name_on_server +"/" + sheet_name + filters = ["--filter", "Product Type=Tea", "--fullpdf", "--pagelayout", "landscape"] + self._export_wb(friendly_name, "filter_a_wb_to_tea_and_two_pages.pdf", filters) + # NOTE: this test needs a visual check on the returned pdf to confirm the expected appearance + @pytest.mark.order(19) def test_export_wb_pdf(self): command = "export" From d293386cd02bb2c4a215d29e7498078d999b32b6 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Thu, 9 Jan 2025 16:20:22 -0800 Subject: [PATCH 3/7] Enable setting height, width for image export This now supports the get command with url params ?:size=x,y and the export command with options --height, --weight --- .../datasources_and_workbooks_command.py | 32 +++++++++---- .../get_url_command.py | 6 +-- tests/e2e/online_tests.py | 45 ++++++++++--------- 3 files changed, 52 insertions(+), 31 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 5a9cfe17..13bab1a0 100644 --- a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py +++ b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py @@ -106,18 +106,31 @@ def apply_filter_value(logger, request_options: TSC.PDFRequestOptions, value: st # this is called from within from_url_params, for each param value @staticmethod - def apply_options_in_url(logger, request_options: TSC.PDFRequestOptions, value: str) -> None: + def apply_options_in_url(logger, request_options: TSC.ImageRequestOptions, value: str) -> None: logger.debug("handling url option {}".format(value)) setting = value.split("=") - if ":iid" == setting[0]: + if len(setting) != 2: + logger.warn("Unable to read url parameter '{}', skipping".format(value)) + setting_name = setting[0] + setting_val = setting[1] + logger.debug("setting named {}, has value {}".format(setting_name, setting_val)) + + # TODO: if the setting value ends with a filetype, read that and strip it + if ":iid" == setting_name: logger.debug(":iid value ignored in url") - elif ":refresh" == setting[0] and DatasourcesAndWorkbooks.is_truthy(setting[1]): + elif ":refresh" == setting_name and DatasourcesAndWorkbooks.is_truthy(setting_val): # mypy is worried that this is readonly request_options.max_age = 0 # type:ignore logger.debug("Set max age to {} from {}".format(request_options.max_age, value)) - elif ":size" == setting[0]: - height, width = setting[1].split(",") - logger.warn("Height/width parameters not yet implemented ({})".format(value)) + elif ":size" == setting_name: + # this is only used by get as png + try: + height, width = setting_val.split(",") + request_options.viz_height = int(height) + request_options.viz_width = int(width) + except Exception as oops: + logger.warn("Unable to read image size options '{}', skipping".format(setting_val)) + logger.warn(oops) else: logger.debug("Parameter[s] not recognized: {}".format(value)) @@ -127,8 +140,11 @@ def is_truthy(value: str): @staticmethod def apply_png_options(logger, request_options: TSC.ImageRequestOptions, args): - if args.height or args.width: - logger.warn("Height/width arguments not yet implemented in export") + # these are only used in export, not get + if args.height: + request_options.viz_height = int(args.height) + if args.width: + request_options.viz_width = args.width # Always request high-res images request_options.image_resolution = "high" diff --git a/tabcmd/commands/datasources_and_workbooks/get_url_command.py b/tabcmd/commands/datasources_and_workbooks/get_url_command.py index 05e52a8f..1eb0b3ee 100644 --- a/tabcmd/commands/datasources_and_workbooks/get_url_command.py +++ b/tabcmd/commands/datasources_and_workbooks/get_url_command.py @@ -115,9 +115,9 @@ def generate_png(logger, server_content_type, args, get_url_item): logger.trace("Entered method " + inspect.stack()[0].function) try: logger.debug(_("content_type.view") + ": {}".format(get_url_item.name)) - req_option_csv = TSC.ImageRequestOptions(maxage=1) - DatasourcesAndWorkbooks.apply_values_from_url_params(logger, req_option_csv, args.url) - server_content_type.populate_image(get_url_item, req_option_csv) + req_option_png = TSC.ImageRequestOptions(maxage=1) + DatasourcesAndWorkbooks.apply_values_from_url_params(logger, req_option_png, args.url) + server_content_type.populate_image(get_url_item, req_option_png) filename = GetUrl.filename_from_args(args.filename, get_url_item.name, "png") DatasourcesAndWorkbooks.save_to_file(logger, get_url_item.image, filename) except Exception as e: diff --git a/tests/e2e/online_tests.py b/tests/e2e/online_tests.py index 3eb08192..9abb4661 100644 --- a/tests/e2e/online_tests.py +++ b/tests/e2e/online_tests.py @@ -28,7 +28,7 @@ unique = str(time.gmtime().tm_sec) group_name = "test-ing-group" + unique workbook_name = "wb_1_" + unique -default_project_name = "Personal Work" # "default-proj" + unique +default_project_name = "Personal Work" # "default-proj" + unique parent_location = "parent" + unique project_name = "test-proj-" + unique @@ -43,7 +43,7 @@ def _test_command(test_args: list[str]): # this will raise an exception if it gets a non-zero return code # that will bubble up and fail the test - + # default: run tests using tabcmd 2 calling_args = ["python", "-m", "tabcmd"] + test_args + [debug_log] + ["--no-certcheck"] @@ -120,7 +120,7 @@ def _publish_creds_args( def _delete_wb(self, name): command = "delete" - arguments = [command, "--project", default_project_name, name] + arguments = [command, "--project", default_project_name, name] _test_command(arguments) def _delete_ds(self, name): @@ -142,20 +142,17 @@ def _get_custom_view(self): # TODO command = "get" - - def _export_wb(self, friendly_name, filename=None, additional_args=None): command = "export" arguments = [command, friendly_name, "--fullpdf"] - + if filename: arguments = arguments + ["--filename", filename] if additional_args: arguments = arguments + additional_args _test_command(arguments) - - def _export_view(self, wb_name_on_server, sheet_name, export_type, filename=None, additional_args=None): + def _export_view(self, wb_name_on_server, sheet_name, export_type, filename=None, additional_args=None): server_file = "/" + wb_name_on_server + "/" + sheet_name command = "export" arguments = [command, server_file, export_type] @@ -180,7 +177,7 @@ def _get_datasource(self, server_file): def _create_extract(self, type, wb_name): command = "createextracts" - arguments = [command, type, wb_name, "--project", default_project_name] + arguments = [command, type, wb_name, "--project", default_project_name] if extract_encryption_enabled and not use_tabcmd_classic: arguments.append("--encrypt") _test_command(arguments) @@ -188,7 +185,7 @@ def _create_extract(self, type, wb_name): # variation: url def _refresh_extract(self, type, wb_name): command = "refreshextracts" - arguments = [command, "-w", wb_name, "--project", default_project_name] # bug: should not need -w + arguments = [command, "-w", wb_name, "--project", default_project_name] # bug: should not need -w try: _test_command(arguments) except Exception as e: @@ -202,7 +199,7 @@ def _refresh_extract(self, type, wb_name): def _delete_extract(self, type, item_name): command = "deleteextracts" - arguments = [command, type, item_name, "--include-all", "--project", default_project_name] + arguments = [command, type, item_name, "--include-all", "--project", default_project_name] try: _test_command(arguments) except Exception as e: @@ -379,6 +376,17 @@ def test_view_get_pdf(self): # bug in tabcmd classic: doesn't work without download name self._get_view(wb_name_on_server, sheet_name, "downloaded_file.pdf") + @pytest.mark.order(11) + def test_view_get_png_sizes(self): + wb_name_on_server = OnlineCommandTest.TWBX_WITH_EXTRACT_NAME + sheet_name = OnlineCommandTest.TWBX_WITH_EXTRACT_SHEET + + self._get_view(wb_name_on_server, sheet_name, "get_view_default_size.png") + url_params = "?:size=100,200" + self._get_view(wb_name_on_server, sheet_name + url_params, "get_view_sized_sm.png") + url_params = "?:size=500,700" + self._get_view(wb_name_on_server, sheet_name + url_params, "get_view_sized_LARGE.png") + @pytest.mark.order(11) def test_view_get_csv(self): wb_name_on_server = OnlineCommandTest.TWBX_WITH_EXTRACT_NAME @@ -451,21 +459,18 @@ def test__delete_ds_live(self): def test_export_wb_filters(self): wb_name_on_server = OnlineCommandTest.TWBX_WITH_EXTRACT_NAME sheet_name = OnlineCommandTest.TWBX_WITH_EXTRACT_SHEET - friendly_name = wb_name_on_server +"/" + sheet_name - filters = ["--filter", "Product Type=Tea", "--fullpdf", "--pagelayout", "landscape"] + friendly_name = wb_name_on_server + "/" + sheet_name + filters = ["--filter", "Product Type=Tea", "--fullpdf", "--pagelayout", "landscape"] self._export_wb(friendly_name, "filter_a_wb_to_tea_and_two_pages.pdf", filters) # NOTE: this test needs a visual check on the returned pdf to confirm the expected appearance @pytest.mark.order(19) def test_export_wb_pdf(self): - command = "export" wb_name_on_server = OnlineCommandTest.TWBX_WITH_EXTRACT_NAME - friendly_name = ( - wb_name_on_server + "/" + OnlineCommandTest.TWBX_WITH_EXTRACT_SHEET - ) + friendly_name = wb_name_on_server + "/" + OnlineCommandTest.TWBX_WITH_EXTRACT_SHEET filename = "exported_wb.pdf" self._export_wb(friendly_name, filename) - + @pytest.mark.order(19) def test_export_data_csv(self): wb_name_on_server = OnlineCommandTest.TWBX_WITH_EXTRACT_NAME @@ -483,13 +488,13 @@ def test_export_view_pdf(self): wb_name_on_server = OnlineCommandTest.TWBX_WITH_EXTRACT_NAME sheet_name = OnlineCommandTest.TWBX_WITH_EXTRACT_SHEET self._export_view(wb_name_on_server, sheet_name, "--pdf", "export_view_pdf.pdf") - + @pytest.mark.order(19) def test_export_view_filtered(self): wb_name_on_server = OnlineCommandTest.TWBX_WITH_EXTRACT_NAME sheet_name = OnlineCommandTest.TWBX_WITH_EXTRACT_SHEET filename = "view_with_filters.pdf" - + filters = ["--filter", "Product Type=Tea"] self._export_view(wb_name_on_server, sheet_name, "--pdf", filename, filters) From 23e06ce2eca0d06bd5c7ce5673d0479a22730fde Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Thu, 9 Jan 2025 22:16:23 -0800 Subject: [PATCH 4/7] add test to cover the int requirement Stephen caught --- .../datasources_and_workbooks_command.py | 2 +- .../test_datasources_and_workbooks_command.py | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 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 13bab1a0..022a5be5 100644 --- a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py +++ b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py @@ -144,7 +144,7 @@ def apply_png_options(logger, request_options: TSC.ImageRequestOptions, args): if args.height: request_options.viz_height = int(args.height) if args.width: - request_options.viz_width = args.width + request_options.viz_width = int(args.width) # Always request high-res images request_options.image_resolution = "high" diff --git a/tests/commands/test_datasources_and_workbooks_command.py b/tests/commands/test_datasources_and_workbooks_command.py index 446822dc..2d4db913 100644 --- a/tests/commands/test_datasources_and_workbooks_command.py +++ b/tests/commands/test_datasources_and_workbooks_command.py @@ -47,12 +47,20 @@ def test_apply_options_from_url_params(self): assert request_options.max_age == 0 def test_apply_png_options(self): - # these aren't implemented yet. the layout and orientation ones don't apply. - mock_args.width = 800 - mock_args.height = 76 + mock_args.width = "800" + mock_args.height = "76" request_options = tsc.ImageRequestOptions() DatasourcesAndWorkbooks.apply_png_options(mock_logger, request_options, mock_args) assert request_options.image_resolution == "high" + assert request_options.viz_width == 800 + assert request_options.viz_height == 76 + + def test_apply_png_options_bad_values(self): + mock_args.height = "seven" + mock_args.width = "800b" + request_options = tsc.ImageRequestOptions() + with self.assertRaises(ValueError): + DatasourcesAndWorkbooks.apply_png_options(mock_logger, request_options, mock_args) def test_apply_pdf_options(self): expected_page = tsc.PDFRequestOptions.PageType.Folio.__str__() From f680997117581466aa7ef95d2b8ba5750f474279 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Thu, 9 Jan 2025 22:22:44 -0800 Subject: [PATCH 5/7] Update datasources_and_workbooks_command.py --- .../datasources_and_workbooks_command.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 022a5be5..66fbcf63 100644 --- a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py +++ b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py @@ -62,7 +62,7 @@ def get_ds_by_content_url(logger, server, datasource_content_url) -> TSC.Datasou return matching_datasources[0] @staticmethod - def apply_values_from_url_params(logger, request_options: TSC.PDFRequestOptions, url) -> None: + def apply_values_from_url_params(logger, request_options: TSC.RequestOptions, url) -> None: logger.debug(url) try: if "?" in url: From 400fa4bc9e21d926d73f45911b1f7181afc04bb3 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Thu, 9 Jan 2025 22:26:07 -0800 Subject: [PATCH 6/7] mypy --- .../datasources_and_workbooks_command.py | 17 +++++++++-------- .../datasources_and_workbooks/export_command.py | 2 +- 2 files changed, 10 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 66fbcf63..feef5f33 100644 --- a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py +++ b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py @@ -99,14 +99,16 @@ def apply_encoded_filter_value(logger, request_options, value): # from apply_options, which expects an un-encoded input, # or from apply_url_params via apply_encoded_filter_value which decodes the input @staticmethod - def apply_filter_value(logger, request_options: TSC.PDFRequestOptions, value: str) -> None: + def apply_filter_value(logger, request_options: TSC.RequestOptions, value: str) -> None: logger.debug("handling filter param {}".format(value)) data_filter = value.split("=") - request_options.vf(data_filter[0], data_filter[1]) + # we should export the _DataExportOptions class from tsc + request_options.vf(data_filter[0], data_filter[1]) # type: ignore # this is called from within from_url_params, for each param value + # expects either ImageRequestOptions or PDFRequestOptions @staticmethod - def apply_options_in_url(logger, request_options: TSC.ImageRequestOptions, value: str) -> None: + def apply_options_in_url(logger, request_options: TSC.RequestOptions, value: str) -> None: logger.debug("handling url option {}".format(value)) setting = value.split("=") if len(setting) != 2: @@ -115,19 +117,18 @@ def apply_options_in_url(logger, request_options: TSC.ImageRequestOptions, value setting_val = setting[1] logger.debug("setting named {}, has value {}".format(setting_name, setting_val)) - # TODO: if the setting value ends with a filetype, read that and strip it if ":iid" == setting_name: logger.debug(":iid value ignored in url") elif ":refresh" == setting_name and DatasourcesAndWorkbooks.is_truthy(setting_val): # mypy is worried that this is readonly - request_options.max_age = 0 # type:ignore - logger.debug("Set max age to {} from {}".format(request_options.max_age, value)) + request_options.max_age = 0 # type: ignore + logger.debug("Set max age to {} from {}".format((TSC.ImageRequestOptions(request_options)).max_age, value)) elif ":size" == setting_name: # this is only used by get as png try: height, width = setting_val.split(",") - request_options.viz_height = int(height) - request_options.viz_width = int(width) + (TSC.ImageRequestOptions(request_options)).viz_height = int(height) + (TSC.ImageRequestOptions(request_options)).viz_width = int(width) except Exception as oops: logger.warn("Unable to read image size options '{}', skipping".format(setting_val)) logger.warn(oops) diff --git a/tabcmd/commands/datasources_and_workbooks/export_command.py b/tabcmd/commands/datasources_and_workbooks/export_command.py index 762bfba8..69999288 100644 --- a/tabcmd/commands/datasources_and_workbooks/export_command.py +++ b/tabcmd/commands/datasources_and_workbooks/export_command.py @@ -133,7 +133,7 @@ def run_command(args): Errors.exit_with_error(logger, "Error saving to file", e) @staticmethod - def apply_filters_from_args(request_options: TSC.PDFRequestOptions, args, logger=None) -> None: + def apply_filters_from_args(request_options: TSC.RequestOptions, args, logger=None) -> None: if args.filter: logger.debug("filter = {}".format(args.filter)) params = args.filter.split("&") From fe9fe6a7d2b351b36fa693ed4472c09a64342862 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Thu, 9 Jan 2025 23:44:13 -0800 Subject: [PATCH 7/7] reformat --- tests/commands/test_datasources_and_workbooks_command.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/commands/test_datasources_and_workbooks_command.py b/tests/commands/test_datasources_and_workbooks_command.py index 2d4db913..6556d0f6 100644 --- a/tests/commands/test_datasources_and_workbooks_command.py +++ b/tests/commands/test_datasources_and_workbooks_command.py @@ -54,7 +54,7 @@ def test_apply_png_options(self): assert request_options.image_resolution == "high" assert request_options.viz_width == 800 assert request_options.viz_height == 76 - + def test_apply_png_options_bad_values(self): mock_args.height = "seven" mock_args.width = "800b"