Bug description

The issue: - DatasetList component only checks for the can_write permission when showing the delete button (link to code: https://github.com/apache/superset/blob/master/superset-frontend/src/pages/DatasetList/index.tsx#L417) - DeleteDatasetCommand executes security_manager.raise_for_ownership(model) and fails - UI shows the There was an issue deleting <dataset name>: Forbidden error

Expected behavior: - The delete button is disabled if the user is not the dataset owner, similarly to the edit button

Proposed solution: use the allowEdit variable to disable the button

{canDelete && (
  <Tooltip
    id="delete-action-tooltip"
    title={
      allowEdit
        ? t('Delete')
        : t(
            'You must be a dataset owner in order to delete. Please reach out to a dataset owner to request modifications or edit access.',
          )
    }
    placement="bottom"
  >
    <span
      role="button"
      tabIndex={0}
      className={allowEdit ? 'action-button' : 'disabled'}
      onClick={allowEdit ? handleDelete : undefined}
    >
      <Icons.Trash />
    </span>
  </Tooltip>
)}

Screenshots/recordings

Image Image

Superset version

master / latest-dev

Python version

Not applicable

Node version

Not applicable

Browser

Chrome

Additional context

No response

Checklist

  • [x] I have searched Superset docs and Slack and didn't find a solution to my problem.
  • [x] I have searched the GitHub issue tracker and didn't find a similar bug report.
  • [x] I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.

Comment From: dosubot[bot]

@fnardin-maystreet Hello there! I'm here to help you with any issues you're experiencing. I can assist with bug fixes, answer questions, and guide you to become a contributor. Let me know how I can assist you with the problem you're facing.

The issue you're encountering is due to the DatasetList component only checking for the can_write permission to display the delete button, which doesn't account for dataset ownership. This results in a "Forbidden" error when a non-owner attempts to delete a dataset.

To resolve this, you can use the allowEdit variable to conditionally disable the delete button for users who are not the dataset owners. This approach is similar to how the edit button is handled. Here's a proposed solution:

{canDelete && (
  <Tooltip
    id="delete-action-tooltip"
    title={
      allowEdit
        ? t('Delete')
        : t(
            'You must be a dataset owner in order to delete. Please reach out to a dataset owner to request modifications or edit access.',
          )
    }
    placement="bottom"
  >
    <span
      role="button"
      tabIndex={0}
      className={allowEdit ? 'action-button' : 'disabled'}
      onClick={allowEdit ? handleDelete : undefined}
    >
      <Icons.Trash />
    </span>
  </Tooltip>
)}

In this code snippet, the tooltip informs users that they must be the dataset owner to delete it, and the button is visually disabled if they are not. This change should prevent non-owners from attempting to delete datasets and encountering the "Forbidden" error.

To continue talking to Dosu, mention @dosu.

Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Bug Report | Other

Comment From: rusackas

Sounds like a fantastic proposal! Would you be willing to open a PR?

Comment From: qleroy

@fnardin-maystreet is it the same as in https://github.com/apache/superset/issues/18870 ?

Comment From: fnardin-maystreet

Thank you both for your reply!

@qleroy I investigated a bit, and this bug is indeed present on the Dashboards, Charts, and Datasets pages.

@rusackas I'm more than happy to do so! I created https://github.com/apache/superset/pull/32995, please take a look