Switch RemoteXDA slices to Controllers using Generic Components by nbeatty-gpa · Pull Request #953 · GridProtectionAlliance/SystemCenter

nbeatty-gpa · 2026-09-18T20:23:14Z

Adds Generic Components for the tabbed record layouts to simplify future changes and switches RemoteXDA components from slices to controllers.

nbeatty-gpa · 2026-09-25T14:25:06Z

-        setFilters([]); // initialize filter list, which should add additional filters
-    }, [])
+        if (props.Filters != null) setFilters(props.Filters)
+        else setFilters([]); // initialize filter list, which should add additional filters


This is necessary to make sure the Filters passed into ControllerSelectPopup are reflected in the Searchbar. Without this change, the Filters passed into ControllerSelectPopup aren't reflected in the Searchbar, even after different filters are added or removed.

prestoncraw · 2026-09-30T15:03:01Z

+    let cardBody;
+    if (pagedStatus === 'error') {
+        cardBody = <ServerErrorIcon Show={true} Size={40} Label={'A Server Error Occurred. Please Reload the Application.'} />
+    } else if (pagedStatus === 'loading') {
+        cardBody = <LoadingScreen Show={true} />
+    } else {
+        cardBody =
+            <>
+                <div className="row d-flex flex-column" style={{ flex: 1, overflow: 'hidden' }}>


The return (...) here is nested inside the final else of the cardBody chain, so when pagedStatus is 'loading' or 'error' the component returns undefined. The card (header, table, Add button, modals) disappears on every page change, sort, edit and delete, and a failed request shows nothing instead of the ServerErrorIcon.

Can the return move outside the if/else so cardBody is actually used?

prestoncraw · 2026-09-30T15:05:41Z

    React.useEffect(() => {
-        setFilters([]); // initialize filter list, which should add additional filters
-    }, [])
+        if (props.Filters != null) setFilters(props.Filters)
+        else setFilters([]); // initialize filter list, which should add additional filters


Filters only seeds the popup's filters state, and the SearchBar writes that same state. As soon as the user searches, noSameFilter is replaced, so meters already on this instance show up again and can be added a second time.

On master, DefaultSelects.Meter appended AddlFilters to the search filters. Could ControllerSelectPopup treat these as always-applied base filters and combine them with the search filters when it queries?

Something like:

const [searchFilters, setSearchFilters] = React.useState<Search.IFilter<T>[]>([]); const baseFilters = useStringMemonization(props.BaseFilters ?? []); const filters = React.useMemo(() => [...baseFilters, ...searchFilters], [baseFilters, searchFilters]);

Then the [props.Filters] effect can go, and the search effect can skip while !props.Show. Renaming the prop to BaseFilters would also make it clear they're always applied.

prestoncraw · 2026-09-30T15:06:58Z

+    { Id: "remoteAsset", Label: "Remote Asset", Content: (rec) => <RemoteAssetTab ID={rec.ID} /> }
+]

+function RemoteXDAInstance({Roles, ID, Tab }: IProps) {


Tab is destructured but not used, and GenericRecord has no initial-tab prop, so ?Tab=remoteAsset links now open whatever tab is in sessionStorage. The old getTab() gave props.Tab priority.

Could GenericRecord take an optional InitialTab that overrides the stored tab? Meter, Asset, etc. will need this when they move over.

prestoncraw · 2026-09-30T15:13:15Z

@@ -0,0 +1,46 @@
+//******************************************************************************************************


Is the context needed? It has one consumer (GenericInfo) two levels down, duplicates what Content(...) already passes, and OriginalRecord, GetName, RefreshCount and SetRefreshCount aren't read anywhere.

I thought it necessary because there were many props passed down two or three component layers without being needed by all of them. It would also be useful for implementing a genericized form down the line. I'm also thinking Content(...) could be simplified to passing in just the context.

prestoncraw · 2026-09-30T15:14:26Z

+            url: apiPath + recordID.toString(),
+            contentType: "application/json; charset=utf-8",
+            dataType: 'json',
+            cache: true,


We dont want to cache the responses here.

prestoncraw · 2026-09-30T15:25:44Z

+interface U {
+    ID: number | string
+}


The U interface + T extends U forces every record type to have an ID field, and it's only used for KeySelector. Could this take the key from the caller instead, e.g. PrimaryKey: keyof T (or a KeySelector function like the gemstone Table takes), and drop the constraint?

prestoncraw · 2026-09-30T15:26:42Z

+    <TypeScriptCompile Include="wwwroot\Scripts\TSX\SystemCenter\CommonComponents\ControllerSelectPopup.tsx" />
    <TypeScriptCompile Include="wwwroot\Scripts\TSX\SystemCenter\CommonComponents\ExtDBTaskStatusModal.tsx" />
    <TypeScriptCompile Include="wwwroot\Scripts\TSX\SystemCenter\CommonComponents\FileGroupAnalysisJobPriority.tsx" />
+    <TypeScriptCompile Include="wwwroot\Scripts\TSX\SystemCenter\CommonComponents\GenericForm.tsx" />


GenericForm.tsx is added to the csproj here, but the file isn't in the PR.

prestoncraw · 2026-09-30T15:31:13Z

+    React.useEffect(() => {
+        if (selectedRecord == null) return
+        Object.keys(selectedRecord).forEach((key) => {
+            if (!(key in (selectedRecord as object))) return
+            const originalValue = originalRecord[key];
+            const warning = `Changes to ${key} will be lost.`;
+            if (originalValue != selectedRecord[key]) setWarnings(warns => warns.findIndex(warn => warn === warning) < 0 ? [...warns, warning] : warns);
+            else setWarnings(warnings => warnings.filter(warn => warn != warning))
+        })
+    }, [originalRecord, selectedRecord])


warnings can be computed from originalRecord and selectedRecord with a useMemo instead of stored and updated by an effect (which also calls setWarnings once per field). Right now forms can also write it through setChanged, so there are two writers for the same flag. Could we compute it and drop SetWarnings?

https://react.dev/learn/you-might-not-need-an-effect#updating-state-based-on-props-or-state

prestoncraw · 2026-09-30T15:32:41Z

 import * as _ from "lodash";
 import { ReactIcons } from "@gpa-gemstone/gpa-symbols";

 interface U { ID: number | string }


Same ID constraint as GenericRelation, and here ID is also hard-coded for the default sort, uniqBy and selection matching. Since this PR is already changing the popup's props, could we add the key prop here too? I missed this during the original review of this component.

prestoncraw · 2026-09-30T15:37:01Z

+        else return datum.item[datum.field] ?? <></>
+    }, []);
+
+    // needs to be a use effect?


Was this left on accident? were you planning on do something different?

Copilot

Copilot review overview

🟡 Changes recommended

Tab routing, fixed filtering, permissions, and paging contain functional regressions.

Review effort: Balanced
Findings: 6 · 1

Open (7)

Preserve fixed exclusion filters during searches · New
Enforce permissions in button state and click handler · New
Clamp page after row deletion or refresh · New
Use numeric filter type for DetailedMeter IDs · New
Preserve explicit initial tab route parameter · New
Disable caching for post-patch record refreshes · New
Correct misspelled user-facing error message · New

What changed in this PR

Refactors RemoteXDA record pages to use reusable controller-backed components instead of Redux slices.

Changes:

Adds generic record, information, relation, context, and paging utilities.
Migrates RemoteXDA instance, meter, and asset views to generic controllers.
Updates Gemstone dependencies and project compilation entries.

File	Description
`Store/Store.ts`	Removes RemoteXDA slices.
`RemoteXDA/SystemSettingsTab.tsx`	Uses generic information layout.
`RemoteXDA/RemoteXDAInstance.tsx`	Uses generic record layout.
`RemoteXDA/RemoteMeterTab.tsx`	Migrates meter relations to controllers.
`RemoteXDA/RemoteAssetTab.tsx`	Migrates asset relations to controllers.
`hooks.ts`	Adds controller paging and record-fetch hooks.
`global.d.ts`	Defines record context types.
`CommonComponents/RecordContext.tsx`	Adds shared record context.
`CommonComponents/GenericRelation.tsx`	Adds reusable relation tables.
`CommonComponents/GenericRecord.tsx`	Adds reusable tabbed record layout.
`CommonComponents/GenericInfo.tsx`	Adds reusable record editor layout.
`CommonComponents/ControllerSelectPopup.tsx`	Adds fixed-filter support.
`SystemCenter.csproj`	Includes generic components for compilation.
`package.json`	Updates Gemstone dependencies.
`package-lock.json`	Locks updated dependency graph.

Files not reviewed (1)

Source/Applications/SystemCenter/package-lock.json: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

    React.useEffect(() => {
-        setFilters([]); // initialize filter list, which should add additional filters
-    }, [])
+        if (props.Filters != null) setFilters(props.Filters)


+                    <button className={"btn btn-primary" + (context.Warnings.length == 0 || context.Errors.length > 0 ? ' disabled' : '')} onClick={() => {
+                        if (context.Warnings.length > 0 && context.Errors.length == 0) {
+                           context.Patch();
+                        }
+                    }}


+    const [editRecord, setEditRecord] = React.useState<T>(BlankRecord);
+    const [editRecordErrors, setEditRecordErrors] = React.useState<string[]>([]);
+
+    const { Data: pagedData, Status: pagedStatus, RecordsPerPage: recordsPerPage, TotalPages: totalPages, TotalRecords: totalRecords } = usePagedSearch<T>(Controller, Filters, page, sortField, ascending, ParentID, refreshCount);


+        return <LoadingScreen Show={true} />;
+
+    if (originalRecordStatus == 'error')
+        return <ServerErrorIcon Show={true} Label={"Error occured while retrieving record."} />;


nbeatty-gpa changed the title ~~Switch to controller2~~ Switch RemoteXDA slices to Controllers using Generic Components Sep 18, 2026

nbeatty-gpa force-pushed the switchToController2 branch from 38a443f to beb4265 Compare September 21, 2026 14:22

nbeatty-gpa requested a review from prestoncraw September 22, 2026 16:11

nbeatty-gpa mentioned this pull request Sep 23, 2026

Add AssetGroups and Customers tabs to Asset and Meter records. #956

Draft

nbeatty-gpa force-pushed the switchToController2 branch from a23d632 to 9611dc4 Compare September 25, 2026 13:46

nbeatty-gpa commented Sep 25, 2026

View reviewed changes

nbeatty-gpa added 6 commits September 29, 2026 13:51

add hooks for PagedSearch and GetOne

61c6285

create generic components for tabbed record layouts

aad1067

add default filter prop to controller select popup

16efe7c

use new generic components for remote xda tabbed record layout

94e3bdb

remove remote xda and other unused slices and unused imports

213e6b4

update projects and dependencies

099a869

nbeatty-gpa force-pushed the switchToController2 branch from 9611dc4 to 099a869 Compare September 29, 2026 17:51

prestoncraw requested changes Sep 30, 2026

View reviewed changes

elwills requested a balanced review from Copilot September 30, 2026 15:40

Copilot started reviewing on behalf of elwills September 30, 2026 15:41 View session

Copilot AI reviewed Sep 30, 2026

View reviewed changes

		@@ -0,0 +1,46 @@
		//******************************************************************************************************

Conversation

nbeatty-gpa commented Sep 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants