[tabcmd] refactor: extract CSVImport._decompose_site_role, inverse of _evaluate_site_role

tableauserverclient/models/user_item.py

@@ -548,6 +548,49 @@ 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.
+
+            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.
+
+            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"),
+                "SiteAdministratorCreator": ("Creator", "Site", "1"),
+                "SiteAdministratorExplorer": ("Explorer", "Site", "1"),
+                "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"),  # 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"))
+
         # 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
@@ -569,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":

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
     ----------
@@ -765,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,
@@ -790,6 +774,7 @@ def create_users_csv(users: Iterable[UserItem]) -> bytes:
                     admin_level,
                     publish,
                     user.email,
+                    user.auth_setting or "",
                 )
             )
         output.seek(0)

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,23 @@ 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_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:

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)