Cleanup Segment metabase-lib type (#30728)

frontend/src/metabase-lib/metadata/Metric.ts

-// eslint-disable-next-line @typescript-eslint/ban-ts-comment
-// @ts-nocheck
+import { Aggregation, NormalizedMetric } from "metabase-types/api";
 import Filter from "metabase-lib/queries/structured/Filter";
-import Base from "./Base";
 import type Metadata from "./Metadata";
 import type Table from "./Table";
-/**
- * @typedef { import("./Metadata").Aggregation } Aggregation
- */
 
-/**
- * Wrapper class for a metric. Belongs to a {@link Database} and possibly a {@link Table}
- */
+interface Metric extends Omit<NormalizedMetric, "table"> {
+  table?: Table;
+  metadata?: Metadata;
+}
 
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default class Metric extends Base {
-  name: string;
-  table_id: Table["id"];
-  table: Table;
-  metadata: Metadata;
+class Metric {
+  private readonly _plainObject: NormalizedMetric;
+
+  constructor(metric: NormalizedMetric) {
+    this._plainObject = metric;
+    Object.assign(this, metric);
+  }
+
+  getPlainObject() {
+    return this._plainObject;
+  }
 
   displayName() {
     return this.name;
   }
 
-  /**
-   * @returns {Aggregation}
-   */
-  aggregationClause() {
+  aggregationClause(): Aggregation {
     return ["metric", this.id];
   }
 
   /** Underlying query for this metric */
   definitionQuery() {
-    return this.definition
+    return this.table && this.definition
       ? this.table.query().setQuery(this.definition)
       : null;
   }
@@ -66,26 +64,7 @@ export default class Metric extends Base {
   isActive() {
     return !this.archived;
   }
-
-  /**
-   * @private
-   * @param {string} name
-   * @param {string} description
-   * @param {Database} database
-   * @param {Table} table
-   * @param {number} id
-   * @param {StructuredQuery} definition
-   * @param {boolean} archived
-   */
-
-  /* istanbul ignore next */
-  _constructor(name, description, database, table, id, definition, archived) {
-    this.name = name;
-    this.description = description;
-    this.database = database;
-    this.table = table;
-    this.id = id;
-    this.definition = definition;
-    this.archived = archived;
-  }
 }
+
+// eslint-disable-next-line import/no-default-export -- deprecated usage
+export default Metric;

frontend/src/metabase-lib/metadata/Schema.ts

@@ -4,27 +4,22 @@ import type Metadata from "./Metadata";
 import type Database from "./Database";
 import type Table from "./Table";
 
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default class Schema {
-  private readonly schema: NormalizedSchema;
-  metadata?: Metadata;
+interface Schema extends Omit<NormalizedSchema, "database" | "tables"> {
   database?: Database;
-  tables: Table[] = [];
-
-  constructor(schema: NormalizedSchema) {
-    this.schema = schema;
-  }
+  tables?: Table[];
+  metadata?: Metadata;
+}
 
-  get id() {
-    return this.schema.id;
-  }
+class Schema {
+  private readonly _plainObject: NormalizedSchema;
 
-  get name() {
-    return this.schema.name;
+  constructor(schema: NormalizedSchema) {
+    this._plainObject = schema;
+    Object.assign(this, schema);
   }
 
   getPlainObject() {
-    return this.schema;
+    return this._plainObject;
   }
 
   displayName() {
@@ -32,6 +27,9 @@ export default class Schema {
   }
 
   getTables() {
-    return this.tables;
+    return this.tables ?? [];
   }
 }
+
+// eslint-disable-next-line import/no-default-export -- deprecated usage
+export default Schema;

frontend/src/metabase-lib/metadata/Segment.ts

-// eslint-disable-next-line @typescript-eslint/ban-ts-comment
-// @ts-nocheck
-import Base from "./Base";
+import { Filter, NormalizedSegment } from "metabase-types/api";
 import type Metadata from "./Metadata";
 import type Table from "./Table";
-/**
- * @typedef { import("./Metadata").FilterClause } FilterClause
- */
 
-/**
- * Wrapper class for a segment. Belongs to a {@link Database} and possibly a {@link Table}
- */
+interface Segment extends Omit<NormalizedSegment, "table"> {
+  table?: Table;
+  metadata?: Metadata;
+}
 
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default class Segment extends Base {
-  id: number;
-  name: string;
-  table_id: Table["id"];
-  table: Table;
-  metadata: Metadata;
+class Segment {
+  private readonly _plainObject: NormalizedSegment;
+
+  constructor(segment: NormalizedSegment) {
+    this._plainObject = segment;
+    Object.assign(this, segment);
+  }
+
+  getPlainObject() {
+    return this._plainObject;
+  }
 
   displayName() {
     return this.name;
   }
 
-  /**
-   * @returns {FilterClause}
-   */
-  filterClause() {
+  filterClause(): Filter {
     return ["segment", this.id];
   }
 
   isActive() {
     return !this.archived;
   }
-
-  /**
-   * @private
-   * @param {string} name
-   * @param {string} description
-   * @param {Database} database
-   * @param {Table} table
-   * @param {number} id
-   * @param {boolean} archived
-   */
-
-  /* istanbul ignore next */
-  _constructor(name, description, database, table, id, archived) {
-    this.name = name;
-    this.description = description;
-    this.database = database;
-    this.table = table;
-    this.id = id;
-    this.archived = archived;
-  }
 }
+
+// eslint-disable-next-line import/no-default-export -- deprecated usage
+export default Segment;

frontend/src/metabase-lib/metadata/Segment.unit.spec.ts

 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 // @ts-nocheck
 import Segment from "./Segment";
-import Base from "./Base";
+
 describe("Segment", () => {
   describe("instantiation", () => {
     it("should create an instance of Segment", () => {
       expect(new Segment()).toBeInstanceOf(Segment);
     });
-    it("should add `object` props to the instance (because it extends Base)", () => {
-      expect(new Segment()).toBeInstanceOf(Base);
-      expect(
-        new Segment({
-          foo: "bar",
-        }),
-      ).toHaveProperty("foo", "bar");
-    });
   });
   describe("displayName", () => {
     it("should return the `name` property found on the instance", () => {

frontend/src/metabase-types/api/metric.ts

 import { StructuredQuery } from "./query";
 import { TableId } from "./table";
 
-export type MetricId = number;
+export type MetricId = number | string;
 
 export interface Metric {
   id: MetricId;

frontend/src/metabase-types/api/mocks/segment.ts

@@ -8,5 +8,6 @@ export const createMockSegment = (opts?: Partial<Segment>): Segment => ({
   table_id: 1,
   archived: false,
   definition: createMockStructuredQuery(),
+  definition_description: "",
   ...opts,
 });

frontend/src/metabase-types/api/segment.ts

@@ -10,5 +10,6 @@ export interface Segment {
   table_id: TableId;
   archived: boolean;
   definition: StructuredQuery;
+  definition_description: string;
   revision_message?: string;
 }

frontend/src/metabase/admin/datamodel/components/MetricItem.jsx

@@ -17,7 +17,7 @@ export default class MetricItem extends Component {
       <tr>
         <td className="px1 py1 text-wrap">
           <span className="flex align-center">
-            <Icon {...metric.getIcon()} size={12} className="mr1 text-medium" />
+            <Icon name="sum" size={12} className="mr1 text-medium" />
             <span className="text-dark text-bold">{metric.name}</span>
           </span>
         </td>

frontend/src/metabase/admin/datamodel/components/SegmentItem.jsx

@@ -17,11 +17,7 @@ export default class SegmentItem extends Component {
       <tr className="mt1 mb3">
         <td className="px1 py1 text-wrap">
           <span className="flex align-center">
-            <Icon
-              {...segment.getIcon()}
-              size={12}
-              className="mr1 text-medium"
-            />
+            <Icon name="segment" size={12} className="mr1 text-medium" />
             <span className="text-dark text-bold">{segment.name}</span>
           </span>
         </td>

frontend/src/metabase/admin/datamodel/containers/MetricListApp.jsx

 /* eslint-disable react/prop-types */
 import React from "react";
+import { connect } from "react-redux";
 import { t } from "ttag";
 import _ from "underscore";
 
@@ -12,7 +13,7 @@ import Link from "metabase/core/components/Link";
 
 class MetricListAppInner extends React.Component {
   render() {
-    const { metrics, tableSelector } = this.props;
+    const { metrics, tableSelector, setArchived } = this.props;
 
     return (
       <div className="px3 pb2">
@@ -34,7 +35,7 @@ class MetricListAppInner extends React.Component {
             {metrics.map(metric => (
               <MetricItem
                 key={metric.id}
-                onRetire={() => metric.setArchived(true)}
+                onRetire={() => setArchived(metric, true)}
                 metric={metric}
               />
             ))}
@@ -51,8 +52,9 @@ class MetricListAppInner extends React.Component {
 }
 
 const MetricListApp = _.compose(
-  Metrics.loadList({ wrapped: true }),
+  Metrics.loadList(),
   FilteredToUrlTable("metrics"),
+  connect(null, { setArchived: Metrics.actions.setArchived }),
 )(MetricListAppInner);
 
 export default MetricListApp;

frontend/src/metabase/admin/datamodel/containers/SegmentListApp.jsx

 /* eslint-disable react/prop-types */
 import React from "react";
+import { connect } from "react-redux";
 import { t } from "ttag";
 import _ from "underscore";
 
-import Segment from "metabase/entities/segments";
+import Segments from "metabase/entities/segments";
 import SegmentItem from "metabase/admin/datamodel/components/SegmentItem";
 import FilteredToUrlTable from "metabase/admin/datamodel/hoc/FilteredToUrlTable";
 
@@ -12,7 +13,7 @@ import Link from "metabase/core/components/Link";
 
 class SegmentListAppInner extends React.Component {
   render() {
-    const { segments, tableSelector } = this.props;
+    const { segments, tableSelector, setArchived } = this.props;
 
     return (
       <div className="px3 pb2">
@@ -34,7 +35,7 @@ class SegmentListAppInner extends React.Component {
             {segments.map(segment => (
               <SegmentItem
                 key={segment.id}
-                onRetire={() => segment.setArchived(true)}
+                onRetire={() => setArchived(segment, true)}
                 segment={segment}
               />
             ))}
@@ -51,8 +52,9 @@ class SegmentListAppInner extends React.Component {
 }
 
 const SegmentListApp = _.compose(
-  Segment.loadList({ wrapped: true }),
+  Segments.loadList(),
   FilteredToUrlTable("segments"),
+  connect(null, { setArchived: Segments.actions.setArchived }),
 )(SegmentListAppInner);
 
 export default SegmentListApp;

frontend/src/metabase/admin/permissions/utils/graph/data-permissions.ts

@@ -353,7 +353,7 @@ export function updateTablesPermission(
   downgradeNative?: boolean,
 ) {
   const schema = database.schema(schemaName);
-  const tableIds = schema?.tables.map((t: Table) => t.id);
+  const tableIds = schema?.getTables().map((t: Table) => t.id);
 
   permissions = updateSchemasPermission(
     permissions,

frontend/src/metabase/common/hooks/index.ts

 export * from "./use-database-id-field-list-query";
 export * from "./use-database-list-query";
 export * from "./use-database-query";
+export * from "./use-metric-list-query";
+export * from "./use-metric-query";
 export * from "./use-schema-list-query";
+export * from "./use-segment-list-query";
+export * from "./use-segment-query";
 export * from "./use-table-list-query";
 export * from "./use-table-query";

frontend/src/metabase/common/hooks/use-database-id-field-list-query/use-database-id-field-list-query.ts

@@ -11,10 +11,11 @@ export const useDatabaseIdFieldListQuery = (
   props: UseEntityQueryProps<DatabaseId, DatabaseIdFieldListQuery>,
 ): UseEntityQueryResult<Field[]> => {
   return useEntityQuery(props, {
-    fetch: Databases.actions.fetchIdfields,
-    getObject: Databases.selectors.getIdfields,
+    fetch: Databases.actions.fetchIdFields,
+    getObject: state =>
+      Databases.selectors.getIdFields(state, { databaseId: props.id }),
     getLoading: Databases.selectors.getLoading,
     getError: Databases.selectors.getError,
-    requestType: "idfields",
+    requestType: "idFields",
   });
 };

frontend/src/metabase/common/hooks/use-database-id-field-list-query/use-database-id-field-list-query.unit.spec.tsx

+import React from "react";
+import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper";
+import { createSampleDatabase } from "metabase-types/api/mocks/presets";
+import { setupDatabasesEndpoints } from "__support__/server-mocks";
+import {
+  renderWithProviders,
+  screen,
+  waitForElementToBeRemoved,
+} from "__support__/ui";
+import { useDatabaseIdFieldListQuery } from "./use-database-id-field-list-query";
+
+const TEST_DB = createSampleDatabase();
+
+const TestComponent = () => {
+  const {
+    data = [],
+    isLoading,
+    error,
+  } = useDatabaseIdFieldListQuery({ id: TEST_DB.id });
+
+  if (isLoading || error) {
+    return <LoadingAndErrorWrapper loading={isLoading} error={error} />;
+  }
+
+  return (
+    <div>
+      {data.map(field => (
+        <div key={field.getId()}>
+          {field.displayName({ includeTable: true })}
+        </div>
+      ))}
+    </div>
+  );
+};
+
+const setup = () => {
+  setupDatabasesEndpoints([TEST_DB]);
+  renderWithProviders(<TestComponent />);
+};
+
+describe("useDatabaseIdFieldListQuery", () => {
+  it("should be initially loading", () => {
+    setup();
+    expect(screen.getByText("Loading...")).toBeInTheDocument();
+  });
+
+  it("should show data from the response", async () => {
+    setup();
+    await waitForElementToBeRemoved(() => screen.queryByText("Loading..."));
+    expect(screen.getByText("Orders → ID")).toBeInTheDocument();
+  });
+});

frontend/src/metabase/common/hooks/use-entity-list-query/use-entity-list-query.ts

@@ -7,11 +7,11 @@ export interface EntityFetchOptions {
   reload?: boolean;
 }
 
-export interface EntityQueryOptions<TQuery> {
+export interface EntityQueryOptions<TQuery = never> {
   entityQuery?: TQuery;
 }
 
-export interface UseEntityListOwnProps<TItem, TQuery> {
+export interface UseEntityListOwnProps<TItem, TQuery = never> {
   fetchList: (query?: TQuery, options?: EntityFetchOptions) => Action;
   getList: (
     state: State,
@@ -21,7 +21,7 @@ export interface UseEntityListOwnProps<TItem, TQuery> {
   getError: (state: State, options: EntityQueryOptions<TQuery>) => unknown;
 }
 
-export interface UseEntityListQueryProps<TQuery> {
+export interface UseEntityListQueryProps<TQuery = never> {
   query?: TQuery;
   reload?: boolean;
   enabled?: boolean;
@@ -33,7 +33,7 @@ export interface UseEntityListQueryResult<TItem> {
   error: unknown;
 }
 
-export const useEntityListQuery = <TItem, TQuery>(
+export const useEntityListQuery = <TItem, TQuery = never>(
   {
     query: entityQuery,
     reload = false,

frontend/src/metabase/common/hooks/use-entity-query/use-entity-query.ts

@@ -27,7 +27,7 @@ export interface UseEntityOwnProps<TId, TItem> {
   requestType?: string;
 }
 
-export interface UseEntityQueryProps<TId, TQuery> {
+export interface UseEntityQueryProps<TId, TQuery = never> {
   id?: TId;
   query?: TQuery;
   reload?: boolean;
@@ -40,7 +40,7 @@ export interface UseEntityQueryResult<TItem> {
   error: unknown;
 }
 
-export const useEntityQuery = <TId, TItem, TQuery>(
+export const useEntityQuery = <TId, TItem, TQuery = never>(
   {
     id: entityId,
     query: entityQuery,

frontend/src/metabase/common/hooks/use-metric-list-query/index.ts

+export * from "./use-metric-list-query";

frontend/src/metabase/common/hooks/use-metric-list-query/use-metric-list-query.ts

+import Metrics from "metabase/entities/metrics";
+import {
+  useEntityListQuery,
+  UseEntityListQueryProps,
+  UseEntityListQueryResult,
+} from "metabase/common/hooks/use-entity-list-query";
+import Metric from "metabase-lib/metadata/Metric";
+
+export const useMetricListQuery = (
+  props: UseEntityListQueryProps = {},
+): UseEntityListQueryResult<Metric> => {
+  return useEntityListQuery(props, {
+    fetchList: Metrics.actions.fetchList,
+    getList: Metrics.selectors.getList,
+    getLoading: Metrics.selectors.getLoading,
+    getError: Metrics.selectors.getError,
+  });
+};

frontend/src/metabase/common/hooks/use-metric-list-query/use-metric-list-query.unit.spec.tsx

+import React from "react";
+import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper";
+import { createMockMetric } from "metabase-types/api/mocks";
+import { setupMetricsEndpoints } from "__support__/server-mocks";
+import {
+  renderWithProviders,
+  screen,
+  waitForElementToBeRemoved,
+} from "__support__/ui";
+import { useMetricListQuery } from "./use-metric-list-query";
+
+const TEST_METRIC = createMockMetric();
+
+const TestComponent = () => {
+  const { data = [], isLoading, error } = useMetricListQuery();
+
+  if (isLoading || error) {
+    return <LoadingAndErrorWrapper loading={isLoading} error={error} />;
+  }
+
+  return (
+    <div>
+      {data.map(metric => (
+        <div key={metric.id}>{metric.name}</div>
+      ))}
+    </div>
+  );
+};
+
+const setup = () => {
+  setupMetricsEndpoints([TEST_METRIC]);
+  renderWithProviders(<TestComponent />);
+};
+
+describe("useMetricListQuery", () => {
+  it("should be initially loading", () => {
+    setup();
+    expect(screen.getByText("Loading...")).toBeInTheDocument();
+  });
+
+  it("should show data from the response", async () => {
+    setup();
+    await waitForElementToBeRemoved(() => screen.queryByText("Loading..."));
+    expect(screen.getByText(TEST_METRIC.name)).toBeInTheDocument();
+  });
+});