Skip to content

Sc 374 remove timestamp from daily statistics tables date column and correct sort order #847

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nbeatty-gpa wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Sc 374 remove timestamp from daily statistics tables date column and correct sort order #847

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ba253dc
fix sorting issues in miMD and openXDA
nbeatty-gpa Apr 3, 2026
1a5768d
fix sorting issues
nbeatty-gpa Apr 8, 2026
a0b424f
remove unneeded dependencies
nbeatty-gpa Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ...cations/SystemCenter/wwwroot/Scripts/TSX/SystemCenter/DeviceIssuesPage/MiMDIssuesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ function MiMDIssuesPage(props: { Meter: OpenXDA.Types.Meter }) {
const settingStatus = useAppSelector(SystemCenterSettingSlice.Status);

const order = React.useCallback((data: SC.MiMDDailyStatistic[]) => {
return _.orderBy(data, [sortField], [ascending ? 'asc' : 'desc'])
const sortedData = _.sortBy(data, [(e) => { return sortField == 'Date' ? moment(e['Date']) : e[sortField] }])
if (ascending) return sortedData
return sortedData.reverse()
}, [sortField, ascending]);
Comment thread
elwills marked this conversation as resolved.

React.useEffect(() => {
Expand Down
6 changes: 4 additions & 2 deletions ...ions/SystemCenter/wwwroot/Scripts/TSX/SystemCenter/DeviceIssuesPage/OpenXDAIssuesPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ function OpenXDAIssuesPage(props: { Meter: OpenXDA.Types.Meter }) {
const [ascending, setAscending] = React.useState<boolean>(false);

const order = React.useCallback((data: SC.OpenXDADailyStatistic[]) => {
return _.orderBy(data, [sortField], [ascending ? 'asc' : 'desc'])
const sortedData = _.sortBy(data, [(e) => { return sortField == 'Date' ? moment(e['Date']) : e[sortField] }])
if (ascending) return sortedData
return sortedData.reverse()
}, [sortField, ascending]);

React.useEffect(() => {
Expand All @@ -52,7 +54,7 @@ function OpenXDAIssuesPage(props: { Meter: OpenXDA.Types.Meter }) {
return () => {
if (handle.abort != undefined) handle.abort();
}
}, [props.Meter.AssetKey]);
}, [props.Meter.AssetKey, ascending, sortField]);
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

The data-fetching effect is now re-running whenever ascending or sortField changes, which will trigger a new PagedSearch call on every client-side sort toggle. Since you already re-sort locally via the order callback (and the separate effect that runs when order changes), this dependency expansion adds unnecessary network load and can cause request abort/retry churn while the user is sorting.

Consider limiting this effect’s dependency array back to just props.Meter.AssetKey (and keep client-side sorting handled by the order/re-sort effect).

Suggested change
}, [props.Meter.AssetKey, ascending, sortField]);
}, [props.Meter.AssetKey]);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@clackner-gpa clackner-gpa Apr 10, 2026

Choose a reason for hiding this comment

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

Comment is valid Solution does not align with the rest of the codebase.


React.useEffect(() => {
if (data.length === 0) return;
Expand Down