From c959ed57d48eba1e2ad018ff30d0b835ff78b2f6 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Tue, 4 Oct 2022 10:51:28 -0700 Subject: [PATCH] fix: export-roles in Preset --- CHANGELOG.rst | 1 + src/preset_cli/api/clients/superset.py | 46 ++++----- tests/api/clients/superset_test.py | 126 +++++++++++++------------ 3 files changed, 89 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 52372778..15ecbdb6 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,7 @@ Next - Sync user roles (team, workspace, data access) from a file to a workspace. - Add ``--version`` option. - Do not prompt for workspaces if passing ``--help``. +- Fix ``export-users`` in Preset workspaces. dbt ~~~ diff --git a/src/preset_cli/api/clients/superset.py b/src/preset_cli/api/clients/superset.py index d848324b..a1524349 100644 --- a/src/preset_cli/api/clients/superset.py +++ b/src/preset_cli/api/clients/superset.py @@ -735,11 +735,17 @@ def export_roles(self) -> Iterator[RoleType]: # pylint: disable=too-many-locals """ Return all roles. """ + user_email_map = {user["id"]: user["email"] for user in self.export_users()} + page = 0 while True: params = { + # Superset "psize_RoleModelView": MAX_PAGE_SIZE, "page_RoleModelView": page, + # Preset + "psize_DataRoleModelView": MAX_PAGE_SIZE, + "page_DataRoleModelView": page, } url = self.baseurl / "roles/list/" page += 1 @@ -760,34 +766,31 @@ def export_roles(self) -> Iterator[RoleType]: # pylint: disable=too-many-locals role_id = int(td.find("a").attrs["href"].split("/")[-1]) else: role_id = int(td.find("input").attrs["id"]) - # TODO (betodealmeida): use roles/edit so it works with Preset - role_url = self.baseurl / "roles/show" / str(role_id) + role_url = self.baseurl / "roles/edit" / str(role_id) _logger.debug("GET %s", role_url) response = self.session.get(role_url) soup = BeautifulSoup(response.text, features="html.parser") - tables = soup.find_all("table") - role_table = tables[-1] - keys: List[Tuple[str, Callable[[Any], Any]]] = [ - ("name", str), - ("permissions", parse_html_array), + name = soup.find("input", {"name": "name"}).attrs["value"] + permissions = [ + option.text.strip() + for option in soup.find("select", id="permissions").find_all( + "option", + ) + if "selected" in option.attrs + ] + users = [ + user_email_map[int(option.attrs["value"])] + for option in soup.find("select", id="user").find_all("option") + if "selected" in option.attrs ] - role_info = { - key: parse(tr.find("td").text.strip()) - for (key, parse), tr in zip(keys, role_table.find_all("tr")) - } - - if len(tables) >= 2: - user_table = tables[-2] - role_info["users"] = [ - tr.find_all("td")[4].text.strip() - for tr in user_table.find_all("tr")[1:] - ] - else: - role_info["users"] = [] - yield cast(RoleType, role_info) + yield { + "name": name, + "permissions": permissions, + "users": users, + } def export_rls(self) -> Iterator[RuleType]: """ @@ -819,7 +822,6 @@ def export_rls(self) -> Iterator[RuleType]: # extract the ID to fetch each RLS in a separate request, since the list # view doesn't have all the columns we need rule_id = int(tds[0].find("input").attrs["id"]) - # TODO (betodealmeida): use roles/edit so it works with Preset rule_url = ( self.baseurl / "rowlevelsecurityfiltersmodelview/show" diff --git a/tests/api/clients/superset_test.py b/tests/api/clients/superset_test.py index e0de8049..fbc27bf8 100644 --- a/tests/api/clients/superset_test.py +++ b/tests/api/clients/superset_test.py @@ -1398,7 +1398,7 @@ def test_export_users_preset(requests_mock: Mocker) -> None: ] -def test_export_roles(requests_mock: Mocker) -> None: +def test_export_roles(mocker: MockerFixture, requests_mock: Mocker) -> None: """ Test ``export_roles``. """ @@ -1459,7 +1459,7 @@ def test_export_roles(requests_mock: Mocker) -> None: """, ) requests_mock.get( - "https://superset.example.org/roles/show/1", + "https://superset.example.org/roles/edit/1", text=""" @@ -1467,36 +1467,22 @@ def test_export_roles(requests_mock: Mocker) -> None: - - - - - - - - - - - - - - - - - - - -
First NameLast NameUser NameEmailIs Active?Role
AliceDoeadoeadoe@example.comTrue[Admin]
- - - -
NameAdmin
Permissions[can this, can that]
+ + + """, ) requests_mock.get( - "https://superset.example.org/roles/show/2", + "https://superset.example.org/roles/edit/2", text=""" @@ -1504,14 +1490,28 @@ def test_export_roles(requests_mock: Mocker) -> None: - - - -
NamePublic
Permissions[]
+ + + """, ) + mocker.patch.object( + SupersetClient, + "export_users", + return_value=[ + {"id": 1, "email": "adoe@example.com"}, + {"id": 2, "email": "bdoe@example.com"}, + ], + ) auth = Auth() client = SupersetClient("https://superset.example.org/", auth) @@ -1529,7 +1529,9 @@ def test_export_roles(requests_mock: Mocker) -> None: ] -def test_export_roles_anchor_role_id(requests_mock: Mocker) -> None: +def test_export_roles_anchor_role_id( + mocker: MockerFixture, requests_mock: Mocker, +) -> None: """ Test ``export_roles``. """ @@ -1590,7 +1592,7 @@ def test_export_roles_anchor_role_id(requests_mock: Mocker) -> None: """, ) requests_mock.get( - "https://superset.example.org/roles/show/1", + "https://superset.example.org/roles/edit/1", text=""" @@ -1598,36 +1600,22 @@ def test_export_roles_anchor_role_id(requests_mock: Mocker) -> None: - - - - - - - - - - - - - - - - - - - -
First NameLast NameUser NameEmailIs Active?Role
AliceDoeadoeadoe@example.comTrue[Admin]
- - - -
NameAdmin
Permissions[can this, can that]
+ + + """, ) requests_mock.get( - "https://superset.example.org/roles/show/2", + "https://superset.example.org/roles/edit/2", text=""" @@ -1635,14 +1623,28 @@ def test_export_roles_anchor_role_id(requests_mock: Mocker) -> None: - - - -
NamePublic
Permissions[]
+ + + """, ) + mocker.patch.object( + SupersetClient, + "export_users", + return_value=[ + {"id": 1, "email": "adoe@example.com"}, + {"id": 2, "email": "bdoe@example.com"}, + ], + ) auth = Auth() client = SupersetClient("https://superset.example.org/", auth)