From 73749974250f2dffa58e4e4a016e0a492b62ab25 Mon Sep 17 00:00:00 2001 From: Aditya Toshniwal Date: Fri, 8 Mar 2024 11:45:32 +0530 Subject: [PATCH] Fix an issue in table dialog where changing column name was not syncing table constraints appropriately. #7229 --- .../static/js/exclusion_constraint.ui.js | 18 ++++++++++-------- .../foreign_key/static/js/foreign_key.ui.js | 9 +++++---- .../static/js/primary_key.ui.js | 6 +++--- .../static/js/unique_constraint.ui.js | 6 +++--- .../schemas/tables/static/js/table.ui.js | 9 +++++---- web/pgadmin/static/js/SchemaView/index.jsx | 2 ++ .../exclusion_constraint.ui.spec.js | 11 ++++++----- 7 files changed, 34 insertions(+), 27 deletions(-) diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/static/js/exclusion_constraint.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/static/js/exclusion_constraint.ui.js index 378310abc..c78268c9b 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/static/js/exclusion_constraint.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/static/js/exclusion_constraint.ui.js @@ -64,11 +64,12 @@ class ExclusionColHeaderSchema extends BaseUISchema { /* Data to ExclusionColumnSchema will added using the header form */ getNewData(data) { - let colType = data.is_exp ? null : _.find(this.columnOptions, (col)=>col.value==data.column)?.datatype; + const column = _.find(this.columnOptions, (col)=>col.value==data.column); return this.exColumnSchema.getNewData({ is_exp: data.is_exp, column: data.is_exp ? data.expression : data.column, - col_type: colType, + column_cid: data.is_exp ? null : column?.cid, + col_type: data.is_exp ? null : column?.datatype, }); } @@ -370,20 +371,21 @@ export default class ExclusionConstraintSchema extends BaseUISchema { depChange: (state, source, topState, actionObj)=>{ /* If in table, sync up value with columns in table */ if(obj.inTable && !state) { - /* the FK is removed by some other dep, this can be a no-op */ + /* the constraint is removed by some other dep, this can be a no-op */ return; } + let currColumns = state.columns || []; if(obj.inTable && source[0] == 'columns') { if(actionObj.type == SCHEMA_STATE_ACTIONS.DELETE_ROW) { - let oldColumn = _.get(actionObj.oldState, actionObj.path.concat(actionObj.value)); - currColumns = _.filter(currColumns, (cc)=>cc.local_column != oldColumn.name); + let column = _.get(actionObj.oldState, actionObj.path.concat(actionObj.value)); + currColumns = _.filter(currColumns, (cc)=>cc.column_cid != column.cid); } else if(actionObj.type == SCHEMA_STATE_ACTIONS.SET_VALUE) { let tabColPath = _.slice(actionObj.path, 0, -1); - let oldColName = _.get(actionObj.oldState, tabColPath).name; - let idx = _.findIndex(currColumns, (cc)=>cc.local_column == oldColName); + let column = _.get(topState, tabColPath); + let idx = _.findIndex(currColumns, (cc)=>cc.column_cid == column.cid); if(idx > -1) { - currColumns[idx].local_column = _.get(topState, tabColPath).name; + currColumns[idx].column = column.name; } } } diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/foreign_key/static/js/foreign_key.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/foreign_key/static/js/foreign_key.ui.js index cfa975c41..2b33d14f8 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/foreign_key/static/js/foreign_key.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/foreign_key/static/js/foreign_key.ui.js @@ -66,6 +66,7 @@ class ForeignKeyHeaderSchema extends BaseUISchema { let references_table_name = _.find(this.refTables, (t)=>t.value==data.references || t.value == this.origData.references)?.label; return { local_column: data.local_column, + local_column_cid: _.find(this.fieldOptions.local_column, (c)=>c.value == data.local_column)?.cid, referenced: data.referenced, references: data.references, references_table_name: references_table_name, @@ -346,12 +347,12 @@ export default class ForeignKeySchema extends BaseUISchema { let currColumns = state.columns || []; if(obj.inTable && source[0] == 'columns') { if(actionObj.type == SCHEMA_STATE_ACTIONS.DELETE_ROW) { - let oldColumn = _.get(actionObj.oldState, actionObj.path.concat(actionObj.value)); - currColumns = _.filter(currColumns, (cc)=>cc.local_column != oldColumn.name); + let column = _.get(actionObj.oldState, actionObj.path.concat(actionObj.value)); + currColumns = _.filter(currColumns, (cc)=>cc.local_column_cid != column.cid); } else if(actionObj.type == SCHEMA_STATE_ACTIONS.SET_VALUE) { let tabColPath = _.slice(actionObj.path, 0, -1); - let oldColName = _.get(actionObj.oldState, tabColPath).name; - let idx = _.findIndex(currColumns, (cc)=>cc.local_column == oldColName); + let column = _.get(actionObj.oldState, tabColPath); + let idx = _.findIndex(currColumns, (cc)=>cc.local_column_cid == column.cid); if(idx > -1) { currColumns[idx].local_column = _.get(topState, tabColPath).name; } diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/primary_key.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/primary_key.ui.js index 3678682c4..b0bc8c01d 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/primary_key.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/primary_key.ui.js @@ -86,11 +86,11 @@ export default class PrimaryKeySchema extends BaseUISchema { if(obj.inTable && source[0] == 'columns') { if(actionObj.type == SCHEMA_STATE_ACTIONS.DELETE_ROW) { let oldColumn = _.get(actionObj.oldState, actionObj.path.concat(actionObj.value)); - currColumns = _.filter(currColumns, (cc)=>cc.column != oldColumn.name); + currColumns = _.filter(currColumns, (cc)=>cc.cid != oldColumn.cid); } else if(actionObj.type == SCHEMA_STATE_ACTIONS.SET_VALUE) { let tabColPath = _.slice(actionObj.path, 0, -1); - let oldColName = _.get(actionObj.oldState, tabColPath).name; - let idx = _.findIndex(currColumns, (cc)=>cc.column == oldColName); + let oldCol = _.get(actionObj.oldState, tabColPath); + let idx = _.findIndex(currColumns, (cc)=>cc.cid == oldCol.cid); if(idx > -1) { currColumns[idx].column = _.get(topState, tabColPath).name; } diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/unique_constraint.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/unique_constraint.ui.js index b498b6343..632193db5 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/unique_constraint.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/index_constraint/static/js/unique_constraint.ui.js @@ -88,11 +88,11 @@ export default class UniqueConstraintSchema extends BaseUISchema { if(obj.inTable && source[0] == 'columns') { if(actionObj.type == SCHEMA_STATE_ACTIONS.DELETE_ROW) { let oldColumn = _.get(actionObj.oldState, actionObj.path.concat(actionObj.value)); - currColumns = _.filter(currColumns, (cc)=>cc.column != oldColumn.name); + currColumns = _.filter(currColumns, (cc)=>cc.cid != oldColumn.cid); } else if(actionObj.type == SCHEMA_STATE_ACTIONS.SET_VALUE) { let tabColPath = _.slice(actionObj.path, 0, -1); - let oldColName = _.get(actionObj.oldState, tabColPath).name; - let idx = _.findIndex(currColumns, (cc)=>cc.column == oldColName); + let oldCol = _.get(actionObj.oldState, tabColPath); + let idx = _.findIndex(currColumns, (cc)=>cc.cid == oldCol.cid); if(idx > -1) { currColumns[idx].column = _.get(topState, tabColPath).name; } diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.ui.js index a9afd37ea..a1e8a5d3d 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/table.ui.js @@ -448,7 +448,7 @@ export default class TableSchema extends BaseUISchema { } changeColumnOptions(columns) { - let colOptions = (columns||[]).map((c)=>({label: c.name, value: c.name, image:'icon-column'})); + let colOptions = (columns||[]).map((c)=>({label: c.name, value: c.name, image:'icon-column', cid:c.cid})); this.constraintsObj.changeColumnOptions(colOptions); this.partitionKeysObj.changeColumnOptions(colOptions); this.partitionsObj.changeColumnOptions(colOptions); @@ -682,11 +682,12 @@ export default class TableSchema extends BaseUISchema { let currPk = state.primary_key[0]; /* If col is not PK, remove it */ if(!columnData.is_primary_key) { - currPk.columns = _.filter(currPk.columns, (c)=>c.column !== columnData.name); + currPk.columns = _.filter(currPk.columns, (c)=>c.cid !== columnData.cid); } else { - currPk.columns = _.filter(currPk.columns, (c)=>c.column !== columnData.name); + currPk.columns = _.filter(currPk.columns, (c)=>c.cid !== columnData.cid); currPk.columns.push({ column: columnData.name, + cid: columnData.cid, }); } /* Remove the PK if all columns not PK */ @@ -699,7 +700,7 @@ export default class TableSchema extends BaseUISchema { /* Create PK if none */ return {primary_key: [ obj.constraintsObj.primaryKeyObj.getNewData({ - columns: [{column: columnData.name}], + columns: [{column: columnData.name, cid: columnData.cid}], }) ]}; } diff --git a/web/pgadmin/static/js/SchemaView/index.jsx b/web/pgadmin/static/js/SchemaView/index.jsx index 134f26b7e..3413bbe68 100644 --- a/web/pgadmin/static/js/SchemaView/index.jsx +++ b/web/pgadmin/static/js/SchemaView/index.jsx @@ -615,6 +615,7 @@ function SchemaDialogView({ useEffect(()=>{ /* If reset key changes, reset the form */ + schema.initialise(schema.origData); sessDispatch({ type: SCHEMA_STATE_ACTIONS.INIT, payload: schema.origData, @@ -626,6 +627,7 @@ function SchemaDialogView({ const resetIt = ()=>{ firstEleRef.current?.focus(); setFormResetKey((prev)=>prev+1); + schema.initialise(schema.origData); sessDispatch({ type: SCHEMA_STATE_ACTIONS.INIT, payload: schema.origData, diff --git a/web/regression/javascript/schema_ui_files/exclusion_constraint.ui.spec.js b/web/regression/javascript/schema_ui_files/exclusion_constraint.ui.spec.js index 870d361d3..0e01d3ed9 100644 --- a/web/regression/javascript/schema_ui_files/exclusion_constraint.ui.spec.js +++ b/web/regression/javascript/schema_ui_files/exclusion_constraint.ui.spec.js @@ -106,6 +106,7 @@ describe('ExclusionConstraintSchema', ()=>{ is_exp: true, column: 'abc', col_type: null, + column_cid: null, }); }); }); @@ -125,14 +126,14 @@ describe('ExclusionConstraintSchema', ()=>{ }); it('depChange', ()=>{ - let state = {columns: [{local_column: 'id'}]}; + let state = {columns: [{column: 'id', column_cid: 'c123'}]}; schemaObj.top = new TableSchema({}, null); expect(getFieldDepChange(schemaObj, 'columns')(state, ['columns', 0], null, { type: SCHEMA_STATE_ACTIONS.DELETE_ROW, oldState: { columns: [ - {name: 'id'} + {name: 'id', cid: 'c123'} ], }, path: ['columns'], @@ -143,19 +144,19 @@ describe('ExclusionConstraintSchema', ()=>{ expect(getFieldDepChange(schemaObj, 'columns')(state, ['columns', 0], { columns: [ - {name: 'id123'} + {name: 'id123', cid: 'c123'} ], }, { type: SCHEMA_STATE_ACTIONS.SET_VALUE, oldState: { columns: [ - {name: 'id'} + {name: 'id', cid: 'c123'} ], }, path: ['columns', 0, 'name'], value: 'id123', })).toEqual({ - columns: [{local_column: 'id123'}], + columns: [{column: 'id123', column_cid: 'c123'}], }); state = {};