From 08da08b1094b11d0af93e30c6e491030fb83fdda Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Sat, 28 Jan 2023 17:31:12 -0800 Subject: [PATCH 1/3] encoding filter values Write tests for methods in ds_and_wb class, including a failing test that expects the view_filter to be un-encoded --- .../datasources_and_workbooks_command.py | 1 + .../test_datasources_and_workbooks_command.py | 94 +++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 tests/commands/test_datasources_and_workbooks_command.py 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 1d345a12..3fb6ac6a 100644 --- a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py +++ b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py @@ -1,4 +1,5 @@ import tableauserverclient as TSC +import urllib3 from tabcmd.commands.constants import Errors from tabcmd.commands.server import Server diff --git a/tests/commands/test_datasources_and_workbooks_command.py b/tests/commands/test_datasources_and_workbooks_command.py new file mode 100644 index 00000000..3454f365 --- /dev/null +++ b/tests/commands/test_datasources_and_workbooks_command.py @@ -0,0 +1,94 @@ +import argparse +from unittest.mock import MagicMock + +from tabcmd.commands.datasources_and_workbooks.datasources_and_workbooks_command import DatasourcesAndWorkbooks +import tableauserverclient as tsc +import unittest +from unittest import mock + +mock_logger = mock.MagicMock() + +fake_item = mock.MagicMock() +fake_item.name = "fake-name" +fake_item.id = "fake-id" + +getter = MagicMock() +getter.get = MagicMock("get", return_value=([fake_item], 1)) + +mock_args = argparse.Namespace() + + +class ParameterTests(unittest.TestCase): + def test_get_view_url_from_names(self): + wb_name = "WB" + view_name = "VIEW" + out_value = DatasourcesAndWorkbooks.get_view_url_from_names(wb_name, view_name) + assert out_value == "{}/sheets/{}".format(wb_name, view_name) + + def test_apply_filters_from_url_params(self): + query_params = "?Product=widget" + expected = [("Product", "widget")] + request_options = tsc.PDFRequestOptions() + DatasourcesAndWorkbooks.apply_values_from_url_params(request_options, query_params, mock_logger) + assert request_options.view_filters == expected + + def test_apply_encoded_filters_from_url_params(self): + query_params = "?Product%20type=Z%C3%BCrich" + expected = [("Product type", "Zürich")] + request_options = tsc.PDFRequestOptions() + DatasourcesAndWorkbooks.apply_values_from_url_params(request_options, query_params, mock_logger) + assert request_options.view_filters == expected + + def test_apply_options_from_url_params(self): + query_params = "?:iid=5&:refresh=yes&:size=600,700" + request_options = tsc.PDFRequestOptions() + DatasourcesAndWorkbooks.apply_values_from_url_params(request_options, query_params, mock_logger) + 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 + request_options = tsc.ImageRequestOptions() + DatasourcesAndWorkbooks.apply_png_options(request_options, mock_args, mock_logger) + assert request_options.image_resolution == "high" + + def test_apply_pdf_options(self): + expected_page = tsc.PDFRequestOptions.PageType.Folio.__str__() + expected_layout = tsc.PDFRequestOptions.Orientation.Portrait.__str__() + mock_args.pagelayout = expected_layout + mock_args.pagesize = expected_page + request_options = tsc.PDFRequestOptions() + DatasourcesAndWorkbooks.apply_pdf_options(request_options, mock_args, mock_logger) + params = request_options.get_query_params() + assert request_options.page_type == expected_page + assert request_options.orientation == expected_layout + + +@mock.patch("tableauserverclient.Server") +class MockedServerTests(unittest.TestCase): + def test_mock_getter(self, mock_server): + mock_server.fakes = getter + mock_server.fakes.get() + getter.get.assert_called() + + def test_get_ds_by_content_url(self, mock_server): + mock_server.datasources = getter + content_url = "blah" + DatasourcesAndWorkbooks.get_ds_by_content_url(mock_logger, mock_server, content_url) + getter.get.assert_called() + # should also assert the filter on content url + + def test_get_wb_by_content_url(self, mock_server): + mock_server.workbooks = getter + content_url = "blah" + DatasourcesAndWorkbooks.get_wb_by_content_url(mock_logger, mock_server, content_url) + getter.get.assert_called() + # should also assert the filter on content url + + def test_get_view_by_content_url(self, mock_server): + mock_server.views = getter + content_url = "blah" + DatasourcesAndWorkbooks.get_view_by_content_url(mock_logger, mock_server, content_url) + getter.get.assert_called() + # should also assert the filter on content url From c9720b25462079e8b5b4752a5eec412a5995e64f Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Wed, 1 Feb 2023 01:32:54 -0800 Subject: [PATCH 2/3] Fix special chars in filters The logic of the fix is in ds_and_wb:82 Also refactored some methods to get consistent order of args. --- .../datasources_and_workbooks_command.py | 54 +++++++++++-------- .../export_command.py | 18 ++++--- .../get_url_command.py | 6 +-- .../test_datasources_and_workbooks_command.py | 10 ++-- tests/commands/test_geturl_utils.py | 10 ++-- 5 files changed, 56 insertions(+), 42 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 3fb6ac6a..50315f29 100644 --- a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py +++ b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py @@ -1,5 +1,6 @@ +import urllib + import tableauserverclient as TSC -import urllib3 from tabcmd.commands.constants import Errors from tabcmd.commands.server import Server @@ -24,7 +25,7 @@ def get_view_by_content_url(logger, server, view_content_url) -> TSC.ViewItem: try: req_option = TSC.RequestOptions() req_option.filter.add(TSC.Filter("contentUrl", TSC.RequestOptions.Operator.Equals, view_content_url)) - logger.trace(req_option.get_query_params()) + logger.debug(req_option.get_query_params()) matching_views, paging = server.views.get(req_option) except Exception as e: Errors.exit_with_error(logger, e) @@ -38,7 +39,7 @@ def get_wb_by_content_url(logger, server, workbook_content_url) -> TSC.WorkbookI try: req_option = TSC.RequestOptions() req_option.filter.add(TSC.Filter("contentUrl", TSC.RequestOptions.Operator.Equals, workbook_content_url)) - logger.trace(req_option.get_query_params()) + logger.debug(req_option.get_query_params()) matching_workbooks, paging = server.workbooks.get(req_option) except Exception as e: Errors.exit_with_error(logger, e) @@ -52,7 +53,7 @@ def get_ds_by_content_url(logger, server, datasource_content_url) -> TSC.Datasou try: req_option = TSC.RequestOptions() req_option.filter.add(TSC.Filter("contentUrl", TSC.RequestOptions.Operator.Equals, datasource_content_url)) - logger.trace(req_option.get_query_params()) + logger.debug(req_option.get_query_params()) matching_datasources, paging = server.datasources.get(req_option) except Exception as e: Errors.exit_with_error(logger, e) @@ -61,39 +62,46 @@ def get_ds_by_content_url(logger, server, datasource_content_url) -> TSC.Datasou return matching_datasources[0] @staticmethod - def apply_values_from_url_params(request_options: TSC.PDFRequestOptions, url, logger) -> None: + def apply_values_from_url_params(logger, request_options: TSC.PDFRequestOptions, url) -> None: # should be able to replace this with request_options._append_view_filters(params) logger.debug(url) try: if "?" in url: query = url.split("?")[1] - logger.trace("Query parameters: {}".format(query)) + logger.debug("Query parameters: {}".format(query)) else: logger.debug("No query parameters present in url") return params = query.split("&") - logger.trace(params) + logger.debug(params) for value in params: if value.startswith(":"): - DatasourcesAndWorkbooks.apply_option_value(request_options, value, logger) + DatasourcesAndWorkbooks.apply_option_value(logger, request_options, value) else: # it must be a filter - DatasourcesAndWorkbooks.apply_filter_value(request_options, value, logger) + # the REST API doesn't appear to have the option to disambiguate with "Parameter." + value = value.replace("Parameters.", "") + # the filter values received from the url are already url encoded. tsc will encode them again. + # so we run url.decode, which will be a no-op if they are not encoded. + decoded_value = urllib.parse.unquote(value) + logger.debug("url had `{0}`, saved as `{1}`".format(value, decoded_value)) + DatasourcesAndWorkbooks.apply_filter_value(logger, request_options, decoded_value) except Exception as e: logger.warn("Error building filter params", e) # ExportCommand.log_stack(logger) # type: ignore + # this is called from within from_url_params, for each view_filter value @staticmethod - def apply_filter_value(request_options: TSC.PDFRequestOptions, value: str, logger) -> None: - # todo: do we need to strip Parameters.x -> x? - logger.trace("handling filter param {}".format(value)) + def apply_filter_value(logger, request_options: TSC.PDFRequestOptions, value: str) -> None: + logger.debug("handling filter param {}".format(value)) data_filter = value.split("=") request_options.vf(data_filter[0], data_filter[1]) + # this is called from within from_url_params, for each param value @staticmethod - def apply_option_value(request_options: TSC.PDFRequestOptions, value: str, logger) -> None: - logger.trace("handling url option {}".format(value)) + def apply_option_value(logger, request_options: TSC.PDFRequestOptions, value: str) -> None: + logger.debug("handling url option {}".format(value)) setting = value.split("=") if ":iid" == setting[0]: logger.debug(":iid value ignored in url") @@ -112,19 +120,23 @@ def is_truthy(value: str): return value.lower() in ["yes", "y", "1", "true"] @staticmethod - def apply_png_options(request_options: TSC.ImageRequestOptions, args, logger): - if args.height or args.width: - # only applicable for png - logger.warn("Height/width arguments not yet implemented in export") + def apply_png_options(logger, request_options: TSC.ImageRequestOptions, args): + try: + if args.height or args.width: + # only applicable for png + logger.warn("Height/width arguments not yet implemented in export") + except AttributeError as ae: + logger.debug("Unexpected: no user or default value is present: {}".format(ae.__str__())) # Always request high-res images request_options.image_resolution = "high" @staticmethod def apply_pdf_options(request_options: TSC.PDFRequestOptions, args, logger): - request_options.page_type = args.pagesize - if args.pagelayout: - logger.debug("Setting page layout to: {}".format(args.pagelayout)) + try: + request_options.page_type = args.pagesize request_options.orientation = args.pagelayout + except AttributeError as ae: + logger.debug("Unexpected: no user or default value is present: {}".format(ae.__str__())) @staticmethod def save_to_data_file(logger, output, filename): diff --git a/tabcmd/commands/datasources_and_workbooks/export_command.py b/tabcmd/commands/datasources_and_workbooks/export_command.py index 6775181a..3738f474 100644 --- a/tabcmd/commands/datasources_and_workbooks/export_command.py +++ b/tabcmd/commands/datasources_and_workbooks/export_command.py @@ -104,9 +104,10 @@ def run_command(args): default_filename = "{}.png".format(view_item.name) - except Exception as e: + except TSC.ServerResponseException as e: Errors.exit_with_error(logger, _("publish.errors.unexpected_server_response").format(""), e) - + except Exception as e: + Errors.exit_with_error(logger, exception=e) try: save_name = args.filename or default_filename if args.csv: @@ -129,13 +130,14 @@ def apply_values_from_args(request_options: TSC.PDFRequestOptions, args, logger= if args.filter: params = args.filter.split("&") for value in params: - ExportCommand.apply_filter_value(request_options, value, logger) + 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) - ExportCommand.apply_values_from_url_params(pdf_options, args.url, logger) + ExportCommand.apply_values_from_url_params(logger, pdf_options, args.url) ExportCommand.apply_values_from_args(pdf_options, args, logger) logger.debug(pdf_options.get_query_params()) server.workbooks.populate_pdf(workbook_item, pdf_options) @@ -145,7 +147,7 @@ def download_wb_pdf(server, workbook_item, args, logger): def download_view_pdf(server, view_item, args, logger): logger.debug(args.url) pdf_options = TSC.PDFRequestOptions(maxage=1) - ExportCommand.apply_values_from_url_params(pdf_options, args.url, logger) + ExportCommand.apply_values_from_url_params(logger, pdf_options, args.url) ExportCommand.apply_values_from_args(pdf_options, args, logger) logger.debug(pdf_options.get_query_params()) server.views.populate_pdf(view_item, pdf_options) @@ -155,7 +157,7 @@ def download_view_pdf(server, view_item, args, logger): def download_csv(server, view_item, args, logger): logger.debug(args.url) csv_options = TSC.CSVRequestOptions(maxage=1) - ExportCommand.apply_values_from_url_params(csv_options, args.url, logger) + ExportCommand.apply_values_from_url_params(logger, csv_options, args.url) ExportCommand.apply_values_from_args(csv_options, args, logger) logger.debug(csv_options.get_query_params()) server.views.populate_csv(view_item, csv_options) @@ -165,9 +167,9 @@ def download_csv(server, view_item, args, logger): def download_png(server, view_item, args, logger): logger.debug(args.url) image_options = TSC.ImageRequestOptions(maxage=1) - ExportCommand.apply_values_from_url_params(image_options, args.url, logger) + ExportCommand.apply_values_from_url_params(logger, image_options, args.url) ExportCommand.apply_values_from_args(image_options, args, logger) - DatasourcesAndWorkbooks.apply_png_options(image_options, args, logger) + DatasourcesAndWorkbooks.apply_png_options(logger, image_options, args) logger.debug(image_options.get_query_params()) server.views.populate_image(view_item, image_options) return view_item.image diff --git a/tabcmd/commands/datasources_and_workbooks/get_url_command.py b/tabcmd/commands/datasources_and_workbooks/get_url_command.py index 83fb158a..f24c8dcc 100644 --- a/tabcmd/commands/datasources_and_workbooks/get_url_command.py +++ b/tabcmd/commands/datasources_and_workbooks/get_url_command.py @@ -176,7 +176,7 @@ def generate_pdf(logger, server, args, view_url): view_item: TSC.ViewItem = GetUrl.get_view_by_content_url(logger, server, view_url) logger.debug(_("content_type.view") + ": {}".format(view_item.name)) req_option_pdf = TSC.PDFRequestOptions(maxage=1) - DatasourcesAndWorkbooks.apply_values_from_url_params(req_option_pdf, args.url, logger) + DatasourcesAndWorkbooks.apply_values_from_url_params(logger, req_option_pdf, args.url) server.views.populate_pdf(view_item, req_option_pdf) filename = GetUrl.filename_from_args(args.filename, view_item.name, "pdf") DatasourcesAndWorkbooks.save_to_file(logger, view_item.pdf, filename) @@ -190,7 +190,7 @@ def generate_png(logger, server, args, view_url): view_item: TSC.ViewItem = GetUrl.get_view_by_content_url(logger, server, view_url) logger.debug(_("content_type.view") + ": {}".format(view_item.name)) req_option_csv = TSC.ImageRequestOptions(maxage=1) - DatasourcesAndWorkbooks.apply_values_from_url_params(req_option_csv, args.url, logger) + DatasourcesAndWorkbooks.apply_values_from_url_params(logger, req_option_csv, args.url) server.views.populate_image(view_item, req_option_csv) filename = GetUrl.filename_from_args(args.filename, view_item.name, "png") DatasourcesAndWorkbooks.save_to_file(logger, view_item.image, filename) @@ -204,7 +204,7 @@ def generate_csv(logger, server, args, view_url): view_item: TSC.ViewItem = GetUrl.get_view_by_content_url(logger, server, view_url) logger.debug(_("content_type.view") + ": {}".format(view_item.name)) req_option_csv = TSC.CSVRequestOptions(maxage=1) - DatasourcesAndWorkbooks.apply_values_from_url_params(req_option_csv, args.url, logger) + DatasourcesAndWorkbooks.apply_values_from_url_params(logger, req_option_csv, args.url) server.views.populate_csv(view_item, req_option_csv) file_name_with_path = GetUrl.filename_from_args(args.filename, view_item.name, "csv") DatasourcesAndWorkbooks.save_to_data_file(logger, view_item.csv, file_name_with_path) diff --git a/tests/commands/test_datasources_and_workbooks_command.py b/tests/commands/test_datasources_and_workbooks_command.py index 3454f365..f64b3b18 100644 --- a/tests/commands/test_datasources_and_workbooks_command.py +++ b/tests/commands/test_datasources_and_workbooks_command.py @@ -29,28 +29,28 @@ def test_apply_filters_from_url_params(self): query_params = "?Product=widget" expected = [("Product", "widget")] request_options = tsc.PDFRequestOptions() - DatasourcesAndWorkbooks.apply_values_from_url_params(request_options, query_params, mock_logger) + DatasourcesAndWorkbooks.apply_values_from_url_params(mock_logger, request_options, query_params) assert request_options.view_filters == expected def test_apply_encoded_filters_from_url_params(self): query_params = "?Product%20type=Z%C3%BCrich" expected = [("Product type", "Zürich")] request_options = tsc.PDFRequestOptions() - DatasourcesAndWorkbooks.apply_values_from_url_params(request_options, query_params, mock_logger) + DatasourcesAndWorkbooks.apply_values_from_url_params(mock_logger, request_options, query_params) assert request_options.view_filters == expected def test_apply_options_from_url_params(self): query_params = "?:iid=5&:refresh=yes&:size=600,700" request_options = tsc.PDFRequestOptions() - DatasourcesAndWorkbooks.apply_values_from_url_params(request_options, query_params, mock_logger) + DatasourcesAndWorkbooks.apply_values_from_url_params(mock_logger, request_options, query_params) 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.height = 76 request_options = tsc.ImageRequestOptions() - DatasourcesAndWorkbooks.apply_png_options(request_options, mock_args, mock_logger) + DatasourcesAndWorkbooks.apply_png_options(mock_logger, request_options, mock_args) assert request_options.image_resolution == "high" def test_apply_pdf_options(self): diff --git a/tests/commands/test_geturl_utils.py b/tests/commands/test_geturl_utils.py index 117a9181..32497be6 100644 --- a/tests/commands/test_geturl_utils.py +++ b/tests/commands/test_geturl_utils.py @@ -149,7 +149,7 @@ def test_apply_filter(self): options = TSC.PDFRequestOptions() assert options.view_filters is not None assert len(options.view_filters) is 0 - ExportCommand.apply_filter_value(options, "param1=value1", mock_logger) + ExportCommand.apply_filter_value(mock_logger, options, "param1=value1") assert len(options.view_filters) == 1 assert options.view_filters[0] == ("param1", "value1") @@ -158,7 +158,7 @@ def test_extract_query_params(self): options = TSC.PDFRequestOptions() assert options.view_filters is not None assert len(options.view_filters) is 0 - ExportCommand.apply_values_from_url_params(options, url, mock_logger) + ExportCommand.apply_values_from_url_params(mock_logger, options, url) assert len(options.view_filters) == 1 assert options.view_filters[0] == ("param1", "value1") @@ -166,21 +166,21 @@ def test_refresh_true(self): url = "wb-name/view-name?:refresh=TRUE" options = TSC.PDFRequestOptions() assert options.max_age == -1 - ExportCommand.apply_values_from_url_params(options, url, mock_logger) + ExportCommand.apply_values_from_url_params(mock_logger, options, url) assert options.max_age == 0 def test_refresh_yes(self): url = "wb-name/view-name?:refresh=yes" options = TSC.PDFRequestOptions() assert options.max_age == -1 - ExportCommand.apply_values_from_url_params(options, url, mock_logger) + ExportCommand.apply_values_from_url_params(mock_logger, options, url) assert options.max_age == 0 def test_refresh_y(self): url = "wb-name/view-name?:refresh=y" options = TSC.PDFRequestOptions() assert options.max_age == -1 - ExportCommand.apply_values_from_url_params(options, url, mock_logger) + ExportCommand.apply_values_from_url_params(mock_logger, options, url) assert options.max_age == 0 def test_save_to_binary_file(self): From f66905946ecca65504997ef6ebe6ebc4015ce068 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Thu, 2 Feb 2023 00:42:41 -0800 Subject: [PATCH 3/3] refactoring - removed super defensive exception catching - stopped sending filters for workbooks - CR feedback --- .../datasources_and_workbooks_command.py | 43 ++++++++++--------- .../export_command.py | 22 ++++------ .../test_datasources_and_workbooks_command.py | 3 +- 3 files changed, 31 insertions(+), 37 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 50315f29..1a2169f5 100644 --- a/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py +++ b/tabcmd/commands/datasources_and_workbooks/datasources_and_workbooks_command.py @@ -63,7 +63,6 @@ def get_ds_by_content_url(logger, server, datasource_content_url) -> TSC.Datasou @staticmethod def apply_values_from_url_params(logger, request_options: TSC.PDFRequestOptions, url) -> None: - # should be able to replace this with request_options._append_view_filters(params) logger.debug(url) try: if "?" in url: @@ -77,15 +76,9 @@ def apply_values_from_url_params(logger, request_options: TSC.PDFRequestOptions, logger.debug(params) for value in params: if value.startswith(":"): - DatasourcesAndWorkbooks.apply_option_value(logger, request_options, value) + DatasourcesAndWorkbooks.apply_options_in_url(logger, request_options, value) else: # it must be a filter - # the REST API doesn't appear to have the option to disambiguate with "Parameter." - value = value.replace("Parameters.", "") - # the filter values received from the url are already url encoded. tsc will encode them again. - # so we run url.decode, which will be a no-op if they are not encoded. - decoded_value = urllib.parse.unquote(value) - logger.debug("url had `{0}`, saved as `{1}`".format(value, decoded_value)) - DatasourcesAndWorkbooks.apply_filter_value(logger, request_options, decoded_value) + DatasourcesAndWorkbooks.apply_encoded_filter_value(logger, request_options, value) except Exception as e: logger.warn("Error building filter params", e) @@ -93,6 +86,19 @@ def apply_values_from_url_params(logger, request_options: TSC.PDFRequestOptions, # this is called from within from_url_params, for each view_filter value @staticmethod + def apply_encoded_filter_value(logger, request_options, value): + # the REST API doesn't appear to have the option to disambiguate with "Parameters." + value = value.replace("Parameters.", "") + # the filter values received from the url are already url encoded. tsc will encode them again. + # so we run url.decode, which will be a no-op if they are not encoded. + decoded_value = urllib.parse.unquote(value) + logger.debug("url had `{0}`, saved as `{1}`".format(value, decoded_value)) + DatasourcesAndWorkbooks.apply_filter_value(logger, request_options, decoded_value) + + # this is called for each filter 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: logger.debug("handling filter param {}".format(value)) data_filter = value.split("=") @@ -100,7 +106,7 @@ 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_option_value(logger, request_options: TSC.PDFRequestOptions, value: str) -> None: + def apply_options_in_url(logger, request_options: TSC.PDFRequestOptions, value: str) -> None: logger.debug("handling url option {}".format(value)) setting = value.split("=") if ":iid" == setting[0]: @@ -121,22 +127,17 @@ def is_truthy(value: str): @staticmethod def apply_png_options(logger, request_options: TSC.ImageRequestOptions, args): - try: - if args.height or args.width: - # only applicable for png - logger.warn("Height/width arguments not yet implemented in export") - except AttributeError as ae: - logger.debug("Unexpected: no user or default value is present: {}".format(ae.__str__())) + if args.height or args.width: + logger.warn("Height/width arguments not yet implemented in export") # Always request high-res images request_options.image_resolution = "high" @staticmethod - def apply_pdf_options(request_options: TSC.PDFRequestOptions, args, logger): - try: - request_options.page_type = args.pagesize + def apply_pdf_options(logger, request_options: TSC.PDFRequestOptions, args): + if args.pagelayout: request_options.orientation = args.pagelayout - except AttributeError as ae: - logger.debug("Unexpected: no user or default value is present: {}".format(ae.__str__())) + if args.pagesize: + request_options.page_type = args.pagesize @staticmethod def save_to_data_file(logger, output, filename): diff --git a/tabcmd/commands/datasources_and_workbooks/export_command.py b/tabcmd/commands/datasources_and_workbooks/export_command.py index 3738f474..a76766d3 100644 --- a/tabcmd/commands/datasources_and_workbooks/export_command.py +++ b/tabcmd/commands/datasources_and_workbooks/export_command.py @@ -59,8 +59,8 @@ def define_args(export_parser): group.add_argument("--height", default=600, help=_("export.options.height")) group.add_argument( "--filter", - metavar="COLUMN:VALUE", - help="View filter to apply to the view", + metavar="COLUMN=VALUE", + help="Data filter to apply to the view", ) """ @@ -119,14 +119,7 @@ def run_command(args): Errors.exit_with_error(logger, "Error saving to file", e) @staticmethod - def apply_values_from_args(request_options: TSC.PDFRequestOptions, args, logger=None) -> None: - logger.debug( - "Args: {}, {}, {}, {}, {}".format(args.pagelayout, args.pagesize, args.width, args.height, args.filter) - ) - if args.pagelayout: - request_options.orientation = args.pagelayout - if args.pagesize: - request_options.page_type = args.pagesize + def apply_filters_from_args(request_options: TSC.PDFRequestOptions, args, logger=None) -> None: if args.filter: params = args.filter.split("&") for value in params: @@ -138,7 +131,7 @@ def download_wb_pdf(server, workbook_item, args, logger): logger.debug(args.url) pdf_options = TSC.PDFRequestOptions(maxage=1) ExportCommand.apply_values_from_url_params(logger, pdf_options, args.url) - ExportCommand.apply_values_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) return workbook_item.pdf @@ -148,7 +141,8 @@ def download_view_pdf(server, view_item, args, logger): logger.debug(args.url) pdf_options = TSC.PDFRequestOptions(maxage=1) ExportCommand.apply_values_from_url_params(logger, pdf_options, args.url) - ExportCommand.apply_values_from_args(pdf_options, args, logger) + 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.views.populate_pdf(view_item, pdf_options) return view_item.pdf @@ -158,7 +152,7 @@ def download_csv(server, view_item, args, logger): logger.debug(args.url) csv_options = TSC.CSVRequestOptions(maxage=1) ExportCommand.apply_values_from_url_params(logger, csv_options, args.url) - ExportCommand.apply_values_from_args(csv_options, args, logger) + ExportCommand.apply_filters_from_args(csv_options, args, logger) logger.debug(csv_options.get_query_params()) server.views.populate_csv(view_item, csv_options) return view_item.csv @@ -168,7 +162,7 @@ def download_png(server, view_item, args, logger): logger.debug(args.url) image_options = TSC.ImageRequestOptions(maxage=1) ExportCommand.apply_values_from_url_params(logger, image_options, args.url) - ExportCommand.apply_values_from_args(image_options, args, logger) + ExportCommand.apply_filters_from_args(image_options, args, logger) DatasourcesAndWorkbooks.apply_png_options(logger, image_options, args) logger.debug(image_options.get_query_params()) server.views.populate_image(view_item, image_options) diff --git a/tests/commands/test_datasources_and_workbooks_command.py b/tests/commands/test_datasources_and_workbooks_command.py index f64b3b18..0cbf765b 100644 --- a/tests/commands/test_datasources_and_workbooks_command.py +++ b/tests/commands/test_datasources_and_workbooks_command.py @@ -59,8 +59,7 @@ def test_apply_pdf_options(self): mock_args.pagelayout = expected_layout mock_args.pagesize = expected_page request_options = tsc.PDFRequestOptions() - DatasourcesAndWorkbooks.apply_pdf_options(request_options, mock_args, mock_logger) - params = request_options.get_query_params() + DatasourcesAndWorkbooks.apply_pdf_options(mock_logger, request_options, mock_args) assert request_options.page_type == expected_page assert request_options.orientation == expected_layout