fix: record history rendering issues (#1064)

* fix: record history rendering crash * fix: select editor rendering * fix: duplicate record API return value
teableio · Nov 7, 2024 · ea7b2d2 · ea7b2d2
1 parent 4bd5d03
commit ea7b2d2
Show file tree

Hide file tree

Showing 11 changed files with 102 additions and 31 deletions.
diff --git a/apps/nestjs-backend/src/event-emitter/events/table/record.event.ts b/apps/nestjs-backend/src/event-emitter/events/table/record.event.ts
@@ -1,6 +1,5 @@
-import type { IRecord } from '@teable/core';
+import type { IFieldVo, IRecord } from '@teable/core';
 import { match } from 'ts-pattern';
-import type { IFieldInstance } from '../../../features/field/model/factory';
 import { RawOpType } from '../../../share-db/interface';
 import type { IEventContext } from '../core-event';
 import { Events } from '../event.enum';
@@ -16,7 +15,7 @@ type IRecordDeletePayload = { tableId: string; recordId: string | string[] };
 type IRecordUpdatePayload = {
   tableId: string;
   record: IChangeRecord | IChangeRecord[];
-  oldField: IFieldInstance | undefined;
+  oldField: IFieldVo | undefined;
 };
 
 export class RecordCreateEvent extends OpEvent<IRecordCreatePayload> {
@@ -44,7 +43,7 @@ export class RecordUpdateEvent extends OpEvent<IRecordUpdatePayload> {
   constructor(
     tableId: string,
     record: IChangeRecord | IChangeRecord[],
-    oldField: IFieldInstance | undefined,
+    oldField: IFieldVo | undefined,
     context: IEventContext
   ) {
     super({ tableId, record, oldField }, context, Array.isArray(record));

diff --git a/apps/nestjs-backend/src/features/field/open-api/field-open-api.service.ts b/apps/nestjs-backend/src/features/field/open-api/field-open-api.service.ts
@@ -441,7 +441,6 @@ export class FieldOpenApiService {
     // stage analysis and collect field changes
     const { newField, oldField, modifiedOps, supplementChange, references } =
       await this.fieldConvertingService.stageAnalysis(tableId, fieldId, updateFieldRo);
-    this.cls.set('oldField', oldField);
 
     await this.performConvertField({
       tableId,
@@ -454,6 +453,8 @@ export class FieldOpenApiService {
     const oldFieldVo = instanceToPlain(oldField, { excludePrefixes: ['_'] }) as IFieldVo;
     const newFieldVo = instanceToPlain(newField, { excludePrefixes: ['_'] }) as IFieldVo;
 
+    this.cls.set('oldField', oldFieldVo);
+
     if (windowId) {
       this.eventEmitterService.emitAsync(Events.OPERATION_FIELD_CONVERT, {
         windowId,

diff --git a/apps/nestjs-backend/src/features/record/open-api/record-open-api.service.ts b/apps/nestjs-backend/src/features/record/open-api/record-open-api.service.ts
@@ -585,6 +585,6 @@ export class RecordOpenApiService {
     const createdRecords = await this.prismaService.$tx(async () =>
       this.createRecords(tableId, createRecordsRo)
     );
-    return { ids: createdRecords.records.map((record) => record.id) };
+    return { id: createdRecords.records[0]?.id };
   }
 }
diff --git a/apps/nestjs-backend/src/types/cls.ts b/apps/nestjs-backend/src/types/cls.ts
@@ -1,7 +1,6 @@
-import type { Action } from '@teable/core';
+import type { Action, IFieldVo } from '@teable/core';
 import type { Prisma } from '@teable/db-main-prisma';
 import type { ClsStore } from 'nestjs-cls';
-import type { IFieldInstance } from '../features/field/model/factory';
 import type { IRawOpMap } from '../share-db/interface';
 
 export interface IClsStore extends ClsStore {
@@ -26,5 +25,5 @@ export interface IClsStore extends ClsStore {
   permissions: Action[];
   // for share db adapter
   cookie?: string;
-  oldField?: IFieldInstance;
+  oldField?: IFieldVo;
 }
diff --git a/apps/nestjs-backend/test/record.e2e-spec.ts b/apps/nestjs-backend/test/record.e2e-spec.ts
@@ -303,7 +303,7 @@ describe('OpenAPI RecordController (e2e)', () => {
         anchorId: addRecord.id,
         position: 'after',
       });
-      const record = await getRecord(table.id, duplicateRes.ids[0], undefined, 200);
+      const record = await getRecord(table.id, duplicateRes.id, undefined, 200);
       expect(record.fields[table.fields[0].id]).toEqual(value1);
     });
   });

diff --git a/packages/core/src/models/field/cell-value-validation.ts b/packages/core/src/models/field/cell-value-validation.ts
@@ -0,0 +1,63 @@
+import { z } from 'zod';
+import { assertNever } from '../../asserts';
+import { FieldType } from './constant';
+import {
+  attachmentCellValueSchema,
+  autoNumberCellValueSchema,
+  dataFieldCellValueSchema,
+  getFormulaCellValueSchema,
+  linkCellValueSchema,
+  numberCellValueSchema,
+  singleLineTextCelValueSchema,
+  userCellValueSchema,
+} from './derivate';
+import type { IFieldVo } from './field.schema';
+
+const validateWithSchema = (schema: z.ZodType, value: unknown) => {
+  return z
+    .union([z.array(schema).nonempty(), schema])
+    .nullable()
+    .safeParse(value);
+};
+
+export const validateCellValue = (field: IFieldVo, cellValue: unknown) => {
+  const { type, cellValueType } = field;
+
+  switch (type) {
+    case FieldType.LongText:
+    case FieldType.SingleLineText:
+    case FieldType.SingleSelect:
+    case FieldType.MultipleSelect:
+      return validateWithSchema(singleLineTextCelValueSchema, cellValue);
+    case FieldType.Number:
+      return validateWithSchema(numberCellValueSchema, cellValue);
+    case FieldType.Rating:
+    case FieldType.AutoNumber:
+      return validateWithSchema(autoNumberCellValueSchema, cellValue);
+    case FieldType.Attachment:
+      return attachmentCellValueSchema.nonempty().nullable().safeParse(cellValue);
+    case FieldType.Date:
+    case FieldType.CreatedTime:
+    case FieldType.LastModifiedTime:
+      return validateWithSchema(dataFieldCellValueSchema, cellValue);
+    case FieldType.Checkbox:
+      return validateWithSchema(z.literal(true), cellValue);
+    case FieldType.Link:
+      return validateWithSchema(linkCellValueSchema, cellValue);
+    case FieldType.User:
+    case FieldType.CreatedBy:
+    case FieldType.LastModifiedBy:
+      return validateWithSchema(userCellValueSchema, cellValue);
+    case FieldType.Rollup:
+    case FieldType.Formula: {
+      const schema = getFormulaCellValueSchema(cellValueType);
+      return validateWithSchema(schema, cellValue);
+    }
+    case FieldType.Button:
+    case FieldType.Count:
+    case FieldType.Duration:
+      throw new Error('did not implement yet');
+    default:
+      assertNever(type);
+  }
+};
diff --git a/packages/core/src/models/field/derivate/abstract/formula.field.abstract.ts b/packages/core/src/models/field/derivate/abstract/formula.field.abstract.ts
@@ -21,6 +21,21 @@ import { dataFieldCellValueSchema } from '../date.field';
 import { numberCellValueSchema } from '../number.field';
 import { singleLineTextCelValueSchema } from '../single-line-text.field';
 
+export const getFormulaCellValueSchema = (cellValueType: CellValueType) => {
+  switch (cellValueType) {
+    case CellValueType.Number:
+      return numberCellValueSchema;
+    case CellValueType.DateTime:
+      return dataFieldCellValueSchema;
+    case CellValueType.String:
+      return singleLineTextCelValueSchema;
+    case CellValueType.Boolean:
+      return booleanCellValueSchema;
+    default:
+      assertNever(cellValueType);
+  }
+};
+
 export abstract class FormulaAbstractCore extends FieldCore {
   static parse(expression: string) {
     const inputStream = CharStreams.fromString(expression);
@@ -98,21 +113,7 @@ export abstract class FormulaAbstractCore extends FieldCore {
   }
 
   validateCellValue(value: unknown) {
-    const getFormulaCellValueSchema = () => {
-      switch (this.cellValueType) {
-        case CellValueType.Number:
-          return numberCellValueSchema;
-        case CellValueType.DateTime:
-          return dataFieldCellValueSchema;
-        case CellValueType.String:
-          return singleLineTextCelValueSchema;
-        case CellValueType.Boolean:
-          return booleanCellValueSchema;
-        default:
-          assertNever(this.cellValueType);
-      }
-    };
-    const schema = getFormulaCellValueSchema();
+    const schema = getFormulaCellValueSchema(this.cellValueType);
 
     if (this.isMultipleCellValue) {
       return z.array(schema).nullable().safeParse(value);

diff --git a/packages/core/src/models/field/index.ts b/packages/core/src/models/field/index.ts
@@ -7,3 +7,4 @@ export * from './formatting';
 export * from './show-as';
 export * from './field.schema';
 export * from './field-validation';
+export * from './cell-value-validation';
diff --git a/packages/openapi/src/record/duplicate.ts b/packages/openapi/src/record/duplicate.ts
@@ -7,7 +7,7 @@ import { recordInsertOrderRoSchema } from './create';
 export const DUPLICATE_URL = '/table/{tableId}/record/{recordId}';
 
 export const duplicateVoSchema = z.object({
-  ids: z.array(z.string()),
+  id: z.string(),
 });
 
 export type IDuplicateVo = z.infer<typeof duplicateVoSchema>;

diff --git a/packages/sdk/src/components/editor/select/components/OptionList.tsx b/packages/sdk/src/components/editor/select/components/OptionList.tsx
@@ -54,11 +54,12 @@ export const OptionList = (props: IOptionListProps) => {
               onSelect={() => onSelect?.(value)}
             >
               <SelectTag
+                className="truncate"
                 label={label || t('common.untitled')}
                 backgroundColor={backgroundColor}
                 color={color}
               />
-              {checkIsActive(value) && <Check className="ml-2 size-4" />}
+              {checkIsActive(value) && <Check className="ml-2 size-4 shrink-0" />}
             </CommandItem>
           );
         })}

diff --git a/packages/sdk/src/components/expand-record/RecordHistory.tsx b/packages/sdk/src/components/expand-record/RecordHistory.tsx
@@ -1,6 +1,8 @@
 import type { QueryFunctionContext } from '@tanstack/react-query';
 import { useInfiniteQuery } from '@tanstack/react-query';
 import type { ColumnDef } from '@tanstack/react-table';
+import type { IFieldVo } from '@teable/core';
+import { validateCellValue } from '@teable/core';
 import { ArrowRight, ChevronRight } from '@teable/icons';
 import type { IRecordHistoryItemVo, IRecordHistoryVo } from '@teable/openapi';
 import { getRecordHistory, getRecordListHistory } from '@teable/openapi';
@@ -114,11 +116,13 @@ export const RecordHistory = (props: IRecordHistoryProps) => {
         size: actionVisible ? 220 : 280,
         cell: ({ row }) => {
           const before = row.getValue<IRecordHistoryItemVo['before']>('before');
+          const validatedCellValue = validateCellValue(before.meta as IFieldVo, before.data);
+          const cellValue = validatedCellValue.success ? validatedCellValue.data : undefined;
           return (
             <Fragment>
-              {before.data != null ? (
+              {cellValue != null ? (
                 <CellValue
-                  value={before.data}
+                  value={cellValue}
                   field={before.meta as IFieldInstance}
                   className={actionVisible ? 'max-w-52' : 'max-w-[264px]'}
                 />
@@ -147,11 +151,13 @@ export const RecordHistory = (props: IRecordHistoryProps) => {
         size: actionVisible ? 220 : 280,
         cell: ({ row }) => {
           const after = row.getValue<IRecordHistoryItemVo['after']>('after');
+          const validatedCellValue = validateCellValue(after.meta as IFieldVo, after.data);
+          const cellValue = validatedCellValue.success ? validatedCellValue.data : undefined;
           return (
             <Fragment>
-              {after.data != null ? (
+              {cellValue != null ? (
                 <CellValue
-                  value={after.data}
+                  value={cellValue}
                   field={after.meta as IFieldInstance}
                   className={actionVisible ? 'max-w-52' : 'max-w-[264px]'}
                 />