Browse Source

refactor: suggested changes

pull/7304/head
Pranav C 11 months ago
parent
commit
ec92d8c177
  1. 4
      packages/nc-gui/components/smartsheet/VirtualCell.vue
  2. 12
      packages/nc-gui/components/smartsheet/header/Menu.vue
  3. 2
      packages/nocodb-sdk/src/lib/UITypes.ts
  4. 2
      packages/nocodb-sdk/src/lib/index.ts
  5. 8
      packages/nocodb/src/db/BaseModelSqlv2.ts
  6. 6
      packages/nocodb/src/helpers/getAst.ts
  7. 18
      packages/nocodb/src/services/columns.service.ts
  8. 6
      packages/nocodb/src/services/tables.service.ts
  9. 4
      packages/nocodb/src/version-upgrader/ncXcdbCreatedAndUpdatedTimeUpgrader.ts
  10. 4
      packages/nocodb/tests/unit/factory/row.ts
  11. 12
      packages/nocodb/tests/unit/rest/tests/newDataApis.test.ts

4
packages/nc-gui/components/smartsheet/VirtualCell.vue

@ -1,6 +1,6 @@
<script setup lang="ts"> <script setup lang="ts">
import type { ColumnType } from 'nocodb-sdk' import type { ColumnType } from 'nocodb-sdk'
import { isCreatedTimeOrUpdatedTimeCol } from 'nocodb-sdk' import { isCreatedOrLastModifiedTimeCol } from 'nocodb-sdk'
import { import {
ActiveCellInj, ActiveCellInj,
CellValueInj, CellValueInj,
@ -115,7 +115,7 @@ onUnmounted(() => {
<LazyVirtualCellBarcode v-else-if="isBarcode(column)" /> <LazyVirtualCellBarcode v-else-if="isBarcode(column)" />
<LazyVirtualCellCount v-else-if="isCount(column)" /> <LazyVirtualCellCount v-else-if="isCount(column)" />
<LazyVirtualCellLookup v-else-if="isLookup(column)" /> <LazyVirtualCellLookup v-else-if="isLookup(column)" />
<LazyCellReadOnlyDateTimePicker v-else-if="isCreatedTimeOrUpdatedTimeCol(column)" :model-value="modelValue" /> <LazyCellReadOnlyDateTimePicker v-else-if="isCreatedOrLastModifiedTimeCol(column)" :model-value="modelValue" />
</template> </template>
</div> </div>
</template> </template>

12
packages/nc-gui/components/smartsheet/header/Menu.vue

@ -279,18 +279,10 @@ const onInsertAfter = () => {
} }
const isDeleteAllowed = computed(() => { const isDeleteAllowed = computed(() => {
if (!column?.value) return false return column?.value && !column.value.system
if (column.value.uidt && [UITypes.CreatedTime, UITypes.LastModifiedTime].includes(column.value.uidt) && column.value.system) {
return false
}
return true
}) })
const isDuplicateAllowed = computed(() => { const isDuplicateAllowed = computed(() => {
if (!column?.value) return false return column?.value && !column.value.system
if (column.value.system) {
return false
}
return true
}) })
</script> </script>

2
packages/nocodb-sdk/src/lib/UITypes.ts

@ -88,7 +88,7 @@ export function isVirtualCol(
// UITypes.Count, // UITypes.Count,
].includes(<UITypes>(typeof col === 'object' ? col?.uidt : col)); ].includes(<UITypes>(typeof col === 'object' ? col?.uidt : col));
} }
export function isCreatedTimeOrUpdatedTimeCol( export function isCreatedOrLastModifiedTimeCol(
col: col:
| UITypes | UITypes
| { readonly uidt: UITypes | string } | { readonly uidt: UITypes | string }

2
packages/nocodb-sdk/src/lib/index.ts

@ -13,7 +13,7 @@ export {
isNumericCol, isNumericCol,
isVirtualCol, isVirtualCol,
isLinksOrLTAR, isLinksOrLTAR,
isCreatedTimeOrUpdatedTimeCol, isCreatedOrLastModifiedTimeCol,
} from '~/lib/UITypes'; } from '~/lib/UITypes';
export { default as CustomAPI, FileType } from '~/lib/CustomAPI'; export { default as CustomAPI, FileType } from '~/lib/CustomAPI';
export { default as TemplateGenerator } from '~/lib/TemplateGenerator'; export { default as TemplateGenerator } from '~/lib/TemplateGenerator';

8
packages/nocodb/src/db/BaseModelSqlv2.ts

@ -8,7 +8,7 @@ import { nocoExecute } from 'nc-help';
import { import {
AuditOperationSubTypes, AuditOperationSubTypes,
AuditOperationTypes, AuditOperationTypes,
isCreatedTimeOrUpdatedTimeCol, isCreatedOrLastModifiedTimeCol,
isLinksOrLTAR, isLinksOrLTAR,
isSystemColumn, isSystemColumn,
isVirtualCol, isVirtualCol,
@ -103,7 +103,7 @@ function checkColumnRequired(
} }
export async function getColumnName(column: Column<any>, columns?: Column[]) { export async function getColumnName(column: Column<any>, columns?: Column[]) {
if (!isCreatedTimeOrUpdatedTimeCol(column)) return column.column_name; if (!isCreatedOrLastModifiedTimeCol(column)) return column.column_name;
columns = columns || (await Column.list({ fk_model_id: column.fk_model_id })); columns = columns || (await Column.list({ fk_model_id: column.fk_model_id }));
switch (column.uidt) { switch (column.uidt) {
@ -2962,7 +2962,7 @@ class BaseModelSqlv2 {
for (let i = 0; i < this.model.columns.length; ++i) { for (let i = 0; i < this.model.columns.length; ++i) {
const col = this.model.columns[i]; const col = this.model.columns[i];
if (col.title in d && isCreatedTimeOrUpdatedTimeCol(col)) { if (col.title in d && isCreatedOrLastModifiedTimeCol(col)) {
NcError.badRequest( NcError.badRequest(
`Column "${col.title}" is auto generated and cannot be updated`, `Column "${col.title}" is auto generated and cannot be updated`,
); );
@ -3847,7 +3847,7 @@ class BaseModelSqlv2 {
for (let i = 0; i < this.model.columns.length; ++i) { for (let i = 0; i < this.model.columns.length; ++i) {
const column = this.model.columns[i]; const column = this.model.columns[i];
if (column.title in data && isCreatedTimeOrUpdatedTimeCol(column)) { if (column.title in data && isCreatedOrLastModifiedTimeCol(column)) {
NcError.badRequest( NcError.badRequest(
`Column "${column.title}" is auto generated and cannot be updated`, `Column "${column.title}" is auto generated and cannot be updated`,
); );

6
packages/nocodb/src/helpers/getAst.ts

@ -1,5 +1,5 @@
import { import {
isCreatedTimeOrUpdatedTimeCol, isCreatedOrLastModifiedTimeCol,
isSystemColumn, isSystemColumn,
RelationTypes, RelationTypes,
UITypes, UITypes,
@ -155,13 +155,13 @@ const getAst = async ({
if (getHiddenColumn) { if (getHiddenColumn) {
isRequested = isRequested =
!isSystemColumn(col) || !isSystemColumn(col) ||
(isCreatedTimeOrUpdatedTimeCol(col) && col.system) || (isCreatedOrLastModifiedTimeCol(col) && col.system) ||
col.pk; col.pk;
} else if (allowedCols && (!includePkByDefault || !col.pk)) { } else if (allowedCols && (!includePkByDefault || !col.pk)) {
isRequested = isRequested =
allowedCols[col.id] && allowedCols[col.id] &&
(!isSystemColumn(col) || (!isSystemColumn(col) ||
(!view && isCreatedTimeOrUpdatedTimeCol(col)) || (!view && isCreatedOrLastModifiedTimeCol(col)) ||
view.show_system_fields || view.show_system_fields ||
col.pv) && col.pv) &&
(!fields?.length || fields.includes(col.title)) && (!fields?.length || fields.includes(col.title)) &&

18
packages/nocodb/src/services/columns.service.ts

@ -1,7 +1,7 @@
import { Injectable } from '@nestjs/common'; import { Injectable } from '@nestjs/common';
import { import {
AppEvents, AppEvents,
isCreatedTimeOrUpdatedTimeCol, isCreatedOrLastModifiedTimeCol,
isLinksOrLTAR, isLinksOrLTAR,
isVirtualCol, isVirtualCol,
substituteColumnAliasWithIdInFormula, substituteColumnAliasWithIdInFormula,
@ -172,7 +172,7 @@ export class ColumnsService {
if ( if (
!isVirtualCol(param.column) && !isVirtualCol(param.column) &&
!isCreatedTimeOrUpdatedTimeCol(param.column) && !isCreatedOrLastModifiedTimeCol(param.column) &&
!(await Column.checkTitleAvailable({ !(await Column.checkTitleAvailable({
column_name: param.column.column_name, column_name: param.column.column_name,
fk_model_id: column.fk_model_id, fk_model_id: column.fk_model_id,
@ -197,7 +197,7 @@ export class ColumnsService {
parsed_tree?: any; parsed_tree?: any;
}; };
if ( if (
isCreatedTimeOrUpdatedTimeCol(column) || isCreatedOrLastModifiedTimeCol(column) ||
[ [
UITypes.Lookup, UITypes.Lookup,
UITypes.Rollup, UITypes.Rollup,
@ -1694,9 +1694,6 @@ export class ColumnsService {
// else create a new column in meta and db // else create a new column in meta and db
const existingColumn = columns.find( const existingColumn = columns.find(
(c) => c.uidt === colBody.uidt && c.system, (c) => c.uidt === colBody.uidt && c.system,
// ||
// c.column_name ===
// (c.uidt === UITypes.CreatedTime ? 'created_at' : 'updated_at'),
); );
if (!existingColumn) { if (!existingColumn) {
@ -1704,15 +1701,6 @@ export class ColumnsService {
colBody.uidt === UITypes.CreatedTime colBody.uidt === UITypes.CreatedTime
? 'created_at' ? 'created_at'
: 'updated_at'; : 'updated_at';
// const sqlClient = await reuseOrSave('sqlClient', reuse, async () =>
// NcConnectionMgrv2.getSqlClient(source),
// );
// const dbColumns = (
// await sqlClient.columnList({
// tn: table.table_name,
// schema: source.getConfig()?.schema,
// })
// )?.data?.list;
// todo: check type as well // todo: check type as well
const dbColumn = columns.find((c) => c.column_name === columnName); const dbColumn = columns.find((c) => c.column_name === columnName);

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

@ -2,7 +2,7 @@ import { Injectable } from '@nestjs/common';
import DOMPurify from 'isomorphic-dompurify'; import DOMPurify from 'isomorphic-dompurify';
import { import {
AppEvents, AppEvents,
isCreatedTimeOrUpdatedTimeCol, isCreatedOrLastModifiedTimeCol,
isLinksOrLTAR, isLinksOrLTAR,
isVirtualCol, isVirtualCol,
ModelTypes, ModelTypes,
@ -514,7 +514,7 @@ export class TablesService {
for (const column of param.table.columns) { for (const column of param.table.columns) {
if ( if (
!isVirtualCol(column) || !isVirtualCol(column) ||
(isCreatedTimeOrUpdatedTimeCol(column) && (column as any).system) (isCreatedOrLastModifiedTimeCol(column) && (column as any).system)
) { ) {
const mxColumnLength = Column.getMaxColumnNameLength(sqlClientType); const mxColumnLength = Column.getMaxColumnNameLength(sqlClientType);
@ -549,7 +549,7 @@ export class TablesService {
param.table.columns param.table.columns
// exclude alias columns from column list // exclude alias columns from column list
?.filter((c) => { ?.filter((c) => {
return !isCreatedTimeOrUpdatedTimeCol(c) || (c as any).system; return !isCreatedOrLastModifiedTimeCol(c) || (c as any).system;
}) })
.map(async (c) => ({ .map(async (c) => ({
...(await getColumnPropsFromUIDT(c as any, source)), ...(await getColumnPropsFromUIDT(c as any, source)),

4
packages/nocodb/src/version-upgrader/ncXcdbCreatedAndUpdatedTimeUpgrader.ts

@ -109,7 +109,7 @@ async function upgradeModels({
{ {
uidt: UITypes.CreatedTime, uidt: UITypes.CreatedTime,
column_name: getUniqueColumnName(columns, 'created_at'), column_name: getUniqueColumnName(columns, 'created_at'),
title: getUniqueColumnAliasName(columns, 'Created At'), title: getUniqueColumnAliasName(columns, 'CreatedAt'),
}, },
source, source,
)), )),
@ -125,7 +125,7 @@ async function upgradeModels({
{ {
uidt: UITypes.LastModifiedTime, uidt: UITypes.LastModifiedTime,
column_name: getUniqueColumnName(columns, 'updated_at'), column_name: getUniqueColumnName(columns, 'updated_at'),
title: getUniqueColumnAliasName(columns, 'Updated At'), title: getUniqueColumnAliasName(columns, 'UpdatedAt'),
}, },
source, source,
)), )),

4
packages/nocodb/tests/unit/factory/row.ts

@ -1,4 +1,4 @@
import { isCreatedTimeOrUpdatedTimeCol, UITypes } from 'nocodb-sdk' import { isCreatedOrLastModifiedTimeCol, UITypes } from 'nocodb-sdk'
import request from 'supertest'; import request from 'supertest';
import Model from '../../../src/models/Model'; import Model from '../../../src/models/Model';
import NcConnectionMgrv2 from '../../../src/utils/common/NcConnectionMgrv2'; import NcConnectionMgrv2 from '../../../src/utils/common/NcConnectionMgrv2';
@ -258,7 +258,7 @@ const generateDefaultRowAttributes = ({
column.uidt === UITypes.LinkToAnotherRecord || column.uidt === UITypes.LinkToAnotherRecord ||
column.uidt === UITypes.ForeignKey || column.uidt === UITypes.ForeignKey ||
column.uidt === UITypes.ID || column.uidt === UITypes.ID ||
isCreatedTimeOrUpdatedTimeCol(column) isCreatedOrLastModifiedTimeCol(column)
) { ) {
return acc; return acc;
} }

12
packages/nocodb/tests/unit/rest/tests/newDataApis.test.ts

@ -83,7 +83,7 @@
import 'mocha'; import 'mocha';
import { import {
isCreatedTimeOrUpdatedTimeCol, isCreatedOrLastModifiedTimeCol,
UITypes, UITypes,
ViewTypes, ViewTypes,
WorkspaceUserRoles, WorkspaceUserRoles,
@ -679,7 +679,7 @@ function textBased() {
expect( expect(
verifyColumnsInRsp( verifyColumnsInRsp(
rsp.body.list[0], rsp.body.list[0],
columns.filter((c) => !isCreatedTimeOrUpdatedTimeCol(c) || !c.system), columns.filter((c) => !isCreatedOrLastModifiedTimeCol(c) || !c.system),
), ),
).to.equal(true); ).to.equal(true);
const filteredArray = rsp.body.list.map((r) => r.SingleLineText); const filteredArray = rsp.body.list.map((r) => r.SingleLineText);
@ -700,7 +700,7 @@ function textBased() {
const displayColumns = columns.filter( const displayColumns = columns.filter(
(c) => (c) =>
c.title !== 'SingleLineText' && c.title !== 'SingleLineText' &&
(!isCreatedTimeOrUpdatedTimeCol(c) || !c.system), (!isCreatedOrLastModifiedTimeCol(c) || !c.system),
); );
expect(verifyColumnsInRsp(rsp.body.list[0], displayColumns)).to.equal(true); expect(verifyColumnsInRsp(rsp.body.list[0], displayColumns)).to.equal(true);
}); });
@ -749,7 +749,7 @@ function textBased() {
(c) => (c) =>
c.title !== 'MultiLineText' && c.title !== 'MultiLineText' &&
c.title !== 'Email' && c.title !== 'Email' &&
!isCreatedTimeOrUpdatedTimeCol(c), !isCreatedOrLastModifiedTimeCol(c),
); );
expect(verifyColumnsInRsp(rsp.body.list[0], displayColumns)).to.equal(true); expect(verifyColumnsInRsp(rsp.body.list[0], displayColumns)).to.equal(true);
return gridView; return gridView;
@ -769,7 +769,7 @@ function textBased() {
(c) => (c) =>
c.title !== 'MultiLineText' && c.title !== 'MultiLineText' &&
c.title !== 'Email' && c.title !== 'Email' &&
!isCreatedTimeOrUpdatedTimeCol(c), !isCreatedOrLastModifiedTimeCol(c),
); );
expect(rsp.body.pageInfo.totalRows).to.equal(61); expect(rsp.body.pageInfo.totalRows).to.equal(61);
expect(verifyColumnsInRsp(rsp.body.list[0], displayColumns)).to.equal(true); expect(verifyColumnsInRsp(rsp.body.list[0], displayColumns)).to.equal(true);
@ -791,7 +791,7 @@ function textBased() {
(c) => (c) =>
c.title !== 'MultiLineText' && c.title !== 'MultiLineText' &&
c.title !== 'Email' && c.title !== 'Email' &&
!isCreatedTimeOrUpdatedTimeCol(c), !isCreatedOrLastModifiedTimeCol(c),
); );
expect(rsp.body.pageInfo.totalRows).to.equal(7); expect(rsp.body.pageInfo.totalRows).to.equal(7);
expect(verifyColumnsInRsp(rsp.body.list[0], displayColumns)).to.equal(true); expect(verifyColumnsInRsp(rsp.body.list[0], displayColumns)).to.equal(true);

Loading…
Cancel
Save