From 8cf316b853176df9d97a4b024389a4bf6e1bd95c Mon Sep 17 00:00:00 2001 From: Ashesh Vashi Date: Mon, 19 Aug 2024 09:53:00 +0530 Subject: [PATCH] Fixed some of the issues reported due to introduction of the custom hook 'useSchemaState'. #7776 --- .../publications/static/js/publication.ui.js | 7 ++-- .../static/js/domain_constraints.ui.js | 2 +- .../schemas/packages/static/js/package.ui.js | 10 ++++-- .../tables/indexes/static/js/index.ui.js | 4 ++- .../static/js/SchemaView/SchemaDialogView.jsx | 10 ++++-- .../static/js/SchemaView/base_schema.ui.js | 12 +++++-- .../static/js/SchemaView/useSchemaState.js | 34 ++++++++++++------- .../schema_ui_files/column.ui.spec.js | 5 +-- .../schema_ui_files/packages.ui.spec.js | 11 +++--- .../partition.utils.ui.spec.js | 10 +++--- .../javascript/schema_ui_files/utils.js | 19 +++++++++++ 11 files changed, 88 insertions(+), 36 deletions(-) create mode 100644 web/regression/javascript/schema_ui_files/utils.js diff --git a/web/pgadmin/browser/server_groups/servers/databases/publications/static/js/publication.ui.js b/web/pgadmin/browser/server_groups/servers/databases/publications/static/js/publication.ui.js index fa3ece2e4..e174d7acb 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/publications/static/js/publication.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/publications/static/js/publication.ui.js @@ -207,7 +207,10 @@ export default class PublicationSchema extends BaseUISchema { state.only_table = false; return true; } - if (!_.isUndefined(table) && table.length > 0 && !_.isEqual(this._origData.pubtable, state.pubtable)){ + if ( + !_.isUndefined(table) && table.length > 0 && + !_.isEqual(this.origData.pubtable, state.pubtable) + ){ return false; } state.only_table = false; @@ -304,4 +307,4 @@ export default class PublicationSchema extends BaseUISchema { }, ]; } -} \ No newline at end of file +} diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/domain_constraints/static/js/domain_constraints.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/domain_constraints/static/js/domain_constraints.ui.js index 7d7140a96..cae3a8c7b 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/domain_constraints/static/js/domain_constraints.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/domains/domain_constraints/static/js/domain_constraints.ui.js @@ -51,7 +51,7 @@ export default class DomainConstraintSchema extends BaseUISchema { cell:'boolean', group: gettext('Definition'), min_version: 90200, mode: ['properties', 'create', 'edit'], readonly: function(state) { - return !obj.isNew(state) && obj._origData.convalidated; + return !obj.isNew(state) && obj.origData.convalidated; } } ]; diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/packages/static/js/package.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/packages/static/js/package.ui.js index 5f19ec329..f48b752d5 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/packages/static/js/package.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/packages/static/js/package.ui.js @@ -100,7 +100,10 @@ export default class PackageSchema extends BaseUISchema { group: gettext('Header'), depChange: (state, source, topState, actionObj) => { - if(packageSchemaObj._origData.oid && state.pkgheadsrc != actionObj.oldState.pkgheadsrc) { + if( + packageSchemaObj.origData.oid && + state.pkgheadsrc != actionObj.oldState.pkgheadsrc + ) { packageSchemaObj.warningText = gettext( 'Updating the package header definition may remove its existing body.' ) + '

' + gettext('Do you want to continue?') + @@ -116,7 +119,10 @@ export default class PackageSchema extends BaseUISchema { mode: ['properties', 'create', 'edit'], group: gettext('Body'), depChange: (state, source, topState, actionObj) => { - if(packageSchemaObj._origData.oid && state.pkgbodysrc != actionObj.oldState.pkgbodysrc) { + if( + packageSchemaObj.origData.oid && + state.pkgbodysrc != actionObj.oldState.pkgbodysrc + ) { packageSchemaObj.warningText = gettext( 'Updating the package header definition may remove its existing body.' ) + '

' + gettext('Do you want to continue?') + diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/static/js/index.ui.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/static/js/index.ui.js index e56f6a908..650dca4a5 100644 --- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/static/js/index.ui.js +++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/indexes/static/js/index.ui.js @@ -155,7 +155,9 @@ class IndexColumnSchema extends BaseUISchema { * to access method selected by user if not selected * send btree related op_class options */ - let amname = obj._top?._sessData ? obj._top?._sessData.amname : obj._top?._origData.amname; + let amname = obj._top?._sessData ? + obj._top?._sessData.amname : + obj._top?.origData.amname; if(_.isUndefined(amname)) return options; diff --git a/web/pgadmin/static/js/SchemaView/SchemaDialogView.jsx b/web/pgadmin/static/js/SchemaView/SchemaDialogView.jsx index 6283ae2e6..e7983b376 100644 --- a/web/pgadmin/static/js/SchemaView/SchemaDialogView.jsx +++ b/web/pgadmin/static/js/SchemaView/SchemaDialogView.jsx @@ -102,6 +102,12 @@ export default function SchemaDialogView({ updateSchemaState(schemaState); }, [sessData.__changeId]); + useEffect(()=>{ + if (!props.resetKey) return; + reset(); + }, [props.resetKey]); + + const onResetClick = () => { const resetIt = () => { firstEleRef.current?.focus(); @@ -151,14 +157,14 @@ export default function SchemaDialogView({ setLoaderText('Saving...'); if (!schema.warningText) { - save(schemaState.changes); + save(schemaState.Changes(true)); return; } Notifier.confirm( gettext('Warning'), schema.warningText, - ()=> { save(schemaState.changes); }, + ()=> { save(schemaState.Changes(true)); }, () => { setSaving(false); setLoaderText(''); diff --git a/web/pgadmin/static/js/SchemaView/base_schema.ui.js b/web/pgadmin/static/js/SchemaView/base_schema.ui.js index f42b5599e..4c6578a57 100644 --- a/web/pgadmin/static/js/SchemaView/base_schema.ui.js +++ b/web/pgadmin/static/js/SchemaView/base_schema.ui.js @@ -38,11 +38,11 @@ export default class BaseUISchema { /* The original data before any changes */ set origData(val) { - this._origData = val; + throw new Error('Property \'origData\' is readonly.'); } get origData() { - return this._origData || {}; + return this.state?.initData || {}; } set state(state) { @@ -53,6 +53,14 @@ export default class BaseUISchema { return this._state; } + get _sessData() { + return this._state?.data; + } + + set _sessData(val) { + throw new Error('Property _sessData is readonly.', val); + } + /* * The session data, can be useful but setting this will not affect UI. * this._sessData is set by SchemaView directly. set sessData should not be diff --git a/web/pgadmin/static/js/SchemaView/useSchemaState.js b/web/pgadmin/static/js/SchemaView/useSchemaState.js index 103296699..7bfc0308b 100644 --- a/web/pgadmin/static/js/SchemaView/useSchemaState.js +++ b/web/pgadmin/static/js/SchemaView/useSchemaState.js @@ -179,7 +179,7 @@ const LOADING_STATE = { ERROR: 'Error' }; -class SchemaState extends DepListener { +export class SchemaState extends DepListener { constructor( schema, getInitData, immutableData, mode, keepCid, onDataChange @@ -309,7 +309,7 @@ class SchemaState extends DepListener { // If schema does not have the data or does not have any 'onDataChange' // callback, there is no need to validate the current data. - if(!state.isReady || !state.onDataChange) return; + if(!state.isReady) return; if( !validateSchema(schema, sessData, (path, message) => { @@ -317,12 +317,22 @@ class SchemaState extends DepListener { }) ) state.setError({}); + state.data = sessData; + state.changes = state.Changes(); + state.onDataChange && state.onDataChange(state.hasChanges, state.changes); + } + + Changes(includeSkipChange=false) { + const state = this; + const sessData = this.data; + const schema = state.schema; + // Check if anything changed. let dataDiff = getSchemaDataDiff( schema, state.initData, sessData, - state.mode, state.keepCid, false, false + state.mode, state.keepCid, false, includeSkipChange ); - const hasDataChanged = state.hasChanges = Object.keys(dataDiff).length > 0; + state.hasChanges = Object.keys(dataDiff).length > 0; // Inform the callbacks about change in the data. if(state.mode !== 'edit') { @@ -331,21 +341,19 @@ class SchemaState extends DepListener { // Remove internal '__changeId' attribute. delete dataDiff.__changeId; + // In case of 'non-edit' mode, changes are always there. - state.changes = dataDiff; - } else if (hasDataChanged) { + return dataDiff; + } else if (state.hasChanges) { const idAttr = schema.idAttribute; const idVal = state.initData[idAttr]; // Append 'idAttr' only if it actually exists if (idVal) dataDiff[idAttr] = idVal; - state.changes = dataDiff; - } else { - state.changes = null; + + return dataDiff; } - state.data = sessData; - - state.onDataChange(hasDataChanged, dataDiff); + return {}; } get isNew() { @@ -467,7 +475,7 @@ export const useSchemaState = ({ }); }); }); - }, [schemaState.__deferred__?.length]); + }, [sessData.__deferred__?.length]); schemaState.reload = reload; schemaState.reset = resetData; 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 6121a5ca0..95b36e84e 100644 --- a/web/regression/javascript/schema_ui_files/column.ui.spec.js +++ b/web/regression/javascript/schema_ui_files/column.ui.spec.js @@ -12,6 +12,7 @@ import ColumnSchema from '../../../pgadmin/browser/server_groups/servers/databas import BaseUISchema from '../../../pgadmin/static/js/SchemaView/base_schema.ui'; import _ from 'lodash'; import {addNewDatagridRow, genericBeforeEach, getCreateView, getEditView, getPropertiesView} from '../genericFunctions'; +import { initializeSchemaWithData } from './utils'; class MockSchema extends BaseUISchema { get baseFields() { @@ -188,7 +189,7 @@ describe('ColumnSchema', ()=>{ state.attnum = 1; state.attidentity = 'a'; state.colconstype = 'i'; - schemaObj.origData = {attidentity:'a'}; + initializeSchemaWithData(schemaObj, {attidentity:'a'}); schemaObj.validate(state, setError); expect(setError).toHaveBeenCalledWith('seqincrement', 'Increment value cannot be empty.'); @@ -208,7 +209,7 @@ describe('ColumnSchema', ()=>{ state.attnum = null; state.seqmin = null; state.seqmax = null; - schemaObj.origData.attidentity = undefined; + initializeSchemaWithData(schemaObj, {attidentity: undefined}); expect(schemaObj.validate(state, setError)).toBe(false); state.seqmin = 3; diff --git a/web/regression/javascript/schema_ui_files/packages.ui.spec.js b/web/regression/javascript/schema_ui_files/packages.ui.spec.js index 1d21b0f47..ea3e44871 100644 --- a/web/regression/javascript/schema_ui_files/packages.ui.spec.js +++ b/web/regression/javascript/schema_ui_files/packages.ui.spec.js @@ -11,6 +11,7 @@ import { getNodePrivilegeRoleSchema } from '../../../pgadmin/browser/server_groups/servers/static/js/privilege.ui'; import PackageSchema from '../../../pgadmin/browser/server_groups/servers/databases/schemas/packages/static/js/package.ui'; import {genericBeforeEach, getCreateView, getEditView, getPropertiesView} from '../genericFunctions'; +import { initializeSchemaWithData } from './utils'; describe('PackageSchema', ()=>{ @@ -49,9 +50,9 @@ describe('PackageSchema', ()=>{ pkgheadsrc: 'changed text' }; packageSchemaObj.warningText = null; - packageSchemaObj._origData = { - oid: '123' - }; + + initializeSchemaWithData(packageSchemaObj, { oid: '123' }); + let actionObj = { oldState: { pkgheadsrc: 'original text' @@ -69,9 +70,7 @@ describe('PackageSchema', ()=>{ pkgheadsrc: 'changed text' }; packageSchemaObj.warningText = null; - packageSchemaObj._origData = { - oid: '123' - }; + initializeSchemaWithData(packageSchemaObj, { oid: '123' }); let actionObj = { oldState: { pkgbodysrc: 'original text' diff --git a/web/regression/javascript/schema_ui_files/partition.utils.ui.spec.js b/web/regression/javascript/schema_ui_files/partition.utils.ui.spec.js index 9e9c8d26d..eca02bd6a 100644 --- a/web/regression/javascript/schema_ui_files/partition.utils.ui.spec.js +++ b/web/regression/javascript/schema_ui_files/partition.utils.ui.spec.js @@ -13,6 +13,7 @@ import _ from 'lodash'; import * as nodeAjax from '../../../pgadmin/browser/static/js/node_ajax'; import { PartitionKeysSchema, PartitionsSchema } from '../../../pgadmin/browser/server_groups/servers/databases/schemas/tables/static/js/partition.utils.ui'; import {addNewDatagridRow, genericBeforeEach, getCreateView, getEditView, getPropertiesView} from '../genericFunctions'; +import { initializeSchemaWithData } from './utils'; function getFieldDepChange(schema, id) { return _.find(schema.fields, (f)=>f.id==id)?.depChange; @@ -160,9 +161,8 @@ describe('PartitionsSchema', ()=>{ state.is_sub_partitioned = false; state.is_default = false; - schemaObj.top._sessData = { - partition_type: 'range', - }; + initializeSchemaWithData(schemaObj.top, {partition_type: 'range'}); + schemaObj.validate(state, setError); expect(setError).toHaveBeenCalledWith('values_from', 'For range partition From field cannot be empty.'); @@ -170,11 +170,11 @@ describe('PartitionsSchema', ()=>{ schemaObj.validate(state, setError); expect(setError).toHaveBeenCalledWith('values_to', 'For range partition To field cannot be empty.'); - schemaObj.top._sessData.partition_type = 'list'; + initializeSchemaWithData(schemaObj.top, {partition_type: 'list'}); schemaObj.validate(state, setError); expect(setError).toHaveBeenCalledWith('values_in', 'For list partition In field cannot be empty.'); - schemaObj.top._sessData.partition_type = 'hash'; + initializeSchemaWithData(schemaObj.top, {partition_type: 'hash'}); schemaObj.validate(state, setError); expect(setError).toHaveBeenCalledWith('values_modulus', 'For hash partition Modulus field cannot be empty.'); diff --git a/web/regression/javascript/schema_ui_files/utils.js b/web/regression/javascript/schema_ui_files/utils.js new file mode 100644 index 000000000..7099f1b6a --- /dev/null +++ b/web/regression/javascript/schema_ui_files/utils.js @@ -0,0 +1,19 @@ +///////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2024, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////// + +import { SchemaState } from '../../../pgadmin/static/js/SchemaView/useSchemaState'; + +export function initializeSchemaWithData(schema, data) { + const state = schema.state = new SchemaState( + schema, null, null, 'create', false, null + ); + state.initData = data; + state.data = data; +} +