From 948e382d624f0e0769f226576a5fddb8977cbaa9 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Mon, 6 Jul 2026 22:33:19 -0700 Subject: [PATCH 1/6] fix: CSVImport AUTH column, case-insensitive validation, username preservation; add find_by_name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes for UserItem.CSVImport (issue #1809): - MAX=8 (was 7=AUTH index): 8-column lines with auth type no longer rejected as "too many columns" - create_user_from_line no longer lowercases the whole line before splitting — username case is preserved - _validate_import_line_or_throw normalizes license/admin/publisher to lowercase and auth to canonical form before comparison, so 'Viewer', 'Creator', 'SAML', 'tableauidwithmfa' etc. are all accepted - Add TableauIDWithMFA to valid auth values in validation (was missing) - 5 new tests covering each fix Add QuerysetEndpoint.find_by_name(name) (issue #1810): - Thin wrapper over .filter(name=name) returning a list - Available on all content-item endpoints (workbooks, datasources, views, users, projects, groups) Co-Authored-By: Claude Sonnet 4.6 --- tableauserverclient/models/user_item.py | 52 +++++++++++++------ .../server/endpoint/endpoint.py | 3 ++ test/test_user_model.py | 33 ++++++++++++ 3 files changed, 73 insertions(+), 15 deletions(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 0ba1e8eb2..05e0f5916 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -432,36 +432,37 @@ class ColumnType(IntEnum): EMAIL = 6 AUTH = 7 - MAX = 7 + MAX = 8 # total number of columns (not last index) # Read a csv line and create a user item populated by the given attributes @staticmethod def create_user_from_line(line: str): if line is None or line is False or line == "\n" or line == "": return None - line = line.strip().lower() - values: list[str] = list(map(str.strip, line.split(","))) - user = UserItem(values[UserItem.CSVImport.ColumnType.USERNAME]) + values: list[str] = list(map(str.strip, line.strip().split(","))) + if len(values) > UserItem.CSVImport.ColumnType.MAX: + raise ValueError("Too many attributes for user import") + username = values[UserItem.CSVImport.ColumnType.USERNAME] + user = UserItem(username) if len(values) > 1: - if len(values) > UserItem.CSVImport.ColumnType.MAX: - raise ValueError("Too many attributes for user import") - while len(values) <= UserItem.CSVImport.ColumnType.MAX: + while len(values) < UserItem.CSVImport.ColumnType.MAX: values.append("") site_role = UserItem.CSVImport._evaluate_site_role( values[UserItem.CSVImport.ColumnType.LICENSE], values[UserItem.CSVImport.ColumnType.ADMIN], values[UserItem.CSVImport.ColumnType.PUBLISHER], ) - + raw_auth = values[UserItem.CSVImport.ColumnType.AUTH] + auth = UserItem.CSVImport._auth_canonical().get(raw_auth.lower()) if raw_auth else None user._set_values( None, - values[UserItem.CSVImport.ColumnType.USERNAME], + username, site_role, None, None, values[UserItem.CSVImport.ColumnType.DISPLAY_NAME], values[UserItem.CSVImport.ColumnType.EMAIL], - values[UserItem.CSVImport.ColumnType.AUTH], + auth, None, None, None, @@ -491,6 +492,16 @@ def validate_file_for_import(csv_file: io.TextIOWrapper, logger) -> tuple[int, l # Some fields in the import file are restricted to specific values # Iterate through each field and validate the given value against hardcoded constraints + @staticmethod + def _auth_canonical() -> dict[str, str]: + """Lowercase → canonical form mapping for Auth values.""" + return { + "saml": "SAML", + "openid": "OpenID", + "serverdefault": "ServerDefault", + "tableauidwithmfa": "TableauIDWithMFA", + } + @staticmethod def _validate_import_line_or_throw(incoming, logger) -> None: _valid_attributes: list[list[str]] = [ @@ -501,7 +512,12 @@ def _validate_import_line_or_throw(incoming, logger) -> None: ["system", "site", "none", "no"], # admin ["yes", "true", "1", "no", "false", "0"], # publisher [], - [UserItem.Auth.SAML, UserItem.Auth.OpenID, UserItem.Auth.ServerDefault], # auth + [ + "SAML", + "OpenID", + "ServerDefault", + "TableauIDWithMFA", + ], # auth — normalized by _auth_canonical before comparison ] line = list(map(str.strip, incoming.split(","))) @@ -511,10 +527,16 @@ def _validate_import_line_or_throw(incoming, logger) -> None: logger.debug(f"> details - {username}") UserItem.validate_username_or_throw(username) for i in range(1, len(line)): - logger.debug(f"column {UserItem.CSVImport.ColumnType(i).name}: {line[i]}") - UserItem.CSVImport._validate_attribute_value( - line[i], _valid_attributes[i], UserItem.CSVImport.ColumnType(i) - ) + value = line[i] + valid = _valid_attributes[i] + # normalize case for fields with a restricted value set + if valid: + if i == UserItem.CSVImport.ColumnType.AUTH: + value = UserItem.CSVImport._auth_canonical().get(value.lower(), value) + else: + value = value.lower() + logger.debug(f"column {UserItem.CSVImport.ColumnType(i).name}: {value}") + UserItem.CSVImport._validate_attribute_value(value, valid, UserItem.CSVImport.ColumnType(i)) # Given a restricted set of possible values, confirm the item is in that set @staticmethod diff --git a/tableauserverclient/server/endpoint/endpoint.py b/tableauserverclient/server/endpoint/endpoint.py index 31a0806dc..300311b4c 100644 --- a/tableauserverclient/server/endpoint/endpoint.py +++ b/tableauserverclient/server/endpoint/endpoint.py @@ -431,6 +431,9 @@ def fields(self: Self, *fields: str) -> QuerySet: queryset.request_options.fields |= set(fields) | set(("_default_",)) return queryset + def find_by_name(self, name: str) -> list[T]: + return list(self.filter(name=name)) + def only_fields(self: Self, *fields: str) -> QuerySet: """ Add fields to the request options. If no fields are provided, the diff --git a/test/test_user_model.py b/test/test_user_model.py index 49e8dc25c..1b9b74f4e 100644 --- a/test/test_user_model.py +++ b/test/test_user_model.py @@ -136,3 +136,36 @@ def test_validate_usernames_file() -> None: test_data = _mock_file_content(usernames) valid, invalid = TSC.UserItem.CSVImport.validate_file_for_import(test_data, logger) assert valid == 5, f"Exactly 5 of the lines were valid, counted {valid + len(invalid)}" + + +def test_validate_mixed_case_license() -> None: + # Regression: issue #1809 — 'Viewer' (capital V) was rejected by case-sensitive check + TSC.UserItem.CSVImport._validate_import_line_or_throw("username, pword, fname, Viewer, None, no, email", logger) + TSC.UserItem.CSVImport._validate_import_line_or_throw("username, pword, fname, Creator, Site, yes, email", logger) + TSC.UserItem.CSVImport._validate_import_line_or_throw("username, pword, fname, EXPLORER, NONE, YES, email", logger) + + +def test_validate_tableauid_with_mfa_auth() -> None: + # TableauIDWithMFA is a valid auth value and must not be rejected + TSC.UserItem.CSVImport._validate_import_line_or_throw( + "username, pword, fname, creator, none, yes, email, TableauIDWithMFA", logger + ) + + +def test_create_user_preserves_username_case() -> None: + # Username must not be lowercased — case matters for LDAP and email-format usernames + user = TSC.UserItem.CSVImport.create_user_from_line("JSmith, pword, John Smith, creator, none, yes, j@example.com") + assert user is not None + assert user.name == "JSmith", f"Username was lowercased: {user.name}" + + +def test_create_user_with_auth_column() -> None: + # AUTH column (position 7) must be parsed — was broken by MAX=7 off-by-one + user = TSC.UserItem.CSVImport.create_user_from_line("username, pword, fname, creator, none, yes, email, SAML") + assert user is not None + assert user.auth_setting == "SAML", f"Expected SAML, got {user.auth_setting}" + + +def test_too_many_columns_raises() -> None: + with pytest.raises((ValueError, AttributeError)): + TSC.UserItem.CSVImport.create_user_from_line("u, p, n, creator, none, yes, email, SAML, extra") From 157019fd04f72fa6b16e278a3137369eda23d759 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Tue, 7 Jul 2026 12:33:45 -0700 Subject: [PATCH 2/6] fix: add auth column to create_users_csv, fix create_from_file extension check and debug prints - create_users_csv was producing 7-column CSV, silently dropping auth_setting; bulk_add roundtrip would lose auth type - create_from_file extension check used filepath.find("csv") which evaluates as falsy only when "csv" is at index 0, letting all other paths through; fixed to "csv" not in filepath - remove two debug print() calls left in create_from_file Co-Authored-By: Claude Sonnet 4.6 --- tableauserverclient/server/endpoint/users_endpoint.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tableauserverclient/server/endpoint/users_endpoint.py b/tableauserverclient/server/endpoint/users_endpoint.py index 48c77da66..b134bda73 100644 --- a/tableauserverclient/server/endpoint/users_endpoint.py +++ b/tableauserverclient/server/endpoint/users_endpoint.py @@ -527,7 +527,7 @@ def create_from_file(self, filepath: str) -> tuple[list[UserItem], list[tuple[Us warnings.warn("This method is deprecated, use bulk_add instead", DeprecationWarning) created = [] failed = [] - if not filepath.find("csv"): + if "csv" not in filepath: raise ValueError("Only csv files are accepted") with open(filepath) as csv_file: @@ -536,11 +536,9 @@ def create_from_file(self, filepath: str) -> tuple[list[UserItem], list[tuple[Us while line and line != "": user: UserItem = UserItem.CSVImport.create_user_from_line(line) try: - print(user) result = self.add(user) created.append(result) except ServerResponseError as serverError: - print("failed") failed.append((user, serverError)) line = csv_file.readline() return created, failed @@ -751,6 +749,7 @@ def create_users_csv(users: Iterable[UserItem]) -> bytes: - Admin Level - Publish capability - Email + - Auth setting Parameters ---------- @@ -790,6 +789,7 @@ def create_users_csv(users: Iterable[UserItem]) -> bytes: admin_level, publish, user.email, + user.auth_setting or "", ) ) output.seek(0) From 7d4522d01aa422fa856415bb8b41919203c351d0 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Tue, 7 Jul 2026 12:40:39 -0700 Subject: [PATCH 3/6] refactor: extract _decompose_site_role into CSVImport, symmetrical with _evaluate_site_role MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the ad-hoc site role → (license, admin_level, publish) logic from create_users_csv into UserItem.CSVImport._decompose_site_role, making it the explicit inverse of _evaluate_site_role. Also fixes pre-existing bugs in the decomposition: - ExplorerCanPublish was emitted as license="ExplorerCanPublish" (not a valid CSV license value); now correctly "Explorer" with publish=1 - SiteAdministrator (legacy role) was emitting license="" via str.replace; now maps to ("Explorer", "Site", "1") - Non-admin roles now emit admin_level="None" (explicit CSV spec value) rather than "" (empty string); both are accepted by the server but "None" is consistent with the spec and _evaluate_site_role input Co-Authored-By: Claude Sonnet 4.6 --- tableauserverclient/models/user_item.py | 21 +++++++++++++++++++ .../server/endpoint/users_endpoint.py | 17 +-------------- test/test_user.py | 5 +++-- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 05e0f5916..0f1aa57bf 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -548,6 +548,27 @@ def _validate_attribute_value(item: str, possible_values: list[str], column_type return raise AttributeError(f"Invalid value {item} for {column_type}") + # Inverse of _evaluate_site_role: decompose a site role back to (license, admin_level, publish) + # for writing the CSV import format. + @staticmethod + def _decompose_site_role(site_role: str) -> tuple[str, str, str]: + """Return (license, admin_level, publish) CSV column values for a given site role.""" + _role_map: dict[str, tuple[str, str, str]] = { + "ServerAdministrator": ("Creator", "System", "1"), + "SiteAdministratorCreator": ("Creator", "Site", "1"), + "SiteAdministratorExplorer": ("Explorer", "Site", "1"), + "SiteAdministrator": ("Explorer", "Site", "1"), # legacy role, treat as SiteAdministratorExplorer + "Creator": ("Creator", "None", "1"), + "ExplorerCanPublish": ("Explorer", "None", "1"), + "Explorer": ("Explorer", "None", "0"), + "Viewer": ("Viewer", "None", "0"), + "Unlicensed": ("Unlicensed", "None", "0"), + "ReadOnly": ("Viewer", "None", "0"), + "Publisher": ("Explorer", "None", "1"), + "Interactor": ("Explorer", "None", "0"), + } + return _role_map.get(site_role, ("Unlicensed", "None", "0")) + # https://help.tableau.com/current/server/en-us/csvguidelines.htm#settings_and_site_roles # This logic is hardcoded to match the existing rules for import csv files @staticmethod diff --git a/tableauserverclient/server/endpoint/users_endpoint.py b/tableauserverclient/server/endpoint/users_endpoint.py index b134bda73..099dc3531 100644 --- a/tableauserverclient/server/endpoint/users_endpoint.py +++ b/tableauserverclient/server/endpoint/users_endpoint.py @@ -764,22 +764,7 @@ def create_users_csv(users: Iterable[UserItem]) -> bytes: with io.StringIO() as output: writer = csv.writer(output, quoting=csv.QUOTE_MINIMAL) for user in users: - site_role = user.site_role or "Unlicensed" - if site_role == "ServerAdministrator": - license = "Creator" - admin_level = "System" - elif site_role.startswith("SiteAdministrator"): - admin_level = "Site" - license = site_role.replace("SiteAdministrator", "") - else: - license = site_role - admin_level = "" - - if any(x in site_role for x in ("Creator", "Admin", "Publish")): - publish = 1 - else: - publish = 0 - + license, admin_level, publish = UserItem.CSVImport._decompose_site_role(user.site_role or "Unlicensed") writer.writerow( ( f"{user.domain_name}\\{user.name}" if user.domain_name else user.name, diff --git a/test/test_user.py b/test/test_user.py index fec21d25f..316c2a6f1 100644 --- a/test/test_user.py +++ b/test/test_user.py @@ -405,7 +405,7 @@ def test_create_users_csv() -> None: "ServerAdministrator": "System", } - csv_columns = ["name", "password", "fullname", "license", "admin", "publish", "email"] + csv_columns = ["name", "password", "fullname", "license", "admin", "publish", "email", "auth"] csv_data = create_users_csv(users) csv_file = io.StringIO(csv_data.decode("utf-8")) csv_reader = csv.reader(csv_file) @@ -417,8 +417,9 @@ def test_create_users_csv() -> None: assert (user.fullname or "") == csv_user["fullname"] assert (user.email or "") == csv_user["email"] assert license_map[site_role] == csv_user["license"] - assert admin_map.get(site_role, "") == csv_user["admin"] + assert admin_map.get(site_role, "None") == csv_user["admin"] assert publish_map[site_role] == int(csv_user["publish"]) + assert (user.auth_setting or "") == csv_user["auth"] def test_bulk_add(server: TSC.Server) -> None: From ac84fd3c9d81f6e79c4c3c44f6bccf1d2aba9551 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Mon, 27 Jul 2026 23:54:40 -0700 Subject: [PATCH 4/6] fix: decompose unsupported site roles to license="Invalid" not "Unlicensed" Legacy values in UserItem.Roles (UnlicensedWithPublish, ViewerWithPublish, Guest, SupportUser) have never been accepted by the server-side CSV license parser (workgroup: CsvLicenseRoleTypeConverter). The initial _decompose_site_role default of ("Unlicensed", "None", "0") silently coerced these to a valid but semantically wrong Unlicensed user, replacing an old server-side per-row rejection with silent success. Default to license="Invalid" instead so the server continues to reject rows with USER_CSV_INVALID_LICENSE, preserving the pre-refactor observable behavior for callers who inspect job results for per-row failures. Batch resilience is unaffected: bad rows fail, good rows succeed, no client-side throw takes down the whole bulk_add call. Follow-up: deprecate UnlicensedWithPublish/ViewerWithPublish from UserItem.Roles (never worked on any code path); see forthcoming issue. --- tableauserverclient/models/user_item.py | 11 +++++++++-- test/test_user.py | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 0f1aa57bf..9790f408d 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -552,7 +552,14 @@ def _validate_attribute_value(item: str, possible_values: list[str], column_type # for writing the CSV import format. @staticmethod def _decompose_site_role(site_role: str) -> tuple[str, str, str]: - """Return (license, admin_level, publish) CSV column values for a given site role.""" + """Return (license, admin_level, publish) CSV column values for a given site role. + + Site roles not in _role_map emit license="Invalid" so the server rejects that + row with USER_CSV_INVALID_LICENSE. This preserves per-row error semantics for + legacy UserItem.Roles values (UnlicensedWithPublish, ViewerWithPublish, Guest, + SupportUser) that the server-side CSV parser has never accepted, rather than + silently coercing to a valid-but-wrong Unlicensed user. + """ _role_map: dict[str, tuple[str, str, str]] = { "ServerAdministrator": ("Creator", "System", "1"), "SiteAdministratorCreator": ("Creator", "Site", "1"), @@ -567,7 +574,7 @@ def _decompose_site_role(site_role: str) -> tuple[str, str, str]: "Publisher": ("Explorer", "None", "1"), "Interactor": ("Explorer", "None", "0"), } - return _role_map.get(site_role, ("Unlicensed", "None", "0")) + return _role_map.get(site_role, ("Invalid", "None", "0")) # https://help.tableau.com/current/server/en-us/csvguidelines.htm#settings_and_site_roles # This logic is hardcoded to match the existing rules for import csv files diff --git a/test/test_user.py b/test/test_user.py index 316c2a6f1..4dc8baa4d 100644 --- a/test/test_user.py +++ b/test/test_user.py @@ -422,6 +422,20 @@ def test_create_users_csv() -> None: assert (user.auth_setting or "") == csv_user["auth"] +def test_decompose_unsupported_role_emits_invalid_license() -> None: + # UnlicensedWithPublish and ViewerWithPublish are in UserItem.Roles for + # historical reasons but the server-side CSV license parser has never + # accepted them. _decompose_site_role emits license="Invalid" for these + # (and any other unmapped role) so the server rejects the row with + # USER_CSV_INVALID_LICENSE, preserving the per-row error semantics + # callers of bulk_add had before this refactor. + for role in ("UnlicensedWithPublish", "ViewerWithPublish", "Guest", "SupportUser"): + license, admin, publish = TSC.UserItem.CSVImport._decompose_site_role(role) + assert license == "Invalid" + assert admin == "None" + assert publish == "0" + + def test_bulk_add(server: TSC.Server) -> None: server.version = "3.15" users = [ From ee2fb8c295ba8e9ebd79b9833d4547065883137b Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Tue, 28 Jul 2026 14:45:13 -0700 Subject: [PATCH 5/6] docs: clarify _decompose_site_role's asymmetric legacy-role handling The role map handles legacy UserItem.Roles values in two ways depending on whether the server has a modern equivalent. SiteAdministrator, Publisher, Interactor, and ReadOnly map to their current-model equivalents; UnlicensedWithPublish, ViewerWithPublish, Guest, and SupportUser fall to license="Invalid" so the server rejects the row. Reviewers otherwise read the code and see "why does Publisher get mapped but Guest doesn't?" - the answer is server behavior, not arbitrary choice. Docstring now says so. Co-Authored-By: Claude Opus 4.7 (1M context) --- tableauserverclient/models/user_item.py | 27 ++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 9790f408d..275958027 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -554,25 +554,34 @@ def _validate_attribute_value(item: str, possible_values: list[str], column_type def _decompose_site_role(site_role: str) -> tuple[str, str, str]: """Return (license, admin_level, publish) CSV column values for a given site role. - Site roles not in _role_map emit license="Invalid" so the server rejects that - row with USER_CSV_INVALID_LICENSE. This preserves per-row error semantics for - legacy UserItem.Roles values (UnlicensedWithPublish, ViewerWithPublish, Guest, - SupportUser) that the server-side CSV parser has never accepted, rather than - silently coercing to a valid-but-wrong Unlicensed user. + Legacy `UserItem.Roles` values are handled in two ways depending on whether + the server has a sensible modern equivalent: + + - **Mapped to modern equivalents** (row emitted, server accepts): the legacy + roles `SiteAdministrator`, `Publisher`, `Interactor`, and `ReadOnly` each + map to the current-model role that best matches their historical intent + (SiteAdministratorExplorer, ExplorerCanPublish, Explorer, Viewer). + - **Emitted as `license="Invalid"`** (row rejected server-side with + USER_CSV_INVALID_LICENSE): the legacy roles `UnlicensedWithPublish`, + `ViewerWithPublish`, `Guest`, and `SupportUser` have no equivalent in the + current server model (`RestApiSiteRole` does not accept them on any code + path). Emitting `"Invalid"` preserves the per-row error semantics callers + of `bulk_add` had before this refactor, rather than silently coercing + those users to a valid-but-wrong Unlicensed account. """ _role_map: dict[str, tuple[str, str, str]] = { "ServerAdministrator": ("Creator", "System", "1"), "SiteAdministratorCreator": ("Creator", "Site", "1"), "SiteAdministratorExplorer": ("Explorer", "Site", "1"), - "SiteAdministrator": ("Explorer", "Site", "1"), # legacy role, treat as SiteAdministratorExplorer + "SiteAdministrator": ("Explorer", "Site", "1"), # legacy, mapped to SiteAdministratorExplorer "Creator": ("Creator", "None", "1"), "ExplorerCanPublish": ("Explorer", "None", "1"), "Explorer": ("Explorer", "None", "0"), "Viewer": ("Viewer", "None", "0"), "Unlicensed": ("Unlicensed", "None", "0"), - "ReadOnly": ("Viewer", "None", "0"), - "Publisher": ("Explorer", "None", "1"), - "Interactor": ("Explorer", "None", "0"), + "ReadOnly": ("Viewer", "None", "0"), # legacy, mapped to Viewer + "Publisher": ("Explorer", "None", "1"), # legacy, mapped to ExplorerCanPublish + "Interactor": ("Explorer", "None", "0"), # legacy, mapped to Explorer } return _role_map.get(site_role, ("Invalid", "None", "0")) From af21f2b9b856f698eaf031c13c2cc6b8057a7c77 Mon Sep 17 00:00:00 2001 From: Jac Fitzgerald Date: Mon, 17 Aug 2026 12:17:18 -0700 Subject: [PATCH 6/6] fix: accept "1"/"0" and "true"/"false" for publisher, add round-trip test Two related changes to make `_evaluate_site_role` and `_decompose_site_role` a real inverse pair: - `_evaluate_site_role` now accepts "1", "true", and "yes" for the publisher column (and, symmetrically, treats anything else as "no"). Previously only "yes" was accepted, which meant `_decompose_site_role` emitting "1" for Creator/ExplorerCanPublish would round-trip as Explorer. The set of publisher values mirrors what `_valid_attributes[publisher]` already documents as legal (["yes", "true", "1", "no", "false", "0"]), so this brings the two code paths in agreement. - Added a parametrized `test_decompose_then_evaluate_round_trips` covering every entry in `_role_map` plus the two documented label asymmetries (ServerAdministrator -> SiteAdministrator on the way back; legacy roles Publisher/Interactor/ReadOnly/SiteAdministrator folded into their modern equivalents). If either function drifts, a specific input names the broken case rather than the whole loop dying on the first mismatch. - `_decompose_site_role` docstring updated with a Round-trip note naming the two intentional asymmetries so readers do not have to reason about them from the mapping table. Full suite still passes: 884 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) --- tableauserverclient/models/user_item.py | 10 ++++++-- test/test_user_model.py | 33 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/tableauserverclient/models/user_item.py b/tableauserverclient/models/user_item.py index 275958027..4de36e080 100644 --- a/tableauserverclient/models/user_item.py +++ b/tableauserverclient/models/user_item.py @@ -568,6 +568,12 @@ def _decompose_site_role(site_role: str) -> tuple[str, str, str]: path). Emitting `"Invalid"` preserves the per-row error semantics callers of `bulk_add` had before this refactor, rather than silently coercing those users to a valid-but-wrong Unlicensed account. + + Round-trip note: `_evaluate_site_role(*_decompose_site_role(r)) == r` for + every current-model role. Two label asymmetries: `ServerAdministrator` + round-trips through the legacy label `SiteAdministrator` (that's the only + label `_evaluate_site_role` emits for `admin="System"`), and the legacy + roles above are folded into their modern equivalents by design. """ _role_map: dict[str, tuple[str, str, str]] = { "ServerAdministrator": ("Creator", "System", "1"), @@ -606,14 +612,14 @@ def _evaluate_site_role(license_level, admin_level, publisher): else: site_role = "SiteAdministratorExplorer" else: # if it wasn't 'system' or 'site' then we can treat it as 'none' - if publisher == "yes": + if publisher in ("yes", "true", "1"): if license_level == "creator": site_role = "Creator" elif license_level == "explorer": site_role = "ExplorerCanPublish" else: site_role = "Unlicensed" # is this the expected outcome? - else: # publisher == 'no': + else: # publisher is "no" / "false" / "0" / any other value: if license_level == "explorer" or license_level == "creator": site_role = "Explorer" elif license_level == "viewer": diff --git a/test/test_user_model.py b/test/test_user_model.py index 1b9b74f4e..3cfb96c73 100644 --- a/test/test_user_model.py +++ b/test/test_user_model.py @@ -80,6 +80,39 @@ def test_evaluate_role() -> None: assert actual == line[3], line + [actual] +# _decompose_site_role writes CSV rows that the server (and TSC's own +# _evaluate_site_role) parse back into a site role. This parametrized test +# pins the round-trip so a change in either direction can't drift silently. +# The two documented asymmetries are the ServerAdministrator/SiteAdministrator +# label pair and the legacy-role fold; both are captured explicitly below. +@pytest.mark.parametrize( + "role, expected", + [ + # Canonical current-model roles round-trip identity. + ("SiteAdministratorCreator", "SiteAdministratorCreator"), + ("SiteAdministratorExplorer", "SiteAdministratorExplorer"), + ("Creator", "Creator"), + ("ExplorerCanPublish", "ExplorerCanPublish"), + ("Explorer", "Explorer"), + ("Viewer", "Viewer"), + ("Unlicensed", "Unlicensed"), + # admin="System" always evaluates back to the legacy "SiteAdministrator" + # label -- that's the only label _evaluate_site_role emits for System. + ("ServerAdministrator", "SiteAdministrator"), + # Legacy roles fold into their modern equivalents on the way through. + # Documented in _decompose_site_role's docstring. + ("SiteAdministrator", "SiteAdministratorExplorer"), + ("ReadOnly", "Viewer"), + ("Publisher", "ExplorerCanPublish"), + ("Interactor", "Explorer"), + ], +) +def test_decompose_then_evaluate_round_trips(role: str, expected: str) -> None: + license_level, admin_level, publish = TSC.UserItem.CSVImport._decompose_site_role(role) + actual = TSC.UserItem.CSVImport._evaluate_site_role(license_level, admin_level, publish) + assert actual == expected, (role, license_level, admin_level, publish, actual) + + def test_get_user_detail_empty_line() -> None: test_line = "" test_user = TSC.UserItem.CSVImport.create_user_from_line(test_line)