fix(dashboard): reconcile stale results tab in Chart Data modal

Address PR #43454 review feedback: reuse DataTablesPane's stale-tab
fallback so ResultsPaneOnDashboard doesn't render blank when a mixed
chart's active results tab disappears, and drop CSS rules now
redundant with the fullHeight prop.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

superset-frontend/src/dashboard/util/permissionUtils.test.ts

@@ -259,6 +259,21 @@ describe('isUserEditorOrAdmin', () => {
   test('returns false when editors is omitted', () => {
     expect(isUserEditorOrAdmin(outsiderUser)).toEqual(false);
   });
+
+  test('returns true when the user is granted editorship only through extra_editors', () => {
+    expect(isUserEditorOrAdmin(editorUser, [], [10])).toEqual(true);
+  });
+
+  test('unions editors and extra_editors rather than preferring one', () => {
+    const nonMatchingSubject: Subject = { id: 999, label: 'Other', type: 1 };
+    expect(isUserEditorOrAdmin(editorUser, [nonMatchingSubject], [10])).toEqual(
+      true,
+    );
+  });
+
+  test('returns false when extra_editors names other subjects', () => {
+    expect(isUserEditorOrAdmin(editorUser, [], [999])).toEqual(false);
+  });
 });
 
 // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks

superset-frontend/src/dashboard/util/permissionUtils.ts

@@ -55,9 +55,6 @@ export const isUserInSubjects = (
   );
 };
 
-const isUserInEditors = (editors: Subject[] = []): boolean =>
-  isUserInSubjects(editors);
-
 export const isUserAdmin = (
   user?: UserWithPermissionsAndRoles | UndefinedUser,
 ) =>
@@ -66,10 +63,12 @@ export const isUserAdmin = (
     role => role.toLowerCase() === ADMIN_ROLE_NAME.toLowerCase(),
   );
 
+/** `extraEditors` is editorship granted via a deployment's EXTRA_EDITORS_RESOLVER. */
 export const isUserEditorOrAdmin = (
   user?: UserWithPermissionsAndRoles | UndefinedUser,
   editors: Subject[] = [],
-): boolean => isUserInEditors(editors) || isUserAdmin(user);
+  extraEditors?: SubjectRef[] | null,
+): boolean => isUserInSubjects(editors, extraEditors) || isUserAdmin(user);
 
 /**
  * Editorship of *dashboard*, matching the server's `is_editor`: the explicit

superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx

@@ -29,22 +29,7 @@ import {
 } from 'src/utils/localStorageHelpers';
 import { SamplesPane, useResultsPane } from './components';
 import { DataTablesPaneProps, ResultTypes } from './types';
-
-/**
- * A mixed chart can be reconfigured to return fewer result panes than before
- * (e.g. dropping a query), which removes the corresponding results tab. If the
- * selected tab was one of those, the active key goes stale and the data panel
- * renders blank until the user reselects a valid tab. Returns the first
- * results tab to fall back to in that case, otherwise undefined.
- */
-export const getStaleResultsTabFallback = (
-  activeTabKey: string,
-  resultsTabKeys: string[],
-): string | undefined =>
-  activeTabKey.startsWith(ResultTypes.Results) &&
-  !resultsTabKeys.includes(activeTabKey)
-    ? ResultTypes.Results
-    : undefined;
+import { getStaleResultsTabFallback } from './utils';
 
 const StyledDiv = styled.div`
   ${() => `

superset-frontend/src/explore/components/DataTablesPane/components/ResultsPaneOnDashboard.tsx

@@ -20,22 +20,15 @@ import { t } from '@apache-superset/core/translation';
 import { styled } from '@apache-superset/core/theme';
 import Tabs from '@superset-ui/core/components/Tabs';
 import { ResultTypes, ResultsPaneProps } from '../types';
+import { getStaleResultsTabFallback } from '../utils';
 import { useResultsPane } from './useResultsPane';
-import { useState } from 'react';
+import { useEffect, useState } from 'react';
 
 const Wrapper = styled.div`
   display: flex;
   flex-direction: column;
   height: 100%;
 
-  .ant-tabs {
-    height: 100%;
-  }
-
-  .ant-tabs-body {
-    height: 100%;
-  }
-
   .ant-tabs-content {
     display: flex;
     flex-direction: column;
@@ -86,9 +79,25 @@ export const ResultsPaneOnDashboard = ({
     };
   });
 
+  const resultsTabFallback = getStaleResultsTabFallback(
+    activeTabKey,
+    items.map(({ key }) => key),
+  );
+
+  useEffect(() => {
+    if (resultsTabFallback) {
+      setActiveTabKey(resultsTabFallback);
+    }
+  }, [resultsTabFallback]);
+
   return (
     <Wrapper>
-      <Tabs activeKey={activeTabKey} onChange={setActiveTabKey} items={items} />
+      <Tabs
+        fullHeight
+        activeKey={activeTabKey}
+        onChange={setActiveTabKey}
+        items={items}
+      />
     </Wrapper>
   );
 };

superset-frontend/src/explore/components/DataTablesPane/test/ResultsPaneOnDashboard.test.tsx

@@ -20,14 +20,34 @@ import fetchMock from 'fetch-mock';
 import {
   screen,
   render,
+  act,
   waitForElementToBeRemoved,
   waitFor,
 } from 'spec/helpers/testing-library';
 import { ChartMetadata, ChartPlugin, VizType } from '@superset-ui/core';
 import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact';
+import Tabs from '@superset-ui/core/components/Tabs';
 import { ResultsPaneOnDashboard } from '../components';
+import { useResultsPane } from '../components/useResultsPane';
 import { createResultsPaneOnDashboardProps } from './fixture';
 
+// `fullHeight`'s CSS isn't testable under jsdom (no `importSource` for the
+// `css` prop in jest's babel config), so spy on call args instead.
+jest.mock('@superset-ui/core/components/Tabs', () => {
+  const actual = jest.requireActual('@superset-ui/core/components/Tabs');
+  return { __esModule: true, ...actual, default: jest.fn(actual.default) };
+});
+
+// Wraps the real hook; only overridden below to avoid mounting a second
+// real AG Grid instance, which jsdom doesn't support.
+jest.mock('../components/useResultsPane', () => {
+  const actual = jest.requireActual('../components/useResultsPane');
+  return {
+    __esModule: true,
+    useResultsPane: jest.fn(actual.useResultsPane),
+  };
+});
+
 beforeAll(() => {
   setupAGGridModules();
 });
@@ -106,6 +126,10 @@ describe('ResultsPaneOnDashboard', () => {
     expect(
       await findByText('No results were returned for this query'),
     ).toBeVisible();
+    expect(Tabs).toHaveBeenCalledWith(
+      expect.objectContaining({ fullHeight: true }),
+      expect.anything(),
+    );
   });
 
   test('render errorMessage', async () => {
@@ -219,4 +243,36 @@ describe('ResultsPaneOnDashboard', () => {
     expect(tab2).toBeVisible();
     expect(tab3).toBeNull();
   });
+
+  test('falls back to the first results tab when the active one disappears', async () => {
+    const mockedUseResultsPane = useResultsPane as jest.Mock;
+    mockedUseResultsPane.mockReturnValue([<div key="a" />, <div key="b" />]);
+
+    const props = createResultsPaneOnDashboardProps({ sliceId: 999 });
+    const { rerender } = render(<ResultsPaneOnDashboard {...props} />, {
+      useRedux: true,
+    });
+
+    const latestTabsProps = () => {
+      const { calls } = (Tabs as unknown as jest.Mock).mock;
+      return calls[calls.length - 1][0];
+    };
+    expect(latestTabsProps().items.map((i: { key: string }) => i.key)).toEqual([
+      'results',
+      'results 2',
+    ]);
+
+    act(() => {
+      latestTabsProps().onChange('results 2');
+    });
+    expect(latestTabsProps().activeKey).toBe('results 2');
+
+    // A mixed chart dropped from two query results to one, removing "results 2"
+    mockedUseResultsPane.mockReturnValue([<div key="a" />]);
+    rerender(<ResultsPaneOnDashboard {...props} />);
+
+    await waitFor(() => {
+      expect(latestTabsProps().activeKey).toBe('results');
+    });
+  });
 });

superset-frontend/src/explore/components/DataTablesPane/test/getStaleResultsTabFallback.test.ts

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { getStaleResultsTabFallback } from '../DataTablesPane';
+import { getStaleResultsTabFallback } from '../utils';
 import { ResultTypes } from '../types';
 
 test('keeps the active tab when it still exists', () => {

superset-frontend/src/explore/components/DataTablesPane/utils.ts

@@ -0,0 +1,35 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ResultTypes } from './types';
+
+/**
+ * A mixed chart can be reconfigured to return fewer result panes than before
+ * (e.g. dropping a query), which removes the corresponding results tab. If the
+ * selected tab was one of those, the active key goes stale and the data panel
+ * renders blank until the user reselects a valid tab. Returns the first
+ * results tab to fall back to in that case, otherwise undefined.
+ */
+export const getStaleResultsTabFallback = (
+  activeTabKey: string,
+  resultsTabKeys: string[],
+): string | undefined =>
+  activeTabKey.startsWith(ResultTypes.Results) &&
+  !resultsTabKeys.includes(activeTabKey)
+    ? ResultTypes.Results
+    : undefined;

superset-frontend/src/features/charts/ChartCard.tsx

@@ -96,7 +96,11 @@ export default function ChartCard({
   const canEdit = hasPerm('can_write');
   const canDelete = hasPerm('can_write');
   const canExport = hasPerm('can_export');
-  const allowEdit = isUserEditorOrAdmin(user, chart.editors);
+  const allowEdit = isUserEditorOrAdmin(
+    user,
+    chart.editors,
+    chart.extra_editors,
+  );
   const menuItems: MenuItem[] = [];
 
   if (canEdit) {

superset-frontend/src/features/dashboards/DashboardCard.tsx

@@ -83,7 +83,11 @@ function DashboardCard({
   const canEdit = hasPerm('can_write');
   const canDelete = hasPerm('can_write');
   const canExport = hasPerm('can_export');
-  const allowEdit = isUserEditorOrAdmin(user, dashboard.editors);
+  const allowEdit = isUserEditorOrAdmin(
+    user,
+    dashboard.editors,
+    dashboard.extra_editors,
+  );
   const digest = dashboard.changed_on_utc || dashboard.changed_on;
   const thumbnailUrl =
     isFeatureEnabled(FeatureFlag.Thumbnails) && dashboard.id && digest

superset-frontend/src/pages/ChartList/index.tsx

@@ -650,7 +650,11 @@ function ChartList(props: ChartListProps) {
       },
       {
         Cell: ({ row: { original } }: CellProps<Chart>) => {
-          const allowEdit = isUserEditorOrAdmin(user, original.editors);
+          const allowEdit = isUserEditorOrAdmin(
+            user,
+            original.editors,
+            original.extra_editors,
+          );
           const openEditModal = () => openChartEditModal(original);
           const handleExport = () => handleBulkChartExport([original]);
           if (!canEdit && !canDelete && !canExport) {

superset-frontend/src/pages/DashboardList/index.tsx

@@ -122,6 +122,8 @@ export interface Dashboard {
   description?: string;
   thumbnail_url?: string | null;
   editors?: Subject[];
+  // Bare subject ids from a deployment's EXTRA_EDITORS_RESOLVER.
+  extra_editors?: number[];
   viewers?: Subject[];
   tags: TagType[];
   created_by: object;
@@ -505,7 +507,11 @@ function DashboardList(props: DashboardListProps) {
       },
       {
         Cell: ({ row: { original } }: CellProps<Dashboard>) => {
-          const allowEdit = isUserEditorOrAdmin(user, original.editors);
+          const allowEdit = isUserEditorOrAdmin(
+            user,
+            original.editors,
+            original.extra_editors,
+          );
           const handleDelete = () =>
             handleDashboardDelete(
               original,

superset-frontend/src/types/Chart.ts

@@ -45,6 +45,8 @@ export interface Chart {
   cache_timeout: number | null;
   thumbnail_url?: string;
   editors?: Subject[];
+  // Bare subject ids from a deployment's EXTRA_EDITORS_RESOLVER.
+  extra_editors?: number[];
   viewers?: Subject[];
   tags?: TagType[];
   last_saved_at?: string;

superset-frontend/src/views/CRUD/types.ts

@@ -67,6 +67,8 @@ export interface Dashboard {
   url: string;
   thumbnail_url?: string | null;
   editors?: Subject[];
+  // Bare subject ids from a deployment's EXTRA_EDITORS_RESOLVER.
+  extra_editors?: number[];
   viewers?: Subject[];
   loading?: boolean;
 }

superset/charts/api.py

@@ -92,7 +92,10 @@ from superset.exceptions import (
 )
 from superset.extensions import event_logger, security_manager
 from superset.models.slice import Slice
-from superset.security.manager import get_extra_editor_subject_ids
+from superset.security.manager import (
+    get_extra_editor_subject_ids,
+    get_extra_editors_by_pk,
+)
 from superset.subjects.filters import (
     FilterRelatedSubjects,
     subject_type_filter,
@@ -410,6 +413,15 @@ class ChartRestApi(SoftDeleteApiMixin, BaseSupersetModelRestApi):
         except ChartNotFoundError:
             return self.response_404()
 
+    def pre_get_list(self, data: dict[str, Any]) -> None:
+        """Attach ``extra_editors`` to each row, matching the single-object GET."""
+        super().pre_get_list(data)
+        ids = data.get("ids", [])
+        extra_editors_by_id = get_extra_editors_by_pk(Slice, ids)
+        for row, row_id in zip(data.get("result", []), ids, strict=False):
+            if row_id in extra_editors_by_id:
+                row["extra_editors"] = extra_editors_by_id[row_id]
+
     @expose("/<pk>/deck_layers/", methods=("GET",))
     @protect()
     @safe

superset/commands/tag/create.py

@@ -18,8 +18,6 @@ import logging
 from functools import partial
 from typing import Any
 
-from jinja2.exceptions import TemplateError
-
 from superset import security_manager
 from superset.commands.base import BaseCommand, CreateMixin
 from superset.commands.tag.exceptions import TagCreateFailedError, TagInvalidError
@@ -29,7 +27,7 @@ from superset.commands.tag.utils import (
     to_object_type,
 )
 from superset.daos.tag import TagDAO
-from superset.exceptions import SupersetParseError, SupersetSecurityException
+from superset.exceptions import SupersetSecurityException
 from superset.tags.models import ObjectType, TagType
 from superset.utils.decorators import on_error, transaction
 
@@ -100,30 +98,8 @@ class CreateCustomTagCommand(CreateMixin, BaseCommand):
                     )
                 )
         except SupersetSecurityException:
-            # A routine, expected authorization denial; swallowed silently by
-            # design (no logging) and surfaced to the caller as a validation
-            # failure rather than an unhandled 500.
             exceptions.append(
-                TagCreateFailedError(
-                    f"Could not validate access for {object_type} {object_id}"
-                )
-            )
-        except (TemplateError, SupersetParseError) as ex:
-            # Authorizing a saved query parses its Jinja-templated SQL to resolve
-            # table references. Malformed Jinja (TemplateError) or an
-            # unresolvable partition macro (SupersetParseError) is a validation
-            # failure, not an unhandled 500 -- but unlike an access denial it is
-            # genuinely unexpected, so log it for server-side visibility and
-            # preserve the underlying error text instead of discarding it.
-            logger.warning(
-                "Could not parse query %s while validating tag access: %s",
-                object_id,
-                str(ex),
-            )
-            exceptions.append(
-                TagCreateFailedError(
-                    f"Could not validate access for {object_type} {object_id}: {ex}"
-                )
+                TagCreateFailedError(f"Access denied for {object_type} {object_id}")
             )
 
 

superset/common/form_data_query_context.py

@@ -213,7 +213,9 @@ def orderby_from_form_data(
     # The drag-and-drop "sort by" control persists a list; the frontend unwraps it
     # with ``ensureIsArray(...)[0]`` (``plugin-chart-table/src/buildQuery.ts:67``).
     # Read raw, a list would nest inside ``orderby`` and fail the query.
-    raw_sort_metric = form_data.get("timeseries_limit_metric")
+    raw_sort_metric = form_data.get("series_limit_metric") or form_data.get(
+        "timeseries_limit_metric"
+    )
     sort_metric = (
         next(iter(as_list(raw_sort_metric)), None) if raw_sort_metric else None
     ) or (metrics[0] if form_data.get("sort_by_metric") else None)

superset/dashboards/api.py

@@ -142,7 +142,10 @@ from superset.extensions import event_logger, security_manager
 from superset.models.dashboard import Dashboard
 from superset.models.embedded_dashboard import EmbeddedDashboard
 from superset.security.guest_token import GuestUser
-from superset.security.manager import get_extra_editor_subject_ids
+from superset.security.manager import (
+    get_extra_editor_subject_ids,
+    get_extra_editors_by_pk,
+)
 from superset.subjects.filters import (
     FilterRelatedSubjects,
     subject_type_filter,
@@ -433,6 +436,15 @@ class DashboardRestApi(
         """
         return super().get_list(**kwargs)
 
+    def pre_get_list(self, data: dict[str, Any]) -> None:
+        """Attach ``extra_editors`` to each row, matching the single-object GET."""
+        super().pre_get_list(data)
+        ids = data.get("ids", [])
+        extra_editors_by_id = get_extra_editors_by_pk(Dashboard, ids)
+        for row, row_id in zip(data.get("result", []), ids, strict=False):
+            if row_id in extra_editors_by_id:
+                row["extra_editors"] = extra_editors_by_id[row_id]
+
     list_select_columns = list_columns + ["changed_on", "created_on", "changed_by_fk"]
     order_columns = [
         "changed_by.first_name",

superset/examples/birth_names.py

@@ -290,7 +290,7 @@ def create_slices(tbl: SqlaTable) -> tuple[list[Slice], list[Slice]]:
                 groupby=["name"],
                 adhoc_filters=[gen_filter("gender", "girl")],
                 row_limit=50,
-                timeseries_limit_metric=metric,
+                series_limit_metric=metric,
                 metrics=[metric],
             ),
             editors=[],
@@ -321,7 +321,7 @@ def create_slices(tbl: SqlaTable) -> tuple[list[Slice], list[Slice]]:
                 groupby=["name"],
                 adhoc_filters=[gen_filter("gender", "boy")],
                 row_limit=50,
-                timeseries_limit_metric=metric,
+                series_limit_metric=metric,
                 metrics=[metric],
             ),
             editors=[],
@@ -498,7 +498,7 @@ def create_slices(tbl: SqlaTable) -> tuple[list[Slice], list[Slice]]:
                 viz_type="echarts_timeseries_line",
                 granularity_sqla="ds",
                 groupby=["name"],
-                timeseries_limit_metric={
+                series_limit_metric={
                     "expressionType": "SIMPLE",
                     "column": {
                         "column_name": "num_california",
@@ -522,7 +522,7 @@ def create_slices(tbl: SqlaTable) -> tuple[list[Slice], list[Slice]]:
                 metrics=metrics,
                 groupby=["name"],
                 row_limit=50,
-                timeseries_limit_metric={
+                series_limit_metric={
                     "expressionType": "SIMPLE",
                     "column": {
                         "column_name": "num_california",

superset/examples/usa_births_names/charts/Boys.yaml

@@ -36,8 +36,8 @@ params:
   metrics:
   - sum__num
   row_limit: 50
+  series_limit_metric: sum__num
   time_range: '100 years ago : now'
-  timeseries_limit_metric: sum__num
   viz_type: table
 query_context: null
 slice_name: Boys

superset/examples/usa_births_names/charts/Girls.yaml

@@ -36,8 +36,8 @@ params:
   metrics:
   - sum__num
   row_limit: 50
+  series_limit_metric: sum__num
   time_range: '100 years ago : now'
-  timeseries_limit_metric: sum__num
   viz_type: table
 query_context: null
 slice_name: Girls

superset/security/manager.py

@@ -170,6 +170,36 @@ def get_extra_editor_subject_ids(resource: Model) -> list[int]:
     return subject_ids
 
 
+def get_extra_editors_by_pk(
+    model_cls: type[Model], primary_keys: list[Any]
+) -> dict[Any, list[int]]:
+    """
+    Resolve extra editor subject IDs for a batch of resources, keyed by
+    primary key. List responses only have serialized rows, not model
+    instances, so this re-queries the page's rows in one batched query.
+    """
+    if not primary_keys or not (
+        has_app_context() and current_app.config.get("EXTRA_EDITORS_RESOLVER")
+    ):
+        return {}
+
+    # pylint: disable=import-outside-toplevel
+    from superset import db
+    from superset.models.helpers import SKIP_VISIBILITY_FILTER_CLASSES
+
+    pk_col = inspect(model_cls).primary_key[0]
+    resources = (
+        db.session.query(model_cls)
+        .execution_options(**{SKIP_VISIBILITY_FILTER_CLASSES: {model_cls}})
+        .filter(pk_col.in_(primary_keys))
+        .all()
+    )
+    return {
+        getattr(resource, pk_col.name): get_extra_editor_subject_ids(resource)
+        for resource in resources
+    }
+
+
 def _render_permission_instructions_link(
     *,
     datasource_id: str = "",

tests/integration_tests/dashboards/api_tests.py

@@ -922,6 +922,55 @@ class TestDashboardApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCa
             db.session.delete(dashboard)
             db.session.commit()
 
+    def test_get_dashboards_list_omits_extra_editors_by_default(self):
+        """No EXTRA_EDITORS_RESOLVER configured: list rows omit extra_editors."""
+        admin = self.get_user("admin")
+        dashboard = self.insert_dashboard(
+            "no_extra_editors_list_dashboard",
+            "no-extra-editors-list-dashboard",
+            [admin.id],
+        )
+        try:
+            self.login(ADMIN_USERNAME)
+            rv = self.client.get("api/v1/dashboard/")
+            assert rv.status_code == 200
+            data = json.loads(rv.data.decode("utf-8"))
+            row = next(
+                d
+                for d in data["result"]
+                if d["dashboard_title"] == dashboard.dashboard_title
+            )
+            assert "extra_editors" not in row
+        finally:
+            db.session.delete(dashboard)
+            db.session.commit()
+
+    @with_config({"EXTRA_EDITORS_RESOLVER": lambda resource: [123]})
+    def test_get_dashboards_list_includes_extra_editors_when_resolver_configured(
+        self,
+    ):
+        """List rows get extra_editors too, mirroring the single-object GET."""
+        admin = self.get_user("admin")
+        dashboard = self.insert_dashboard(
+            "extra_editors_list_dashboard",
+            "extra-editors-list-dashboard",
+            [admin.id],
+        )
+        try:
+            self.login(ADMIN_USERNAME)
+            rv = self.client.get("api/v1/dashboard/")
+            assert rv.status_code == 200
+            data = json.loads(rv.data.decode("utf-8"))
+            row = next(
+                d
+                for d in data["result"]
+                if d["dashboard_title"] == dashboard.dashboard_title
+            )
+            assert row["extra_editors"] == [123]
+        finally:
+            db.session.delete(dashboard)
+            db.session.commit()
+
     def test_get_charts_admin_sees_existing_charts(self):
         """Regression for #25890: GET /api/v1/chart/ as an Admin user should
         return existing charts, not an empty list."""
@@ -944,6 +993,41 @@ class TestDashboardApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCa
             db.session.delete(chart)
             db.session.commit()
 
+    def test_get_charts_list_omits_extra_editors_by_default(self):
+        """No EXTRA_EDITORS_RESOLVER configured: list rows omit extra_editors."""
+        admin = self.get_user("admin")
+        chart = self.insert_chart(
+            "no_extra_editors_list_chart", [admin.id], 1, params="{}"
+        )
+        try:
+            self.login(ADMIN_USERNAME)
+            rv = self.client.get("api/v1/chart/")
+            assert rv.status_code == 200
+            data = json.loads(rv.data.decode("utf-8"))
+            row = next(c for c in data["result"] if c["slice_name"] == chart.slice_name)
+            assert "extra_editors" not in row
+        finally:
+            db.session.delete(chart)
+            db.session.commit()
+
+    @with_config({"EXTRA_EDITORS_RESOLVER": lambda resource: [123]})
+    def test_get_charts_list_includes_extra_editors_when_resolver_configured(self):
+        """List rows get extra_editors too, mirroring the single-object GET."""
+        admin = self.get_user("admin")
+        chart = self.insert_chart(
+            "extra_editors_list_chart", [admin.id], 1, params="{}"
+        )
+        try:
+            self.login(ADMIN_USERNAME)
+            rv = self.client.get("api/v1/chart/")
+            assert rv.status_code == 200
+            data = json.loads(rv.data.decode("utf-8"))
+            row = next(c for c in data["result"] if c["slice_name"] == chart.slice_name)
+            assert row["extra_editors"] == [123]
+        finally:
+            db.session.delete(chart)
+            db.session.commit()
+
     def test_get_dashboards_filter(self):
         """
         Dashboard API: Test get dashboards filter

tests/unit_tests/common/test_form_data_query_context.py

@@ -218,6 +218,31 @@ def test_orderby_uses_timeseries_limit_metric_and_order_desc() -> None:
     assert query["orderby"] == [["revenue", True]]
 
 
+def test_orderby_uses_series_limit_metric_and_order_desc() -> None:
+    # series_limit_metric is the current field name; timeseries_limit_metric is
+    # the deprecated alias kept above for back-compat with old saved charts.
+    form_data = {
+        "metrics": ["count"],
+        "groupby": ["c"],
+        "series_limit_metric": "revenue",
+        "order_desc": False,
+    }
+    query = build_query_context_from_form_data(form_data, DATASOURCE)["queries"][0]
+    assert query["orderby"] == [["revenue", True]]
+
+
+def test_orderby_prefers_series_limit_metric_over_deprecated_alias() -> None:
+    form_data = {
+        "metrics": ["count"],
+        "groupby": ["c"],
+        "series_limit_metric": "revenue",
+        "timeseries_limit_metric": "profit",
+        "order_desc": False,
+    }
+    query = build_query_context_from_form_data(form_data, DATASOURCE)["queries"][0]
+    assert query["orderby"] == [["revenue", True]]
+
+
 def test_orderby_pie_sort_by_metric() -> None:
     form_data = {"metric": "count", "groupby": ["c"], "sort_by_metric": True}
     query = build_query_context_from_form_data(form_data, DATASOURCE, viz_type="pie")[

tests/unit_tests/tags/commands/create_test.py

@@ -108,75 +108,6 @@ def test_create_command_success(session_with_data: Session, mocker: MockerFixtur
         )
 
 
-def test_validate_object_access_query_malformed_jinja(
-    session_with_data: Session, mocker: MockerFixture
-):
-    """A saved query whose Jinja-templated SQL fails to parse during access
-    checks must surface as a validation error, not an unhandled
-    ``jinja2.TemplateError`` escaping as a 500.
-
-    When ``raise_for_access(query=...)`` authorizes a saved query via
-    per-table permissions it parses the query's Jinja SQL (e.g. an unclosed
-    ``{% if %}`` block raises ``TemplateSyntaxError``). Mock that call to raise
-    the ``TemplateError`` directly so the test stays hermetic and does not open
-    a live DB connection to introspect table-level perms.
-    """
-    from jinja2.exceptions import TemplateError
-
-    from superset.commands.tag.create import CreateCustomTagCommand
-    from superset.commands.tag.exceptions import TagInvalidError
-    from superset.models.sql_lab import SavedQuery
-    from superset.tags.models import ObjectType
-
-    query = db.session.query(SavedQuery).first()
-
-    mocker.patch("superset.commands.tag.create.to_object_model", return_value=query)
-    mocker.patch(
-        "superset.commands.tag.create.security_manager.raise_for_access",
-        side_effect=TemplateError("unclosed {% if %}"),
-    )
-
-    command = CreateCustomTagCommand(ObjectType.query, query.id, ["tag"])
-
-    with pytest.raises(TagInvalidError):
-        command.validate()
-
-
-def test_validate_object_access_query_unresolvable_partition_macro(
-    session_with_data: Session, mocker: MockerFixture
-):
-    """A saved query whose partition macro cannot be resolved statically raises
-    ``SupersetParseError`` during access checks. Like ``TemplateError``, it is a
-    sibling of ``SupersetSecurityException`` under ``SupersetErrorException`` and
-    would otherwise escape as an unhandled 500, so it must also surface as a
-    validation error.
-
-    Mock ``raise_for_access`` to raise the error directly so the test stays
-    hermetic and does not open a live DB connection to introspect table perms.
-    """
-    from superset.commands.tag.create import CreateCustomTagCommand
-    from superset.commands.tag.exceptions import TagInvalidError
-    from superset.exceptions import SupersetParseError
-    from superset.models.sql_lab import SavedQuery
-    from superset.tags.models import ObjectType
-
-    query = db.session.query(SavedQuery).first()
-
-    mocker.patch("superset.commands.tag.create.to_object_model", return_value=query)
-    mocker.patch(
-        "superset.commands.tag.create.security_manager.raise_for_access",
-        side_effect=SupersetParseError(
-            sql="select * from {{ latest_partition('foo') }}",
-            message="Unresolvable partition macro",
-        ),
-    )
-
-    command = CreateCustomTagCommand(ObjectType.query, query.id, ["tag"])
-
-    with pytest.raises(TagInvalidError):
-        command.validate()
-
-
 def test_create_command_success_clear(
     session_with_data: Session, mocker: MockerFixture
 ):