Skip to content

Commit

Permalink
fix: remove update_charts_owners (#25843)
Browse files Browse the repository at this point in the history
  • Loading branch information
betodealmeida authored and eschutho committed Dec 1, 2023
1 parent 840b486 commit cb6de0a
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 61 deletions.
3 changes: 1 addition & 2 deletions superset/dashboards/commands/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ def __init__(self, data: Dict[str, Any]):
def run(self) -> Model:
self.validate()
try:
dashboard = DashboardDAO.create(self._properties, commit=False)
dashboard = DashboardDAO.update_charts_owners(dashboard, commit=True)
dashboard = DashboardDAO.create(self._properties, commit=True)
except DAOCreateFailedError as ex:
logger.exception(ex.exception)
raise DashboardCreateFailedError() from ex
Expand Down
1 change: 0 additions & 1 deletion superset/dashboards/commands/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ def run(self) -> Model:
data=json.loads(self._properties.get("json_metadata", "{}")),
commit=False,
)
dashboard = DashboardDAO.update_charts_owners(dashboard, commit=False)
db.session.commit()
except DAOUpdateFailedError as ex:
logger.exception(ex.exception)
Expand Down
9 changes: 0 additions & 9 deletions superset/dashboards/dao.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,6 @@ def validate_update_slug_uniqueness(dashboard_id: int, slug: Optional[str]) -> b
return not db.session.query(dashboard_query.exists()).scalar()
return True

@staticmethod
def update_charts_owners(model: Dashboard, commit: bool = True) -> Dashboard:
owners = list(model.owners)
for slc in model.slices:
slc.owners = list(set(owners) | set(slc.owners))
if commit:
db.session.commit()
return model

@staticmethod
def bulk_delete(models: Optional[List[Dashboard]], commit: bool = True) -> None:
item_ids = [model.id for model in models] if models else []
Expand Down
49 changes: 0 additions & 49 deletions tests/integration_tests/dashboards/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1400,55 +1400,6 @@ def test_dashboard_get_no_username(self):
db.session.delete(model)
db.session.commit()

@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_update_dashboard_chart_owners(self):
"""
Dashboard API: Test update chart owners
"""
user_alpha1 = self.create_user(
"alpha1", "password", "Alpha", email="alpha1@superset.org"
)
user_alpha2 = self.create_user(
"alpha2", "password", "Alpha", email="alpha2@superset.org"
)
admin = self.get_user("admin")
slices = []
slices.append(
db.session.query(Slice).filter_by(slice_name="Girl Name Cloud").first()
)
slices.append(db.session.query(Slice).filter_by(slice_name="Trends").first())
slices.append(db.session.query(Slice).filter_by(slice_name="Boys").first())

dashboard = self.insert_dashboard(
"title1",
"slug1",
[admin.id],
slices=slices,
)
self.login(username="admin")
uri = f"api/v1/dashboard/{dashboard.id}"
dashboard_data = {"owners": [user_alpha1.id, user_alpha2.id]}
rv = self.client.put(uri, json=dashboard_data)
self.assertEqual(rv.status_code, 200)

# verify slices owners include alpha1 and alpha2 users
slices_ids = [slice.id for slice in slices]
# Refetch Slices
slices = db.session.query(Slice).filter(Slice.id.in_(slices_ids)).all()
for slice in slices:
self.assertIn(user_alpha1, slice.owners)
self.assertIn(user_alpha2, slice.owners)
self.assertNotIn(admin, slice.owners)
# Revert owners on slice
slice.owners = []
db.session.commit()

# Rollback changes
db.session.delete(dashboard)
db.session.delete(user_alpha1)
db.session.delete(user_alpha2)
db.session.commit()

@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_update_dashboard_chart_owners_propagation(self):
"""
Expand Down

0 comments on commit cb6de0a

Please sign in to comment.