aminghadersohi commented on code in PR #44179:
URL: https://github.com/apache/superset/pull/44179#discussion_r3993671100


##########
superset-frontend/src/pages/ArchivedList/ArchivedList.test.tsx:
##########
@@ -227,6 +238,106 @@ test('issues the deleted-only baseline filter on the list 
request', async () =>
   });
 });
 
+test.each([
+  { resource: 'chart', label: 'Chart', name: 'Deleted Chart One' },
+  { resource: 'dashboard', label: 'Dashboard', name: 'Deleted Dashboard One' },
+  { resource: 'dataset', label: 'Dataset', name: 'deleted_table_one' },
+])(
+  'read-only $label rows do not offer recovery or purge',
+  async ({ resource, label, name }) => {
+    mockRoutes(200, {}, buildPositiveImpact(), {
+      [resource]: { permissions: ['can_read', 'can_export'] },
+    });
+    renderArchivedList(storeWithReadAccess(label));
+
+    const rowName = await screen.findByText(name);
+    expect(
+      screen.queryByRole('columnheader', { name: 'Actions' }),
+    ).not.toBeInTheDocument();
+    expect(screen.queryAllByTestId('archived-row-restore')).toHaveLength(0);
+    expect(screen.queryAllByTestId('archived-row-purge')).toHaveLength(0);
+    userEvent.hover(rowName);

Review Comment:
   `userEvent.hover` (v12) is synchronous and antd Tooltip has a 0.1s 
`mouseEnterDelay`, so this assertion passes even if the tooltip would render — 
it passes on master too. Nothing else asserts the tooltip ever appears, so the 
suppression isn't actually pinned.



##########
superset-frontend/src/pages/ArchivedList/index.tsx:
##########
@@ -432,10 +435,12 @@ function ArchivedListBody({
           // rather than told anything. Neither is a preview, and the silent
           // one is the worse of the two, so no row links out until the object
           // is recovered.
-          return (
+          return canWrite ? (

Review Comment:
   Dropping the tooltip for read-only users also drops the only explanation for 
why the name isn't a link — they get a bare, unexplained non-clickable name. A 
read-only variant (e.g. "Archived items must be recovered before they can be 
opened") may serve them better than none.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to