search

data-dot-all / dataall

A modern data marketplace that makes collaboration among diverse users (like business, analysts and engineers) easier, increasing efficiency and agility in data projects on AWS.
https://data-dot-all.github.io/dataall/
Apache License 2.0
228 stars 82 forks source link

Deleting environment linked team doesn't consider draft share request on that team #1442

Open TejasRGitHub opened 1 month ago

TejasRGitHub commented 1 month ago

Describe the bug

I found out that I was able to delete a linked team on an environment which has draft share request on it. Due to this, whenever I go to the share view and filter for "Draft" share requests, I get a blank page. When I take a look at the console, the error says that the SamlAdminGroup name for a share request was not found while doing the getRequestsToMe graphql request.

How to Reproduce

  1. Create an environment / link an environment to an org in data.all
  2. Link another team
  3. Create draft share request
  4. Delete the team
  5. Go to shares view and then filter on "Draft"

Expected behavior

Should not be able to delete the team without clearing off the shares attached to it

Your project

No response

Screenshots

No response

OS

Mac

Python version

3.9

AWS data.all version

2.6

Additional context

No response

noah-paige commented 1 month ago

Thank you for filing this issue @TejasRGitHub - looks like this is a bug on how we are currently counting existing shares before removing an associated env group or consumption role

I was also able to replicate the above and can see the root cause coming from count_S3_principal_shares and/or count_S3_role_principal_shares where if there are no share items we will get a count of 0 even if the share object exists

I believe a quick fix here would be to filter by S3 Dataset Type on the Share Object rather than trying to filter on the itemTypes (which may not exist on the share object itself)

I will do some evaluation and see how soon we can prioritize a fix for this bug

noah-paige commented 1 month ago

Proposed solution is to change the way we handle deletes rather than the count functions for Env Resources

https://github.com/data-dot-all/dataall/pull/1467#discussion_r1714211320