Fix inherited-column read-only state and Definition-tab type restriction in the Table dialog

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: '',

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) {

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,

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 = {};

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', () => {