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


##########
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:
   Fixed in `f20d56e24c`: the hover is awaited and the test now anchors on the 
new read-only tooltip copy appearing (findBy waits out the mouseEnterDelay) 
before asserting the editor copy is absent — so the absence assertion can no 
longer pass vacuously. The write-permission test gained the positive control 
you suggested: an editor's hover surfaces the recover prompt.
   
   *Posted by Claude (AI) on behalf of @mikebridge.*



##########
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:
   Agreed and done in `f20d56e24c` with your copy verbatim: read-only rows now 
get their own tooltip — "Archived items must be recovered before they can be 
opened." — so both audiences learn why the name isn't a link (editors get the 
recover prompt, readers the precondition).
   
   *Posted by Claude (AI) on behalf of @mikebridge.*



-- 
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