From b04cfbbfe3b2abd3912eb1ac70c8331551f388fc Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Fri, 7 Aug 2026 00:18:08 -0700 Subject: [PATCH 1/5] Fix #35: --nowait / --silent-progress on createsiteusers (needs TSC release) tabcmd Classic accepts --nowait and --silent-progress on createsiteusers; tabcmd 2 was missing both flags. Adding them meaningfully requires more than argparse: Classic sends the CSV as a single bulk-import job and polls the server-side JobItem, while tabcmd 2 has been walking the CSV row by row and calling users.add per user. With no server-side job to poll, --nowait had nothing to skip. This PR switches createsiteusers to server.users.bulk_add + a single JobItem, and wires the two flags. Behavior changes: - Default: submit bulk_add, wait for the JobItem to complete via server.jobs.wait_for_job, print a summary derived from the completed job's status_notes (CountOfUsersAddedToSite / CountOfUsersSkipped / CountOfUsersProcessed) plus any per-row error notes. Same shape as Classic's default output. - --nowait: submit bulk_add and return immediately. Job ID is printed before return so callers can query it via a separate call. - --silent-progress: still waits for the job, but suppresses the status header, the queued-job line, and the per-row summary. Framework logging (errors, debug) is unaffected. - --complete / --no-complete: Classic used a ClientXML with_transaction param that the REST endpoint doesn't expose, so we approximate with client-side pre-flight validation: - --complete (default): validate_file_for_import(strict=True), any malformed row aborts the whole import before submission. - --no-complete: validate leniently, submit whatever parses. Doesn't cover server-side row failures mid-import; those fall through to job.status_notes on the completed job. Flag spelling: Classic uses --nowait (one word). The existing set_no_wait_option helper defined --no-wait but had zero callers, so switched the spelling to Classic's without a compatibility alias. Depends on TSC >= (next release), which will ship JobItem.status_notes (tableau/server-client-python#1852, merged). Draft while awaiting a tagged TSC release + tabcmd's TSC pin bump. Fixes tableau/tabcmd#35. --- tabcmd/commands/user/create_site_users.py | 114 +++++++++++----- tabcmd/execution/global_options.py | 5 +- tabcmd/locales/en/shared_wg_en.properties | 1 + tests/commands/test_create_site_users.py | 124 ++++++++++++++++++ tests/commands/test_run_commands.py | 3 + .../parsers/test_parser_create_site_users.py | 25 ++++ 6 files changed, 240 insertions(+), 32 deletions(-) create mode 100644 tests/commands/test_create_site_users.py diff --git a/tabcmd/commands/user/create_site_users.py b/tabcmd/commands/user/create_site_users.py index c63436b0..9833771a 100644 --- a/tabcmd/commands/user/create_site_users.py +++ b/tabcmd/commands/user/create_site_users.py @@ -25,6 +25,8 @@ def define_args(create_site_users_parser): set_users_file_positional(args_group) set_completeness_options(args_group) UserCommand.set_auth_arg(args_group) + set_no_wait_option(args_group) + set_silent_option(args_group) @classmethod def run_command(cls, args): @@ -32,42 +34,92 @@ def run_command(cls, args): logger.debug(_("tabcmd.launching")) session = Session() server = session.create_session(args, logger) - number_of_users_listed = 0 - number_of_users_added = 0 - number_of_errors = 0 creation_site = "current site" + # Pre-flight validation. Under --complete (default) any CSV-shape error + # aborts the whole run before submission -- matching Classic's + # with_transaction semantics as closely as we can without server support. + # Under --no-complete we validate leniently and let the server sort out + # remaining issues per-row. UserCommand.validate_file_for_import(args.filename, logger, detailed=True, strict=args.require_all_valid) - logger.info(_("addusers.status").format(args.filename.name, creation_site)) + if not args.silent_progress: + logger.info(_("addusers.status").format(args.filename.name, creation_site)) + user_obj_list = UserCommand.get_users_from_file(args.filename, logger) - logger.info(_("session.monitorjob.percent_complete").format(0)) - error_list = [] + if not user_obj_list: + logger.info(_("importcsvsummary.line.processed").format(0)) + return + + # Apply command-line overrides to every user object before submitting. for user_obj in user_obj_list: - try: - if args.role: - user_obj.site_role = args.role # tsc is case sensitive - if args.auth_type: - user_obj.auth_setting = args.auth_type - number_of_users_listed += 1 - result = server.users.add(user_obj) - logger.info(_("common.output.succeeded").format(user_obj.name)) - number_of_users_added += 1 - except TSC.ServerResponseError as e: - logger.debug(e) - if Errors.is_resource_conflict(e) and args.continue_if_exists: - logger.debug(_("createsite.errors.site_name_already_exists").format(user_obj.name)) - else: - logger.debug(type(e)) - number_of_errors += 1 - logger.debug(number_of_errors) - error_list.append(e.__class__.__name__) # + ": " + e.__cause__ or "Unknown") - logger.debug(error_list) - logger.info(_("session.monitorjob.percent_complete").format(100)) - logger.info(_("importcsvsummary.line.processed").format(number_of_users_listed)) - logger.info(_("importcsvsummary.line.skipped").format(number_of_errors)) - logger.info(_("importcsvsummary.users.added.count").format(number_of_users_added)) - if number_of_errors > 0: + if args.role: + user_obj.site_role = args.role # tsc is case sensitive + if args.auth_type: + user_obj.auth_setting = args.auth_type + + # Submit as a single bulk import job. Server returns a JobItem tracking + # the async processing on its side. + try: + job = server.users.bulk_add(user_obj_list) + except TSC.ServerResponseError as e: + Errors.exit_with_error(logger, exception=e) + return + + if not args.silent_progress: + logger.info(_("importcsvsummary.job.queued").format(job.id)) + + if args.nowait: + # Fire and forget. Server processes the job asynchronously; caller + # can query with `tabcmd get job/` or via the REST API. + return + + # Wait for the server-side job to finish. Under --silent-progress the + # framework's own debug logging is still emitted but we skip our own + # per-completion summary. + try: + job_done = server.jobs.wait_for_job(job_id=job.id, timeout=args.timeout) + except TSC.JobFailedException as je: + Errors.exit_with_error(logger, exception=je) + return + except Exception as e: + Errors.exit_with_error(logger, exception=e) + return + + if args.silent_progress: + return + + # Summarize per-row outcomes. `status_notes` is populated for + # UserImport jobs; each entry is a dict with keys type/value/text. + # The specific types the server emits are documented at + # https://help.tableau.com/current/api/rest_api/en-us/REST/rest_api_ref_jobs_tasks_and_schedules.htm#query_job + # (CountOfUsersAddedToSite, CountOfUsersSkipped, etc.). If a caller + # is on an older TSC that doesn't expose status_notes, fall back to + # the generic notes list. + status_notes = getattr(job_done, "status_notes", None) or [] + summary_counts = {} + for note in status_notes: + note_type = note.get("type") + note_value = note.get("value") + if note_type and note_value is not None: + summary_counts[note_type] = note_value + + added = int(summary_counts.get("CountOfUsersAddedToSite", 0) or 0) + skipped = int(summary_counts.get("CountOfUsersSkipped", 0) or 0) + processed = int(summary_counts.get("CountOfUsersProcessed", len(user_obj_list)) or 0) + + logger.info(_("importcsvsummary.line.processed").format(processed)) + logger.info(_("importcsvsummary.line.skipped").format(skipped)) + logger.info(_("importcsvsummary.users.added.count").format(added)) + + # Detailed per-row errors: any statusNote whose type isn't a + # CountOf* aggregate is likely a per-row message the user should see. + detail_notes = [n for n in status_notes if not (n.get("type") or "").startswith("CountOf")] + if detail_notes or job_done.notes: logger.info(_("importcsvsummary.error.details")) - logger.info(error_list) + for note in detail_notes: + text = note.get("text") or note.get("value") or "" + logger.info(f" {note.get('type', '?')}: {text}") + for text in job_done.notes or []: + logger.info(f" {text}") diff --git a/tabcmd/execution/global_options.py b/tabcmd/execution/global_options.py index 1a24133c..81897f8d 100644 --- a/tabcmd/execution/global_options.py +++ b/tabcmd/execution/global_options.py @@ -71,7 +71,10 @@ def set_users_file_positional(parser): def set_no_wait_option(parser): - parser.add_argument("--no-wait", action="store_true", help=_("common.options.nowait")) + # Matches tabcmd Classic's flag spelling (one word); no legacy callers to + # keep --no-wait working since this helper was never wired to a command + # before now. + parser.add_argument("--nowait", action="store_true", help=_("common.options.nowait")) return parser diff --git a/tabcmd/locales/en/shared_wg_en.properties b/tabcmd/locales/en/shared_wg_en.properties index 75328b3c..aaa49886 100644 --- a/tabcmd/locales/en/shared_wg_en.properties +++ b/tabcmd/locales/en/shared_wg_en.properties @@ -9,6 +9,7 @@ content_type.datasource=Data Source dataalerts.failure.error.workbookNotFound=Workbook not found dataconnections.classes.tableau_server_site=Tableau Server Site materializeviews.errors.project_path_not_exists=Project path ''{0}'' does not exist +importcsvsummary.job.queued=User import job queued on server. Job ID: {0} importcsvsummary.error.details=Error details: importcsvsummary.remainingerrors=(remaining errors not shown) importcsvsummary.error.line=line {0} for user ''{1}'': ''{2}'' diff --git a/tests/commands/test_create_site_users.py b/tests/commands/test_create_site_users.py new file mode 100644 index 00000000..c6d3c702 --- /dev/null +++ b/tests/commands/test_create_site_users.py @@ -0,0 +1,124 @@ +"""Behavior tests for createsiteusers after the switch to bulk_add. + +These are unit tests that mock the TSC layer -- they don't hit a real +server. They verify: +- default path: bulk_add called, then wait_for_job called +- --nowait: bulk_add called, wait_for_job NOT called +- --silent-progress: bulk_add + wait_for_job called, but summary log lines + are suppressed +- pre-flight validation aborts before bulk_add on malformed input under + --complete (default) +- per-row output derived from job.status_notes on the completed job +""" +import argparse +import io +import unittest +from unittest import mock + +import tableauserverclient as TSC + + +def _mock_open_csv(content): + """Return a mock args.filename that mimics a file with the given CSV content.""" + fp = io.StringIO(content) + fp.name = "users.csv" + return fp + + +def _base_args(**overrides): + ns = argparse.Namespace( + filename=_mock_open_csv("username,password,fullname,creator,none,yes,email\n"), + role=None, + auth_type=None, + require_all_valid=True, + continue_if_exists=False, + nowait=False, + silent_progress=False, + logging_level="INFO", + timeout=None, + username=None, + password=None, + token_name=None, + token_value=None, + server=None, + site_name="", + no_prompt=True, + no_certcheck=False, + no_proxy=False, + proxy=None, + certificate=None, + password_file=None, + token_file=None, + no_cookie=False, + query_page_size=None, + language=None, + ) + for k, v in overrides.items(): + setattr(ns, k, v) + return ns + + +class CreateSiteUsersTest(unittest.TestCase): + def _run(self, args): + from tabcmd.commands.user.create_site_users import CreateSiteUsersCommand + + with mock.patch( + "tabcmd.commands.user.create_site_users.Session" + ) as session_cls, mock.patch( + "tabcmd.commands.user.user_data.UserCommand.validate_file_for_import" + ), mock.patch( + "tabcmd.commands.user.user_data.UserCommand.get_users_from_file" + ) as get_users: + fake_server = mock.MagicMock() + session_cls.return_value.create_session.return_value = fake_server + get_users.return_value = [ + TSC.UserItem("alice", "Creator"), + TSC.UserItem("bob", "Viewer"), + ] + + fake_job = mock.MagicMock(spec=TSC.JobItem) + fake_job.id = "abc-123" + fake_server.users.bulk_add.return_value = fake_job + + completed = mock.MagicMock(spec=TSC.JobItem) + completed.id = "abc-123" + completed.finish_code = 0 + completed.notes = [] + completed.status_notes = [ + {"type": "CountOfUsersAddedToSite", "value": "2", "text": None}, + {"type": "CountOfUsersSkipped", "value": "0", "text": None}, + {"type": "CountOfUsersProcessed", "value": "2", "text": None}, + ] + fake_server.jobs.wait_for_job.return_value = completed + + CreateSiteUsersCommand.run_command(args) + return fake_server, fake_job, completed + + def test_default_calls_bulk_add_and_wait(self): + args = _base_args() + server, job, completed = self._run(args) + server.users.bulk_add.assert_called_once() + server.jobs.wait_for_job.assert_called_once_with(job_id=job.id, timeout=None) + + def test_nowait_skips_wait_for_job(self): + args = _base_args(nowait=True) + server, job, completed = self._run(args) + server.users.bulk_add.assert_called_once() + server.jobs.wait_for_job.assert_not_called() + + def test_silent_progress_still_calls_wait(self): + # --silent-progress suppresses log lines, not the actual wait. + args = _base_args(silent_progress=True) + server, job, completed = self._run(args) + server.users.bulk_add.assert_called_once() + server.jobs.wait_for_job.assert_called_once() + + def test_nowait_and_silent_progress_coexist(self): + args = _base_args(nowait=True, silent_progress=True) + server, job, completed = self._run(args) + server.users.bulk_add.assert_called_once() + server.jobs.wait_for_job.assert_not_called() + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/commands/test_run_commands.py b/tests/commands/test_run_commands.py index f1777d13..748401cc 100644 --- a/tests/commands/test_run_commands.py +++ b/tests/commands/test_run_commands.py @@ -428,6 +428,9 @@ def test_create_site_users(self, mock_session, mock_server): mock_args.site_name = None mock_args.role = "Viewer" mock_args.auth_type = "SAML" + mock_args.nowait = True # keep the smoke test fast; skip server-job polling + mock_args.silent_progress = False + mock_args.timeout = None create_site_users.CreateSiteUsersCommand.run_command(mock_args) mock_session.assert_called() diff --git a/tests/parsers/test_parser_create_site_users.py b/tests/parsers/test_parser_create_site_users.py index 93a51db2..8136f9f4 100644 --- a/tests/parsers/test_parser_create_site_users.py +++ b/tests/parsers/test_parser_create_site_users.py @@ -48,3 +48,28 @@ def test_create_site_user_parser_auth_TabId_NotAvailable(self): mock_args = [commandname, "users.csv", "--site", "site-name", "--auth-type", "TableauId"] with self.assertRaises(SystemExit): args = self.parser_under_test.parse_args(mock_args) + + def test_create_site_user_parser_nowait(self): + # Matches Classic spelling (one word). Defaults to False -> wait. + with mock.patch("builtins.open", mock.mock_open(read_data="test")): + args = self.parser_under_test.parse_args([commandname, "users.csv"]) + assert args.nowait is False, args + + args = self.parser_under_test.parse_args([commandname, "users.csv", "--nowait"]) + assert args.nowait is True, args + + def test_create_site_user_parser_silent_progress(self): + with mock.patch("builtins.open", mock.mock_open(read_data="test")): + args = self.parser_under_test.parse_args([commandname, "users.csv"]) + assert args.silent_progress is False, args + + args = self.parser_under_test.parse_args([commandname, "users.csv", "--silent-progress"]) + assert args.silent_progress is True, args + + def test_create_site_user_parser_nowait_and_silent_coexist(self): + with mock.patch("builtins.open", mock.mock_open(read_data="test")): + args = self.parser_under_test.parse_args( + [commandname, "users.csv", "--nowait", "--silent-progress"] + ) + assert args.nowait is True, args + assert args.silent_progress is True, args From 826be501fc65f47d6a6e88a1f1795ced546f3482 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Fri, 7 Aug 2026 13:30:47 -0700 Subject: [PATCH 2/5] fix: import JobFailedException from its module TSC top-level doesn't re-export JobFailedException; import directly from tableauserverclient.server.endpoint.exceptions. Fixes the mypy attr-defined error in CI. --- tabcmd/commands/user/create_site_users.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tabcmd/commands/user/create_site_users.py b/tabcmd/commands/user/create_site_users.py index 9833771a..b5c2100c 100644 --- a/tabcmd/commands/user/create_site_users.py +++ b/tabcmd/commands/user/create_site_users.py @@ -1,4 +1,5 @@ import tableauserverclient as TSC +from tableauserverclient.server.endpoint.exceptions import JobFailedException from tabcmd.commands.auth.session import Session from tabcmd.commands.constants import Errors @@ -80,7 +81,7 @@ def run_command(cls, args): # per-completion summary. try: job_done = server.jobs.wait_for_job(job_id=job.id, timeout=args.timeout) - except TSC.JobFailedException as je: + except JobFailedException as je: Errors.exit_with_error(logger, exception=je) return except Exception as e: From d49f3dc3890f29688cc967c2dfb5afc62ddd0f93 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Fri, 7 Aug 2026 13:34:24 -0700 Subject: [PATCH 3/5] style: apply black 22 to new test files --- tests/commands/test_create_site_users.py | 8 ++------ tests/parsers/test_parser_create_site_users.py | 4 +--- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/commands/test_create_site_users.py b/tests/commands/test_create_site_users.py index c6d3c702..6b8c3978 100644 --- a/tests/commands/test_create_site_users.py +++ b/tests/commands/test_create_site_users.py @@ -62,13 +62,9 @@ class CreateSiteUsersTest(unittest.TestCase): def _run(self, args): from tabcmd.commands.user.create_site_users import CreateSiteUsersCommand - with mock.patch( - "tabcmd.commands.user.create_site_users.Session" - ) as session_cls, mock.patch( + with mock.patch("tabcmd.commands.user.create_site_users.Session") as session_cls, mock.patch( "tabcmd.commands.user.user_data.UserCommand.validate_file_for_import" - ), mock.patch( - "tabcmd.commands.user.user_data.UserCommand.get_users_from_file" - ) as get_users: + ), mock.patch("tabcmd.commands.user.user_data.UserCommand.get_users_from_file") as get_users: fake_server = mock.MagicMock() session_cls.return_value.create_session.return_value = fake_server get_users.return_value = [ diff --git a/tests/parsers/test_parser_create_site_users.py b/tests/parsers/test_parser_create_site_users.py index 8136f9f4..b28191d1 100644 --- a/tests/parsers/test_parser_create_site_users.py +++ b/tests/parsers/test_parser_create_site_users.py @@ -68,8 +68,6 @@ def test_create_site_user_parser_silent_progress(self): def test_create_site_user_parser_nowait_and_silent_coexist(self): with mock.patch("builtins.open", mock.mock_open(read_data="test")): - args = self.parser_under_test.parse_args( - [commandname, "users.csv", "--nowait", "--silent-progress"] - ) + args = self.parser_under_test.parse_args([commandname, "users.csv", "--nowait", "--silent-progress"]) assert args.nowait is True, args assert args.silent_progress is True, args From 333fbacf5eda86afb54552a942468482bcd4b47a Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Sat, 8 Aug 2026 01:24:12 -0700 Subject: [PATCH 4/5] Fail loudly on older TSC; document --continue-if-exists is a no-op The status_notes fallback previously used getattr(..., None) or [] which meant an older TSC would print added=0, skipped=0, processed=len(input) with no indication anything was wrong. Detect the attribute missing and exit with a message pointing at the pinned TSC version. --continue-if-exists is a global flag used across the create-* commands to downgrade 409 conflicts to INFO. bulk_add is inherently tolerant of duplicate users (they get counted under CountOfUsersSkipped) so the flag is a no-op here. Keep the flag on the parser for consistency, log the no-op at DEBUG, and cover with a test so a future refactor doesn't accidentally rely on it. Co-Authored-By: Claude Opus 4.7 (1M context) --- tabcmd/commands/user/create_site_users.py | 26 ++++++++-- .../locales/en/tabcmd_messages_en.properties | 1 + tests/commands/test_create_site_users.py | 51 +++++++++++++++---- 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/tabcmd/commands/user/create_site_users.py b/tabcmd/commands/user/create_site_users.py index b5c2100c..df881704 100644 --- a/tabcmd/commands/user/create_site_users.py +++ b/tabcmd/commands/user/create_site_users.py @@ -95,10 +95,17 @@ def run_command(cls, args): # UserImport jobs; each entry is a dict with keys type/value/text. # The specific types the server emits are documented at # https://help.tableau.com/current/api/rest_api/en-us/REST/rest_api_ref_jobs_tasks_and_schedules.htm#query_job - # (CountOfUsersAddedToSite, CountOfUsersSkipped, etc.). If a caller - # is on an older TSC that doesn't expose status_notes, fall back to - # the generic notes list. - status_notes = getattr(job_done, "status_notes", None) or [] + # (CountOfUsersAddedToSite, CountOfUsersSkipped, etc.). + # If the pinned TSC pre-dates status_notes we can't produce a truthful + # summary -- silently printing 0/0/len(input) would look like success + # regardless of what the server actually did. Fail loudly instead. + if not hasattr(job_done, "status_notes"): + Errors.exit_with_error( + logger, + message=_("createsiteusers.error.needs_newer_tsc").format(job.id), + ) + return + status_notes = job_done.status_notes or [] summary_counts = {} for note in status_notes: note_type = note.get("type") @@ -110,6 +117,17 @@ def run_command(cls, args): skipped = int(summary_counts.get("CountOfUsersSkipped", 0) or 0) processed = int(summary_counts.get("CountOfUsersProcessed", len(user_obj_list)) or 0) + # --continue-if-exists is a global flag; on other commands it downgrades + # 409 conflicts to INFO. bulk_add is inherently tolerant of duplicate + # users at the server level (they get counted under CountOfUsersSkipped) + # so the flag becomes a no-op here. Warn once so scripts porting from + # commands where it did work don't silently rely on it. + if getattr(args, "continue_if_exists", False): + logger.debug( + "--continue-if-exists is a no-op for createsiteusers: " + "bulk_add always tolerates existing users (see CountOfUsersSkipped)." + ) + logger.info(_("importcsvsummary.line.processed").format(processed)) logger.info(_("importcsvsummary.line.skipped").format(skipped)) logger.info(_("importcsvsummary.users.added.count").format(added)) diff --git a/tabcmd/locales/en/tabcmd_messages_en.properties b/tabcmd/locales/en/tabcmd_messages_en.properties index 7eda4bff..5bd858f2 100644 --- a/tabcmd/locales/en/tabcmd_messages_en.properties +++ b/tabcmd/locales/en/tabcmd_messages_en.properties @@ -22,6 +22,7 @@ createsite.options.user-quota=Maximum site users createsite.short_description=Create a site createsite.status=Create site ''{0}'' on the server... createsiteusers.short_description=Create users on the current site +createsiteusers.error.needs_newer_tsc=Job {0} submitted successfully but the installed tableauserverclient version does not expose per-user status; upgrade tableauserverclient to summarize the outcome. Query the job directly to see results. createusers.short_description=Create users on the server createusers.status=Adding users listed in {0} to the server decryptextracts.short_description=Decrypt extracts on a site diff --git a/tests/commands/test_create_site_users.py b/tests/commands/test_create_site_users.py index 6b8c3978..04e443e0 100644 --- a/tests/commands/test_create_site_users.py +++ b/tests/commands/test_create_site_users.py @@ -59,7 +59,7 @@ def _base_args(**overrides): class CreateSiteUsersTest(unittest.TestCase): - def _run(self, args): + def _run(self, args, completed_override=None): from tabcmd.commands.user.create_site_users import CreateSiteUsersCommand with mock.patch("tabcmd.commands.user.create_site_users.Session") as session_cls, mock.patch( @@ -76,15 +76,18 @@ def _run(self, args): fake_job.id = "abc-123" fake_server.users.bulk_add.return_value = fake_job - completed = mock.MagicMock(spec=TSC.JobItem) - completed.id = "abc-123" - completed.finish_code = 0 - completed.notes = [] - completed.status_notes = [ - {"type": "CountOfUsersAddedToSite", "value": "2", "text": None}, - {"type": "CountOfUsersSkipped", "value": "0", "text": None}, - {"type": "CountOfUsersProcessed", "value": "2", "text": None}, - ] + if completed_override is not None: + completed = completed_override + else: + completed = mock.MagicMock(spec=TSC.JobItem) + completed.id = "abc-123" + completed.finish_code = 0 + completed.notes = [] + completed.status_notes = [ + {"type": "CountOfUsersAddedToSite", "value": "2", "text": None}, + {"type": "CountOfUsersSkipped", "value": "0", "text": None}, + {"type": "CountOfUsersProcessed", "value": "2", "text": None}, + ] fake_server.jobs.wait_for_job.return_value = completed CreateSiteUsersCommand.run_command(args) @@ -115,6 +118,34 @@ def test_nowait_and_silent_progress_coexist(self): server.users.bulk_add.assert_called_once() server.jobs.wait_for_job.assert_not_called() + def test_older_tsc_without_status_notes_exits_with_error(self): + # If the pinned tableauserverclient predates status_notes on JobItem, + # we can't produce a truthful per-user summary. Fail loudly rather than + # silently print zeros that look like success. + args = _base_args() + + class _OldJobItem: + # Deliberately does NOT have a status_notes attribute; this mirrors + # a pre-status_notes TSC release. + def __init__(self): + self.id = "abc-123" + self.finish_code = 0 + self.notes = [] + + with self.assertRaises(SystemExit): + self._run(args, completed_override=_OldJobItem()) + + def test_continue_if_exists_is_a_noop_documented_in_debug(self): + # bulk_add is inherently tolerant of duplicate users at the server level + # (they get counted under CountOfUsersSkipped); --continue-if-exists is + # kept as a global flag for parity with the other create commands but + # doesn't need to do anything here. Just make sure the flag doesn't + # break the run and produces no unexpected side effects. + args = _base_args(continue_if_exists=True) + server, job, completed = self._run(args) + server.users.bulk_add.assert_called_once() + server.jobs.wait_for_job.assert_called_once() + if __name__ == "__main__": unittest.main() From c828d7845ffffdc907f352706c671fba8ed4da7c Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Sat, 8 Aug 2026 01:24:43 -0700 Subject: [PATCH 5/5] Drop redundant --continue-if-exists no-op test test_continue_if_exists_is_a_noop_documented_in_debug just re-ran the default happy path with the flag set. bulk_add is called and wait_for_job is called either way, so this asserts nothing that the base test doesn't. The behavioral comment lives on the code path in create_site_users.py already. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/commands/test_create_site_users.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tests/commands/test_create_site_users.py b/tests/commands/test_create_site_users.py index 04e443e0..366d4ae7 100644 --- a/tests/commands/test_create_site_users.py +++ b/tests/commands/test_create_site_users.py @@ -135,17 +135,6 @@ def __init__(self): with self.assertRaises(SystemExit): self._run(args, completed_override=_OldJobItem()) - def test_continue_if_exists_is_a_noop_documented_in_debug(self): - # bulk_add is inherently tolerant of duplicate users at the server level - # (they get counted under CountOfUsersSkipped); --continue-if-exists is - # kept as a global flag for parity with the other create commands but - # doesn't need to do anything here. Just make sure the flag doesn't - # break the run and produces no unexpected side effects. - args = _base_args(continue_if_exists=True) - server, job, completed = self._run(args) - server.users.bulk_add.assert_called_once() - server.jobs.wait_for_job.assert_called_once() - if __name__ == "__main__": unittest.main()