diff --git a/tableauserverclient/models/group_item.py b/tableauserverclient/models/group_item.py index 3cf82621f..af9465dfb 100644 --- a/tableauserverclient/models/group_item.py +++ b/tableauserverclient/models/group_item.py @@ -83,17 +83,18 @@ def from_response(cls, resp, ns): name = group_xml.get('name', None) group_item = cls(name) group_item._id = group_xml.get('id', None) - # AD groups have an extra element under this + + # Domain name is returned in a domain element for some calls + domain_elem = group_xml.find('.//t:domain', namespaces=ns) + if domain_elem is not None: + group_item.domain_name = domain_elem.get('name', None) + + # Import element is returned for both local and AD groups (2020.3+) import_elem = group_xml.find('.//t:import', namespaces=ns) - if (import_elem is not None): - group_item.domain_name = import_elem.get('domainName') - group_item.license_mode = import_elem.get('grantLicenseMode') - group_item.minimum_site_role = import_elem.get('siteRole') - else: - # local group, we will just have two extra attributes here - group_item.domain_name = 'local' - group_item.license_mode = group_xml.get('grantLicenseMode') - group_item.minimum_site_role = group_xml.get('siteRole') + if import_elem is not None: + group_item.domain_name = import_elem.get('domainName', None) + group_item.license_mode = import_elem.get('grantLicenseMode', None) + group_item.minimum_site_role = import_elem.get('siteRole', None) all_group_items.append(group_item) return all_group_items diff --git a/tableauserverclient/server/endpoint/groups_endpoint.py b/tableauserverclient/server/endpoint/groups_endpoint.py index c873dc159..6a9b81afd 100644 --- a/tableauserverclient/server/endpoint/groups_endpoint.py +++ b/tableauserverclient/server/endpoint/groups_endpoint.py @@ -7,8 +7,6 @@ logger = logging.getLogger('tableau.endpoint.groups') -UNLICENSED_USER = UserItem.Roles.Unlicensed - class Groups(Endpoint): @property @@ -58,15 +56,28 @@ def delete(self, group_id): logger.info('Deleted single group (ID: {0})'.format(group_id)) @api(version="2.0") - def update(self, group_item, default_site_role=UNLICENSED_USER, as_job=False): + def update(self, group_item, default_site_role=None, as_job=False): + # (1/8/2021): Deprecated starting v0.15 + if default_site_role is not None: + import warnings + warnings.simplefilter('always', DeprecationWarning) + warnings.warn('Groups.update(...default_site_role=""...) is deprecated, ' + 'please set the minimum_site_role field of GroupItem', + DeprecationWarning) + group_item.minimum_site_role = default_site_role + if not group_item.id: error = "Group item missing ID." raise MissingRequiredFieldError(error) + if as_job and (group_item.domain_name is None or group_item.domain_name == 'local'): + error = "Local groups cannot be updated asynchronously." + raise ValueError(error) + url = "{0}/{1}".format(self.baseurl, group_item.id) - update_req = RequestFactory.Group.update_req(group_item, default_site_role) + update_req = RequestFactory.Group.update_req(group_item, None) server_response = self.put_request(url, update_req) logger.info('Updated group item (ID: {0})'.format(group_item.id)) - if (as_job): + if as_job: return JobItem.from_response(server_response.content, self.parent_srv.namespace)[0] else: return GroupItem.from_response(server_response.content, self.parent_srv.namespace)[0] diff --git a/tableauserverclient/server/request_factory.py b/tableauserverclient/server/request_factory.py index d6e962be3..14c755bbd 100644 --- a/tableauserverclient/server/request_factory.py +++ b/tableauserverclient/server/request_factory.py @@ -299,17 +299,30 @@ def create_ad_req(self, group_item): return ET.tostring(xml_request) def update_req(self, group_item, default_site_role=None): + # (1/8/2021): Deprecated starting v0.15 if default_site_role is not None: + import warnings + warnings.simplefilter('always', DeprecationWarning) + warnings.warn('RequestFactory.Group.update_req(...default_site_role="") is deprecated, ' + 'please set the minimum_site_role field of GroupItem', + DeprecationWarning) group_item.minimum_site_role = default_site_role + xml_request = ET.Element('tsRequest') group_element = ET.SubElement(xml_request, 'group') group_element.attrib['name'] = group_item.name - if group_item.domain_name != 'local': - project_element = ET.SubElement(group_element, 'import') - project_element.attrib['source'] = "ActiveDirectory" - project_element.attrib['domainName'] = group_item.domain_name - project_element.attrib['siteRole'] = group_item.minimum_site_role - project_element.attrib['grantLicenseMode'] = group_item.license_mode + if group_item.domain_name is not None and group_item.domain_name != 'local': + # Import element is only accepted in the request for AD groups + import_element = ET.SubElement(group_element, 'import') + import_element.attrib['source'] = "ActiveDirectory" + import_element.attrib['domainName'] = group_item.domain_name + import_element.attrib['siteRole'] = group_item.minimum_site_role + if group_item.license_mode is not None: + import_element.attrib['grantLicenseMode'] = group_item.license_mode + else: + # Local group request does not accept an 'import' element + if group_item.minimum_site_role is not None: + group_element.attrib['minimumSiteRole'] = group_item.minimum_site_role return ET.tostring(xml_request) diff --git a/test/assets/group_update.xml b/test/assets/group_update.xml index 828e3f251..3c54524c0 100644 --- a/test/assets/group_update.xml +++ b/test/assets/group_update.xml @@ -2,7 +2,7 @@ - /> + + + \ No newline at end of file diff --git a/test/test_group.py b/test/test_group.py index 8aeb4817d..082a63ba3 100644 --- a/test/test_group.py +++ b/test/test_group.py @@ -224,3 +224,13 @@ def test_update(self): self.assertEqual('Group updated name', group.name) self.assertEqual('ExplorerCanPublish', group.minimum_site_role) self.assertEqual('onLogin', group.license_mode) + + # async update is not supported for local groups + def test_update_local_async(self): + group = TSC.GroupItem("myGroup") + group._id = 'ef8b19c0-43b6-11e6-af50-63f5805dbe3c' + self.assertRaises(ValueError, self.server.groups.update, group, as_job=True) + + # mimic group returned from server where domain name is set to 'local' + group.domain_name = "local" + self.assertRaises(ValueError, self.server.groups.update, group, as_job=True)