Browse Source

fix: default system column creation and test corrections

pull/7304/head
Pranav C 9 months ago
parent
commit
d48a512334
  1. 26
      packages/nocodb/src/services/tables.service.ts
  2. 6
      packages/nocodb/tests/unit/factory/column.ts
  3. 52
      packages/nocodb/tests/unit/model/tests/baseModelSql.test.ts

26
packages/nocodb/src/services/tables.service.ts

@ -2,6 +2,7 @@ import { Injectable } from '@nestjs/common';
import DOMPurify from 'isomorphic-dompurify'; import DOMPurify from 'isomorphic-dompurify';
import { import {
AppEvents, AppEvents,
isCreatedTimeOrUpdatedTimeCol,
isLinksOrLTAR, isLinksOrLTAR,
isVirtualCol, isVirtualCol,
ModelTypes, ModelTypes,
@ -382,9 +383,8 @@ export class TablesService {
// add CreatedBy and LastModifiedBy system columns if missing in request payload // add CreatedBy and LastModifiedBy system columns if missing in request payload
{ {
for (const uidt of [UITypes.CreatedTime, UITypes.LastModifiedTime]) { for (const uidt of [UITypes.CreatedTime, UITypes.LastModifiedTime]) {
if ( const col = tableCreatePayLoad.columns.find((c) => c.uidt === uidt);
!tableCreatePayLoad.columns.find((c) => c.uidt === uidt && c.system) if (!col || !col.system) {
) {
tableCreatePayLoad.columns.push({ tableCreatePayLoad.columns.push({
...(await getColumnPropsFromUIDT({ uidt } as any, source)), ...(await getColumnPropsFromUIDT({ uidt } as any, source)),
system: true, system: true,
@ -472,7 +472,10 @@ export class TablesService {
const uniqueColumnNameCount = {}; const uniqueColumnNameCount = {};
for (const column of param.table.columns) { for (const column of param.table.columns) {
if (!isVirtualCol(column)) { if (
!isVirtualCol(column) ||
(isCreatedTimeOrUpdatedTimeCol(column) && (column as any).system)
) {
const mxColumnLength = Column.getMaxColumnNameLength(sqlClientType); const mxColumnLength = Column.getMaxColumnNameLength(sqlClientType);
// - 5 is a buffer for suffix // - 5 is a buffer for suffix
@ -503,11 +506,16 @@ export class TablesService {
} }
tableCreatePayLoad.columns = await Promise.all( tableCreatePayLoad.columns = await Promise.all(
param.table.columns?.map(async (c) => ({ param.table.columns
...(await getColumnPropsFromUIDT(c as any, source)), // exclude alias columns from column list
cn: c.column_name, ?.filter((c) => {
column_name: c.column_name, return !isCreatedTimeOrUpdatedTimeCol(c) || (c as any).system;
})), })
.map(async (c) => ({
...(await getColumnPropsFromUIDT(c as any, source)),
cn: c.column_name,
column_name: c.column_name,
})),
); );
await sqlMgr.sqlOpPlus(source, 'tableCreate', { await sqlMgr.sqlOpPlus(source, 'tableCreate', {
...tableCreatePayLoad, ...tableCreatePayLoad,

6
packages/nocodb/tests/unit/factory/column.ts

@ -27,7 +27,8 @@ const defaultColumns = function (context) {
title: 'CreatedAt', title: 'CreatedAt',
dtxp: '', dtxp: '',
dtxs: '', dtxs: '',
uidt: 'DateTime', uidt: 'CreatedTime',
system:true,
dt: isPg(context) ? 'timestamp without time zone' : undefined, dt: isPg(context) ? 'timestamp without time zone' : undefined,
}, },
{ {
@ -40,7 +41,8 @@ const defaultColumns = function (context) {
title: 'UpdatedAt', title: 'UpdatedAt',
dtxp: '', dtxp: '',
dtxs: '', dtxs: '',
uidt: 'DateTime', uidt: 'LastModifiedTime',
system:true,
dt: isPg(context) ? 'timestamp without time zone' : undefined, dt: isPg(context) ? 'timestamp without time zone' : undefined,
}, },
]; ];

52
packages/nocodb/tests/unit/model/tests/baseModelSql.test.ts

@ -54,27 +54,15 @@ function baseModelSqlTests() {
); );
const insertedRow = (await baseModelSql.list())[0]; const insertedRow = (await baseModelSql.list())[0];
if (isPg(context)) { inputData.CreatedAt = response.CreatedAt;
inputData.CreatedAt = new Date(inputData.CreatedAt).toISOString(); inputData.UpdatedAt = response.UpdatedAt;
inputData.UpdatedAt = new Date(inputData.UpdatedAt).toISOString();
insertedRow.CreatedAt = new Date(insertedRow.CreatedAt).toISOString();
insertedRow.UpdatedAt = new Date(insertedRow.UpdatedAt).toISOString();
response.CreatedAt = new Date(response.CreatedAt).toISOString();
response.UpdatedAt = new Date(response.UpdatedAt).toISOString();
} else if (isSqlite(context)) {
// append +00:00 to the date string
inputData.CreatedAt = `${inputData.CreatedAt}+00:00`;
inputData.UpdatedAt = `${inputData.UpdatedAt}+00:00`;
}
expect(insertedRow).to.include(inputData); expect(insertedRow).to.include(inputData);
expect(insertedRow).to.include(response); expect(insertedRow).to.include(response);
const rowInsertedAudit = ( const rowInsertedAudit = (await Audit.baseAuditList(base.id, {})).find(
await Audit.baseAuditList(base.id, {}) (audit) => audit.op_sub_type === 'INSERT',
).find((audit) => audit.op_sub_type === 'INSERT'); );
expect(rowInsertedAudit).to.include({ expect(rowInsertedAudit).to.include({
user: 'test@example.com', user: 'test@example.com',
ip: '::ffff:192.0.0.1', ip: '::ffff:192.0.0.1',
@ -120,9 +108,9 @@ function baseModelSqlTests() {
expect(insertedRows[index]).to.include(inputData); expect(insertedRows[index]).to.include(inputData);
}); });
const rowBulkInsertedAudit = ( const rowBulkInsertedAudit = (await Audit.baseAuditList(base.id, {})).find(
await Audit.baseAuditList(base.id, {}) (audit) => audit.op_sub_type === 'BULK_INSERT',
).find((audit) => audit.op_sub_type === 'BULK_INSERT'); );
expect(rowBulkInsertedAudit).to.include({ expect(rowBulkInsertedAudit).to.include({
user: 'test@example.com', user: 'test@example.com',
ip: '::ffff:192.0.0.1', ip: '::ffff:192.0.0.1',
@ -197,9 +185,9 @@ function baseModelSqlTests() {
updatedRows.forEach((row, index) => { updatedRows.forEach((row, index) => {
expect(row['Title']).to.equal(`new-test-${index}`); expect(row['Title']).to.equal(`new-test-${index}`);
}); });
const rowBulkUpdateAudit = ( const rowBulkUpdateAudit = (await Audit.baseAuditList(base.id, {})).find(
await Audit.baseAuditList(base.id, {}) (audit) => audit.op_sub_type === 'BULK_UPDATE',
).find((audit) => audit.op_sub_type === 'BULK_UPDATE'); );
expect(rowBulkUpdateAudit).to.include({ expect(rowBulkUpdateAudit).to.include({
user: 'test@example.com', user: 'test@example.com',
ip: '::ffff:192.0.0.1', ip: '::ffff:192.0.0.1',
@ -248,9 +236,9 @@ function baseModelSqlTests() {
updatedRows.forEach((row) => { updatedRows.forEach((row) => {
if (row.id < 5) expect(row['Title']).to.equal('new-1'); if (row.id < 5) expect(row['Title']).to.equal('new-1');
}); });
const rowBulkUpdateAudit = ( const rowBulkUpdateAudit = (await Audit.baseAuditList(base.id, {})).find(
await Audit.baseAuditList(base.id, {}) (audit) => audit.op_sub_type === 'BULK_UPDATE',
).find((audit) => audit.op_sub_type === 'BULK_UPDATE'); );
expect(rowBulkUpdateAudit).to.include({ expect(rowBulkUpdateAudit).to.include({
user: 'test@example.com', user: 'test@example.com',
ip: '::ffff:192.0.0.1', ip: '::ffff:192.0.0.1',
@ -327,9 +315,9 @@ function baseModelSqlTests() {
expect(remainingRows).to.length(6); expect(remainingRows).to.length(6);
const rowBulkDeleteAudit = ( const rowBulkDeleteAudit = (await Audit.baseAuditList(base.id, {})).find(
await Audit.baseAuditList(base.id, {}) (audit) => audit.op_sub_type === 'BULK_DELETE',
).find((audit) => audit.op_sub_type === 'BULK_DELETE'); );
expect(rowBulkDeleteAudit).to.include({ expect(rowBulkDeleteAudit).to.include({
user: 'test@example.com', user: 'test@example.com',
@ -376,9 +364,9 @@ function baseModelSqlTests() {
const remainingRows = await baseModelSql.list(); const remainingRows = await baseModelSql.list();
expect(remainingRows).to.length(6); expect(remainingRows).to.length(6);
const rowBulkDeleteAudit = ( const rowBulkDeleteAudit = (await Audit.baseAuditList(base.id, {})).find(
await Audit.baseAuditList(base.id, {}) (audit) => audit.op_sub_type === 'BULK_DELETE',
).find((audit) => audit.op_sub_type === 'BULK_DELETE'); );
expect(rowBulkDeleteAudit).to.include({ expect(rowBulkDeleteAudit).to.include({
user: 'test@example.com', user: 'test@example.com',
ip: '::ffff:192.0.0.1', ip: '::ffff:192.0.0.1',

Loading…
Cancel
Save