diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js index 062464c504f..dc6958a128d 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/columns/static/js/column.ui.js @@ -88,15 +88,21 @@ export default class ColumnSchema extends BaseUISchema { } if(this.nodeInfo && ('schema' in this.nodeInfo)) { - if(this.isNew(state)) { - return false; + // inheritedfrom/inheritedfromtable check is useful when we use this + // schema in table node. A column inherited from a parent table should + // always be read-only, whether it was already present when the table + // was opened (inheritedfromtable, set on the properties fetch) or was + // just added interactively via 'Inherited from table(s)' + // (inheritedfrom, set on the freshly fetched column). This must be + // checked before the isNew() check below, as interactively added + // inherited columns don't carry an attnum yet and would otherwise be + // (wrongly) treated as new, editable rows. + if (this.isInheritedColumn(state)) { + return true; } - // We will disable control if it's system columns - // inheritedfrom check is useful when we use this schema in table node - // inheritedfrom has value then we should disable it - if (!isEmptyString(state.inheritedfrom)){ - return true; + if(this.isNew(state)) { + return false; } // ie: it's position is less than 1 @@ -164,6 +170,33 @@ export default class ColumnSchema extends BaseUISchema { return !isEmptyString(state.inheritedfromtype); } + // A column inherited from a parent table should be treated as read-only, + // whether it was already present when the table was opened + // (inheritedfromtable, set on the properties fetch) or was just added + // interactively via 'Inherited from table(s)' (inheritedfrom, set on the + // freshly fetched column). Shared by any editable/disabled check that + // needs to special-case inherited columns. + isInheritedColumn(state) { + return !isEmptyString(state.inheritedfrom) || + !isEmptyString(state.inheritedfromtable); + } + + // Shared by the inline grid-cell 'Data type' editor and the expanded + // Definition tab's 'Data type' dropdown, so both apply the exact same + // edit_types restriction for the same column. isRowNew must be computed + // by the caller against the *row's* own state (not the enclosing table's + // or the field's own scalar state), since new columns can be set to any + // type whilst existing ones may only be altered to one of edit_types. + editTypesFilter(edit_types, isRowNew) { + return (options)=>{ + if (isRowNew || this.inErd) { + return options; + } + let allowed = edit_types || []; + return _.filter(options, (o)=>allowed.indexOf(o.value) > -1); + }; + } + get baseFields() { let obj = this; @@ -241,20 +274,21 @@ export default class ColumnSchema extends BaseUISchema { group: gettext('Definition'), noEmpty: true, editable: this.editableCheckForTable, options: this.cltypeOptions, optionsLoaded: (options)=>{obj.datatypes = options;}, - type: (state)=>{ + // 'edit_types'/'attnum' are declared as deps purely so that the + // schema view resolves them against this row (not the whole table), + // and passes them through as the 2nd (depVals) argument below. This + // is what lets the expanded Definition tab's dropdown apply the same + // edit_types restriction as the inline grid-cell editor, whose + // 'cell' callback already receives the full row. + deps: ['edit_types', 'attnum'], + type: (state, depVals)=>{ + let [edit_types, attnum] = depVals || []; return { type: 'select', options: this.cltypeOptions, controlProps: { allowClear: false, - filter: (options)=>{ - let result = options; - let edit_types = state?.edit_types || []; - if(!obj.isNew(state) && !this.inErd) { - result = _.filter(options, (o)=>edit_types.indexOf(o.value) > -1); - } - return result; - }, + filter: obj.editTypesFilter(edit_types, obj.isNew({attnum})), } }; }, @@ -264,14 +298,7 @@ export default class ColumnSchema extends BaseUISchema { options: this.cltypeOptions, controlProps: { allowClear: false, - filter: (options)=>{ - let result = options; - let edit_types = row?.edit_types || []; - if(!obj.isNew(row) && !this.inErd) { - result = _.filter(options, (o)=>edit_types.indexOf(o.value) > -1); - } - return result; - }, + filter: obj.editTypesFilter(row?.edit_types, obj.isNew(row)), } }; } @@ -370,8 +397,8 @@ export default class ColumnSchema extends BaseUISchema { return !obj.attlenRange(state); }, editable: function(state) { - // inheritedfrom has value then we should disable it - if (!isEmptyString(state.inheritedfrom)) { + // A column inherited from a parent table should stay read-only. + if (obj.isInheritedColumn(state)) { return false; } return Boolean(obj.attlenRange(state)); @@ -402,11 +429,11 @@ export default class ColumnSchema extends BaseUISchema { return !this.attprecisionRange(state); }, editable: function(state) { - // inheritedfrom has value then we should disable it - if (!isEmptyString(state.inheritedfrom)) { + // A column inherited from a parent table should stay read-only. + if (obj.isInheritedColumn(state)) { return false; } - return Boolean(this.attprecisionRange(state)); + return Boolean(obj.attprecisionRange(state)); }, },{ id: 'min_val_attprecision', skipChange: true, visible: false, type: '', 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 4cc2bc46b47..6efb0064ca4 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 @@ -510,7 +510,12 @@ export default class TableSchema extends BaseUISchema { // Check for column grid when to edit/delete (for each row) canEditDeleteRowColumns(colstate) { - return isEmptyString(colstate.inheritedfrom); + // 'inheritedfrom' is set on columns fetched interactively via + // 'Inherited from table(s)'; 'inheritedfromtable' is set on columns + // already inherited when the table's properties were fetched. Both + // must disable the row's edit/delete buttons. + return isEmptyString(colstate.inheritedfrom) && + isEmptyString(colstate.inheritedfromtable); } isPartitioned(state) { diff --git a/web/pgadmin/static/js/SchemaView/MappedControl.jsx b/web/pgadmin/static/js/SchemaView/MappedControl.jsx index 78353a07097..0adfb0d4285 100644 --- a/web/pgadmin/static/js/SchemaView/MappedControl.jsx +++ b/web/pgadmin/static/js/SchemaView/MappedControl.jsx @@ -401,7 +401,15 @@ export const MappedFormControl = ({ } if (typeof (field.type) === 'function') { - const typeProps = evalFunc(null, field.type, state); + // 'state' here is the whole top-level schema data, not this field's + // row, since a field nested inside a collection row shares the same + // accessPath resolution as any other field. 'depVals' (already resolved + // against this field's own row via 'deps', see listenDepChanges above) + // is passed as a 2nd argument so a field.type() callback can access + // sibling fields from its own row, mirroring what field.cell() already + // gets via its row argument. Existing field.type() callbacks that only + // take a single argument are unaffected. + const typeProps = evalFunc(null, field.type, state, depVals); newProps = { ...newProps, ...typeProps, diff --git a/web/regression/javascript/schema_ui_files/column.ui.spec.js b/web/regression/javascript/schema_ui_files/column.ui.spec.js index cc99104e044..ce31215b909 100644 --- a/web/regression/javascript/schema_ui_files/column.ui.spec.js +++ b/web/regression/javascript/schema_ui_files/column.ui.spec.js @@ -133,6 +133,97 @@ describe('ColumnSchema', ()=>{ expect(schemaObj.inSchemaWithColumnCheck(state)).toBe(false); }); + it('inSchemaWithColumnCheck - column already inherited from a parent table', ()=>{ + // Set on the properties fetch for a column the table already + // inherits when opened (issue #10179, case 1). + schemaObj.nodeInfo = {schema: {}}; + let state = {attnum: 1, inheritedfromtable: 'public.parent'}; + expect(schemaObj.inSchemaWithColumnCheck(state)).toBe(true); + expect(schemaObj.editableCheckForTable(state)).toBe(false); + }); + + it('inSchemaWithColumnCheck - column added interactively via Inherited from table(s)', ()=>{ + // Columns freshly fetched via 'Inherited from table(s)' don't carry an + // attnum yet, so isNew() would otherwise (wrongly) treat them as new, + // editable rows (issue #10179, case 2). + schemaObj.nodeInfo = {schema: {}}; + let state = {name: 'id', inheritedfrom: 'public.parent'}; + expect(schemaObj.inSchemaWithColumnCheck(state)).toBe(true); + expect(schemaObj.editableCheckForTable(state)).toBe(false); + }); + + it('attlen/attprecision editable - column already inherited from a parent table (inheritedfromtable only)', ()=>{ + // A loaded row can carry only inheritedfromtable, with no + // inheritedfrom (issue #10179, case 1). The attlen/attprecision + // editable checks must honour that case too, not just the + // interactively-added 'inheritedfrom' case. + schemaObj.datatypes = datatypes; + let attlenField = _.find(schemaObj.fields, (f)=>f.id === 'attlen'); + let attprecisionField = _.find(schemaObj.fields, (f)=>f.id === 'attprecision'); + + let varcharState = {cltype: 'character varying', inheritedfromtable: 'public.parent'}; + expect(attlenField.editable(varcharState)).toBe(false); + + let numericState = {cltype: 'numeric', inheritedfromtable: 'public.parent'}; + expect(attprecisionField.editable(numericState)).toBe(false); + + // Sanity check: without inheritance, they remain editable when the + // data type supports length/precision. + expect(attlenField.editable({cltype: 'character varying'})).toBe(true); + expect(attprecisionField.editable({cltype: 'numeric'})).toBe(true); + }); + + it('editTypesFilter', ()=>{ + let options = [ + {label: 'integer', value: 'integer'}, + {label: 'text', value: 'text'}, + {label: 'boolean', value: 'boolean'}, + ]; + + // Existing column: restricted to edit_types. + let filtered = schemaObj.editTypesFilter(['integer', 'text'], false)(options); + expect(filtered).toEqual([ + {label: 'integer', value: 'integer'}, + {label: 'text', value: 'text'}, + ]); + + // New column: unrestricted, full list. + expect(schemaObj.editTypesFilter(['integer'], true)(options)).toEqual(options); + + // No edit_types available: restricts down to nothing. + expect(schemaObj.editTypesFilter(undefined, false)(options)).toEqual([]); + + // ERD is always unrestricted, regardless of edit_types/isNew. + schemaObj.inErd = true; + expect(schemaObj.editTypesFilter(['integer'], false)(options)).toEqual(options); + schemaObj.inErd = false; + }); + + it('cltype - expanded Definition tab options match the inline grid-cell options', ()=>{ + let cltypeField = _.find(schemaObj.fields, (f)=>f.id === 'cltype'); + let options = [ + {label: 'integer', value: 'integer'}, + {label: 'text', value: 'text'}, + {label: 'boolean', value: 'boolean'}, + ]; + let row = {attnum: 1, edit_types: ['integer', 'boolean']}; + + // Inline grid-cell editor. + let cellResult = cltypeField.cell(row); + let cellFiltered = cellResult.controlProps.filter(options); + + // Expanded Definition tab, as it is invoked once 'edit_types'/'attnum' + // have been resolved against this row via deps (see MappedControl.jsx). + let typeResult = cltypeField.type(row.cltype, [row.edit_types, row.attnum]); + let typeFiltered = typeResult.controlProps.filter(options); + + expect(typeFiltered).toEqual(cellFiltered); + expect(typeFiltered).toEqual([ + {label: 'integer', value: 'integer'}, + {label: 'boolean', value: 'boolean'}, + ]); + }); + it('editableCheckForTable', ()=>{ let state = {}; schemaObj.nodeInfo = {}; diff --git a/web/regression/javascript/schema_ui_files/table.ui.spec.js b/web/regression/javascript/schema_ui_files/table.ui.spec.js index 46fa0b1d67c..7482debdaad 100644 --- a/web/regression/javascript/schema_ui_files/table.ui.spec.js +++ b/web/regression/javascript/schema_ui_files/table.ui.spec.js @@ -73,6 +73,12 @@ describe('TableSchema', () => { it('canEditDeleteRowColumns', () => { expect(schemaObj.canEditDeleteRowColumns({inheritedfrom: 1234})).toBe(false); expect(schemaObj.canEditDeleteRowColumns({inheritedfrom: null})).toBe(true); + + // Column already inherited from a parent table when the table was + // opened (issue #10179, case 1) - the row's edit/delete buttons must + // be disabled too. + expect(schemaObj.canEditDeleteRowColumns({inheritedfromtable: 'public.parent'})).toBe(false); + expect(schemaObj.canEditDeleteRowColumns({inheritedfromtable: null})).toBe(true); }); it('LikeSchema typname change', () => {