Merge pull request #5102 from commonspider/4982_replace_number

Replace Number() with a util function for casting column ids from string to number

Author: pavish

mathesar_ui/src/component-library/common/utils/typeUtils.ts

@@ -22,6 +22,13 @@ export function hasStringProperty<PropertyName extends string>(
   return hasProperty(object, property) && typeof object[property] === 'string';
 }
 
+export function hasNumberProperty<PropertyName extends string>(
+  object: unknown,
+  property: PropertyName,
+): object is { [k in PropertyName]: number } {
+  return hasProperty(object, property) && typeof object[property] === 'number';
+}
+
 export function hasMethod<MethodName extends string>(
   object: unknown,
   method: MethodName,

mathesar_ui/src/stores/table-data/filtering.ts

@@ -13,6 +13,7 @@ import type {
   RawFilterGroup,
   RawIndividualFilter,
 } from '@mathesar/components/filter/utils';
+import { castColumnIdToNumber } from '@mathesar/utils/columnUtils';
 
 import type { FilterId } from '../abstract-types/types';
 
@@ -45,7 +46,7 @@ function individualFilterToSqlExpr(
 ): SqlExpr {
   const column: SqlColumn = {
     type: 'attnum',
-    value: Number(individualFilter.columnId),
+    value: castColumnIdToNumber(individualFilter.columnId),
   };
 
   /** Generate an SqlLiteral value */
@@ -190,7 +191,7 @@ function getCountOfNonConjunctionalExpr(expr: SqlExpr) {
 }
 
 function getCountOfColumnInExpr(expr: SqlExpr, columnId: string): number {
-  if (expr.type === 'attnum' && expr.value === Number(columnId)) {
+  if (expr.type === 'attnum' && expr.value === castColumnIdToNumber(columnId)) {
     return 1;
   }
   let count = 0;

mathesar_ui/src/stores/table-data/grouping.ts

@@ -1,4 +1,5 @@
 import type { RecordsListParams } from '@mathesar/api/rpc/records';
+import { castColumnIdToNumber } from '@mathesar/utils/columnUtils';
 
 export interface GroupEntry {
   readonly columnId: string;
@@ -78,7 +79,7 @@ export class Grouping {
     }
     return {
       grouping: {
-        columns: this.entries.map((e) => Number(e.columnId)),
+        columns: this.entries.map((e) => castColumnIdToNumber(e.columnId)),
         preproc: this.entries.map((e) => e.preprocFnId ?? null),
       },
     };

mathesar_ui/src/stores/table-data/joinedColumns.ts

@@ -1,6 +1,7 @@
 import type { RawColumnWithMetadata } from '@mathesar/api/rpc/columns';
 import type { JoinPath, JoinableTablesResult } from '@mathesar/api/rpc/tables';
 import { getCellCap } from '@mathesar/components/cell-fabric/utils';
+import { castColumnIdToNumber } from '@mathesar/utils/columnUtils';
 import type { ComponentAndProps } from '@mathesar-component-library/types';
 
 import type { Joining } from './joining';
@@ -82,7 +83,7 @@ export class SimpleManyToManyJoinedColumn {
 
         const [attnum, info] = pkColumnEntry;
         const joinedColumn = {
-          id: Number(attnum),
+          id: castColumnIdToNumber(attnum),
           type: '_array',
           type_options: {
             item_type: info.type,

mathesar_ui/src/stores/table-data/searchFuzzy.ts

@@ -1,5 +1,6 @@
 import type { RecordsSearchParams } from '@mathesar/api/rpc/records';
 import { ImmutableMap } from '@mathesar/component-library';
+import { castColumnIdToNumber } from '@mathesar/utils/columnUtils';
 
 /**
  * Due to the way that the fuzzy search works, it doesn't make sense to search
@@ -20,7 +21,7 @@ export class SearchFuzzy extends ImmutableMap<string, unknown> {
 
   getSearchParams(): RecordsSearchParams['search_params'] {
     return [...this].map(([columnId, value]) => ({
-      attnum: Number(columnId),
+      attnum: castColumnIdToNumber(columnId),
       literal: value,
     }));
   }

mathesar_ui/src/stores/table-data/sorting.ts

@@ -7,6 +7,7 @@ import {
   type SortDirection,
   allowedSortDirections,
 } from '@mathesar/components/sort-entry/utils';
+import { castColumnIdToNumber } from '@mathesar/utils/columnUtils';
 import { ImmutableMap } from '@mathesar-component-library';
 
 import type { Grouping } from './grouping';
@@ -48,7 +49,7 @@ export class Sorting extends ImmutableMap<string, SortDirection> {
   private recordsRequestParams(): Pick<RecordsListParams, 'order'> {
     const sortingEntries: ApiSortingEntry[] = [...this].map(
       ([columnId, sortDirection]) => ({
-        attnum: Number(columnId),
+        attnum: castColumnIdToNumber(columnId),
         direction: getApiSortDirection(sortDirection),
       }),
     );

mathesar_ui/src/stores/table-data/tabularData.ts

@@ -29,6 +29,7 @@ import type {
   RecordRow,
   RecordSummariesForSheet,
 } from '@mathesar/stores/table-data';
+import { castColumnIdToNumber } from '@mathesar/utils/columnUtils';
 import { orderProcessedColumns } from '@mathesar/utils/tables';
 import { ImmutableSet, defined } from '@mathesar-component-library';
 
@@ -172,7 +173,7 @@ export class TabularData {
       database: props.database,
       table: this.table,
       hiddenColumns: Array.from(contextualFilters.keys()).map((id) =>
-        Number(id),
+        castColumnIdToNumber(id),
       ),
     });
     this.constraintsDataStore = new ConstraintsDataStore({

mathesar_ui/src/systems/data-explorer/result-pane/QueryRunErrors.svelte

@@ -7,7 +7,8 @@
   import { getExplorationPageUrl } from '@mathesar/routes/urls';
   import { databasesStore } from '@mathesar/stores/databases';
   import { currentSchema } from '@mathesar/stores/schemas';
-  import { Button, hasProperty } from '@mathesar-component-library';
+  import { castColumnIdToNumber } from '@mathesar/utils/columnUtils';
+  import { Button, hasNumberProperty } from '@mathesar-component-library';
 
   import QueryManager from '../QueryManager';
   import type { QueryRunner } from '../QueryRunner';
@@ -34,8 +35,8 @@
     {#if errors instanceof ApiMultiError}
       {#each errors.errors as apierror}
         <ul>
-          {#if apierror.code === QUERY_CONTAINS_DELETED_COLUMN && hasProperty(apierror.detail, 'column_id')}
-            {@const columnId = Number(apierror.detail.column_id)}
+          {#if apierror.code === QUERY_CONTAINS_DELETED_COLUMN && hasNumberProperty(apierror.detail, 'column_id')}
+            {@const columnId = castColumnIdToNumber(apierror.detail.column_id)}
             <li class="error">
               <p class="strong">
                 {$_('some_columns_in_query_missing')}

mathesar_ui/src/systems/table-view/actions-pane/record-operations/join/joinConfigUtils.ts

@@ -3,6 +3,7 @@ import type {
   JoinableTable,
   JoinableTablesResult,
 } from '@mathesar/api/rpc/tables';
+import { castColumnIdToNumber } from '@mathesar/utils/columnUtils';
 
 /**
  * This represents a "simple" many-to-many relationship such that the mapping
@@ -151,7 +152,7 @@ export function getSimpleManyToManyJoinPath(
     ],
     [
       [intermediateTableOid, intermediateTableFkToTargetColumn],
-      [targetTableOid, Number(targetTablePkColumn)],
+      [targetTableOid, castColumnIdToNumber(targetTablePkColumn)],
     ],
   ];
 }

mathesar_ui/src/utils/columnUtils.ts

@@ -101,3 +101,12 @@ export function columnTypeOptionsAreEqual(
   }
   return true;
 }
+
+export function castColumnIdToNumber(columnId: string | number) {
+  const numericId = columnId === '' ? NaN : Number(columnId);
+  if (Number.isNaN(numericId)) {
+    throw new Error('Invalid columnId');
+  } else {
+    return numericId;
+  }
+}