Skip to content

feat(ui/client): add ability to save metamodels init#1643

Merged
johbaxter merged 2 commits intodevfrom
1559-refresh-data-process
Aug 7, 2025
Merged

feat(ui/client): add ability to save metamodels init#1643
johbaxter merged 2 commits intodevfrom
1559-refresh-data-process

Conversation

@tzylks
Copy link
Copy Markdown
Contributor

@tzylks tzylks commented Aug 4, 2025

Description

Update refresh data logic so that the metamodel saves.

Changes Made

Update calls and create functions related to saving metamodel changes -> adding FE UI button element for running function

How to Test

Navigate to database > metamodel > refresh data > choose tables > click save

Notes

Init change for @neelneelneel to check

@tzylks tzylks requested a review from a team as a code owner August 4, 2025 17:18
@tzylks tzylks linked an issue Aug 4, 2025 that may be closed by this pull request
6 tasks
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 4, 2025

@CodiumAI-Agent /describe

@tzylks
Copy link
Copy Markdown
Contributor Author

tzylks commented Aug 4, 2025

@neelneelneel Here are the initial changes. This should not be considered complete.

@tzylks tzylks requested a review from neelneelneel August 4, 2025 17:18
@QodoAI-Agent
Copy link
Copy Markdown

Title

feat(ui/client): add ability to save metamodels init


User description

Description

Update refresh data logic so that the metamodel saves.

Changes Made

Update calls and create functions related to saving metamodel changes -> adding FE UI button element for running function

How to Test

Navigate to database > metamodel > refresh data > choose tables > click save

Notes

Init change for @neelneelneel to check


PR Type

Enhancement


Description

  • Add "Save" button to metamodel page header

  • Track relationships and enable save after sync

  • Build payload and upload via RdbmsExternalUpload

  • Navigate to saved metamodel page on success


Diagram Walkthrough

flowchart LR
  A["Refresh Data"] -- "open sync modal" --> B["SyncChangesModal"]
  B -- "apply selections" --> C["Update customNodes & relationships"]
  C -- "enable Save button" --> D["Click Save"]
  D -- "onSubmit builds payload" --> E["RdbmsExternalUpload API call"]
  E -- "success response" --> F["Navigate to metamodel page"]
Loading

File Walkthrough

Relevant files
Enhancement
EngineMetadataPage.tsx
Add save logic and UI for metamodel                                           

packages/client/src/pages/engine/EngineMetadataPage.tsx

  • Reorganized imports and added useNavigate
  • Introduced relationships, canSave state hooks
  • Implemented saveDatabase and onSubmit handlers
  • Added "Save" button and conditional disable logic
+500/-391

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 4, 2025

@CodiumAI-Agent /review

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 4, 2025

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Confusing boolean

The use of setCanSave(!true) is unclear; it always sets canSave to false. Use setCanSave(false) for clarity.

setCanSave(!true);
Incorrect payload structure

payloadObj.position is initialized as an array with an empty object, which likely doesn't match the expected API. Verify and correct the expected type for position.

logicalNamesMap: {}, // colName/alias: logicalName
Missing relationships in save

saveDatabase reads relationships from component state but onSubmit only passes payloadObj without relationships. Ensure saveDatabase receives all required fields, including relationships.

const saveDatabase = async (data) => {
	const tables = {};
	const owlPositions = {};

	data.nodes.forEach((node) => {
		const tableInfo = node.data;
		const cols = node.data.properties;
		const firstCol = cols[0].name.replace(/ /g, "_");

		if (!tables[tableInfo.name + "." + firstCol]) {
			const columns = [];

			cols.forEach((col) => {
				columns.push(col.name.replace(/ /g, "_"));
			});

			tables[tableInfo.name + "." + firstCol] = columns;
		}

		if (!owlPositions[node.id]) {
			owlPositions[node.id] = {
				top: node.position.y,
				left: node.position.x,
			};
		}
	});

	const pixel = `RdbmsExternalUpload(database=["${active.id}"], metamodel=[${JSON.stringify({ relationships: relationships, tables: tables })}], existing=[true]); META|SaveOwlPositions(database=["${active.id}"], positionMap=[${JSON.stringify(owlPositions)}]); META|SyncDatabaseWithLocalMaster(database=["${active.id}"])`;

	const resp = await monolithStore.runQuery(pixel);
	const output = resp.pixelReturn[0].output,
		operationType = resp.pixelReturn[0].operationType;

	if (operationType.indexOf("ERROR") > -1) {
		console.warn("RDBMSExternalUpload Reactor bug");
	} else {
		navigate(`/engine/database/${output.database_id}/metamodel`);
		return;
	}
};

const onSubmit = () => {
	const payloadObj = {
		metamodel: {
			relation: [
				// { fromTable: 'id', toTable: 'Drug', relName: 'id_Drug' },
			],
			nodeProp: {
				// tableName: [ // 'colname' EXCLUDING THE FIRST COLUMN
				// ],
			},
		},
		dataTypeMap: {
			// colName: type,
		},
		newHeaders: {}, // oldColName: newColName
		additionalDataTypes: {}, // colName: specificFormat
		descriptionMap: {}, // colName: description
		logicalNamesMap: {}, // colName/alias: logicalName
		position: [{}],
		nodes: customNodes,
	};

	// build metamodel relation for each table
	for (const edge of customEdges) {
		const relName = `${edge.source}_${edge.target}`;
		payloadObj.metamodel.relation.push({
			fromTable: edge.source,
			toTable: edge.target,
			relName: relName,
		});
	}

	// build metamodel node prop

	// build dataTypeMap for each table
	for (const node of customNodes) {
		console.log("node: ", node);
		for (const col of node.data.properties) {
			payloadObj.dataTypeMap[col.name] = col.type;
		}

		payloadObj.metamodel.nodeProp[node.data.name] = [];
	}

	console.log("PAYLOAD OBJ", payloadObj);
	saveDatabase(payloadObj);

@QodoAI-Agent
Copy link
Copy Markdown

QodoAI-Agent commented Aug 4, 2025

PR Code Suggestions ✨

Latest suggestions up to 18e39c4

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fill nodeProp with column names

Populate metamodel.nodeProp with each table's column names (excluding the first one)
instead of leaving it empty. This ensures the payload correctly reflects all non-key
properties for each node.

packages/client/src/pages/engine/EngineMetadataPage.tsx [309-315]

 for (const node of customNodes) {
   console.log("node: ", node);
   for (const col of node.data.properties) {
     payloadObj.dataTypeMap[col.name] = col.type;
   }
-  payloadObj.metamodel.nodeProp[node.data.name] = [];
+  // Skip first column (primary key) and add the rest
+  payloadObj.metamodel.nodeProp[node.data.name] = node.data.properties
+    .slice(1)
+    .map(col => col.name.replace(/ /g, "_"));
 }
Suggestion importance[1-10]: 8

__

Why: Populating metamodel.nodeProp ensures the save payload includes non-key properties, preventing incomplete data on upload.

Medium
General
Escape JSON in queries

Escape or wrap the JSON payloads so that embedded double quotes don’t break the
PIXEL string syntax. This prevents malformed queries when passing complex objects.

packages/client/src/pages/engine/EngineMetadataPage.tsx [261]

-const pixel = `RdbmsExternalUpload(database=["${active.id}"], metamodel=[${JSON.stringify({ relationships: relationships, tables: tables })}], existing=[true]); META|SaveOwlPositions(database=["${active.id}"], positionMap=[${JSON.stringify(owlPositions)}]); META|SyncDatabaseWithLocalMaster(database=["${active.id}"])`;
+const metamodelPayload = JSON.stringify({ relationships, tables }).replace(/"/g, '\\"');
+const positionsPayload = JSON.stringify(owlPositions).replace(/"/g, '\\"');
+const pixel = `RdbmsExternalUpload(database=["${active.id}"], metamodel=["${metamodelPayload}"], existing=[true]); META|SaveOwlPositions(database=["${active.id}"], positionMap=["${positionsPayload}"]); META|SyncDatabaseWithLocalMaster(database=["${active.id}"])`;
Suggestion importance[1-10]: 6

__

Why: Unescaped JSON quotes may break the PIXEL query syntax; escaping prevents malformed commands, though the replacement approach may require refinement.

Low
Simplify canSave state update

Replace the confusing !true literal with an explicit boolean for clarity. This makes
the code more readable and avoids potential misunderstandings about the intended
state.

packages/client/src/pages/engine/EngineMetadataPage.tsx [200]

-setCanSave(!true);
+setCanSave(false);
Suggestion importance[1-10]: 5

__

Why: Using !true is confusing; replacing it with false clarifies intent without changing behavior.

Low

Previous suggestions

Suggestions up to commit 18e39c4
CategorySuggestion                                                                                                                                    Impact
Possible issue
Safeguard loop over possibly null edges

Guard against customEdges being null to avoid runtime errors. Default to an empty
array if it is unset.

packages/client/src/pages/engine/EngineMetadataPage.tsx [297-304]

-for (const edge of customEdges) {
+for (const edge of (customEdges || [])) {
   const relName = `${edge.source}_${edge.target}`;
   payloadObj.metamodel.relation.push({
     fromTable: edge.source,
     toTable: edge.target,
     relName: relName,
   });
 }
Suggestion importance[1-10]: 8

__

Why: Checking customEdges || [] prevents a crash when customEdges is null, improving robustness.

Medium
General
Use explicit boolean instead of !true

Using !true is confusing and error-prone since it always evaluates to false. Replace
it with a literal boolean to clearly indicate intent.

packages/client/src/pages/engine/EngineMetadataPage.tsx [200]

-setCanSave(!true);
+setCanSave(false);
Suggestion importance[1-10]: 4

__

Why: Using false directly is clearer than !true and removes unnecessary negation.

Low

@johbaxter
Copy link
Copy Markdown
Contributor

@RNubla and/or @memisrose do either of you mind testing this with the client databases


// build dataTypeMap for each table
for (const node of customNodes) {
console.log("node: ", node);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment out or remove the console log

payloadObj.metamodel.nodeProp[node.data.name] = [];
}

console.log("PAYLOAD OBJ", payloadObj);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment out console logs

monolithStore.runQuery(pixel).then((response) => {
const output = response.pixelReturn[0]?.output;

console.log("THIS IS OUTPUT", output);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment out or remove the console log

const cols = node.data.properties;
const firstCol = cols[0].name.replace(/ /g, "_");

if (!tables[tableInfo.name + "." + firstCol]) {
Copy link
Copy Markdown
Contributor

@johbaxter johbaxter Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not on you @tzylks, as we asked you to mimic how we do this in legacy. We do this in the legacy ui as well.

This is more a question for the backend. Why do we send tables this way TABLE_NAME._FIRST_COL for RDBMSExternalUpload.

@roxyramos, @EJV20, @ericgonzal8 , @ppatel9703

I feel like we should be sending tables as such

{
TABLE_NAME: ["COL_ONE", COL_TWO"]
}

But it currently does this:

{
TABLE_NAME.COL_ONE: ["COL_ONE", COL_TWO"]
}

@johbaxter
Copy link
Copy Markdown
Contributor

@tzylks Also will likely merge this in as i care about fixing the bug, We need to visit why we do something a certain way with the above reactor.

@RNubla
Copy link
Copy Markdown
Contributor

RNubla commented Aug 7, 2025

This PR passes with my testing of syncing client DB

@johbaxter johbaxter requested review from a team and removed request for neelneelneel August 7, 2025 20:32
@johbaxter johbaxter merged commit a40c3a8 into dev Aug 7, 2025
3 checks passed
@johbaxter johbaxter deleted the 1559-refresh-data-process branch August 7, 2025 21:04
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 7, 2025

@CodiumAI-Agent /update_changelog

@QodoAI-Agent
Copy link
Copy Markdown

Changelog updates: 🔄

2025-08-07 *

Added

  • Metamodel save button and upload flow in the refresh data UI

Changed

  • Refresh data logic now persists metamodel updates

to commit the new content to the CHANGELOG.md file, please type:
'/update_changelog --pr_update_changelog.push_changelog_changes=true'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refresh Data Process

4 participants