Skip to content

fix(frontend): sort all columns on drive details page (closes #656)#1018

Open
justadityaraj wants to merge 1 commit into
AnalogJ:masterfrom
justadityaraj:fix/drive-details-column-sorting
Open

fix(frontend): sort all columns on drive details page (closes #656)#1018
justadityaraj wants to merge 1 commit into
AnalogJ:masterfrom
justadityaraj:fix/drive-details-column-sorting

Conversation

@justadityaraj

Copy link
Copy Markdown

Summary

Closes #656.

Clicking any column header other than Status, Value, or Threshold on the drive details SMART attributes table re-renders the rows in the same order with no error. The mat-sort-header directive is set on every column, but MatTableDataSource's default sortingDataAccessor does a plain row[columnId] lookup, and most matColumnDef ids in the template don't match field names on SmartAttributeModel:

Column id Field actually displayed
id attribute.attribute_id
name getAttributeName(attribute)metadata.display_name
value getAttributeValue(attribute) — branches on display_type (raw vs transformed vs value)
worst getAttributeWorst(attribute)
thresh getAttributeThreshold(attribute)
ideal getAttributeIdeal(attribute)metadata.ideal
failure attribute.failure_rate
history sparkline chart (no scalar)

So row['id'], row['name'], row['ideal'], row['failure'], row['history'] all resolve to undefined and the sort silently no-ops. Only status, value, and thresh happened to match field names.

Fix

Set sortingDataAccessor on smartAttributeDataSource in ngAfterViewInit to map each column id to the value actually rendered in that cell, reusing the existing getAttribute* getters so the sort key is always exactly what the user sees.

  • attribute_id is coerced via Number(...) so numeric SMART ids sort numerically rather than lexicographically (e.g. 5, 10, 12 not 10, 12, 5).
  • worst/thresh getters can return either a number or '' depending on display type — preserved as-is so empty cells group together.
  • history has no meaningful scalar; exposing the latest observed value gives a deterministic order instead of a no-op when the user clicks the header.
  • default branch falls through to the prior row[id] lookup so any future column id that matches a model field keeps working.

41 lines changed in one file: `webapp/frontend/src/app/modules/detail/detail.component.ts`.

Verification

  • `npm ci && npx ng build` — clean compile
  • `docker build -f docker/Dockerfile.web .` — image builds end-to-end
  • Manually verified each column (ID, Name, Value, Worst, Threshold, Ideal, Failure Rate, History) sorts ascending then descending against a live scrutiny instance on real disks

AI disclosure (per AI_POLICY.md)

This PR was written with Claude Code (Opus 4.7) as a pair-programming assistant. I drove issue selection, read the codebase, decided the fix shape (`sortingDataAccessor` mapping), wrote the column-to-getter mapping, verified the build, and manually tested the sort against my own running scrutiny instance with attached disks. The AI assisted with grepping the codebase, drafting the accessor switch statement, and proofreading. No code was shipped that I haven't read end-to-end.

The drive details SMART attributes table declares matSort on every column
header (id, name, value, worst, thresh, ideal, failure, history) but
sorting only ever worked for status, value, and thresh. The other column
ids do not match field names on SmartAttributeModel — id is attribute_id,
name and ideal come from metadata, failure is failure_rate, and history
is a sparkline chart with no scalar key. MatTableDataSource's default
sortingDataAccessor falls back to row[columnId], which silently returns
undefined for those columns, so clicking the headers re-renders the data
in the same order with no error.

Set sortingDataAccessor on smartAttributeDataSource to map each column
id to the value actually rendered in the corresponding cell, reusing the
existing getAttributeName / getAttributeValue / getAttributeWorst /
getAttributeThreshold / getAttributeIdeal getters so the sort key always
matches what the user sees. attribute_id is coerced to Number so numeric
SMART ids sort numerically rather than lexicographically.

history exposes the latest observed value as a sort key — there is no
meaningful scalar for a sparkline, but a deterministic order is better
than a no-op when the user clicks the header.

Closes AnalogJ#656
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 21.51%. Comparing base (60947e8) to head (0611bd0).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1018   +/-   ##
=======================================
  Coverage   21.51%   21.51%           
=======================================
  Files          58       58           
  Lines        3346     3346           
=======================================
  Hits          720      720           
  Misses       2587     2587           
  Partials       39       39           
Flag Coverage Δ
unittests 21.51% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kaysond

kaysond commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the contribution! Can you post a couple screenshots or screen recording of the fix?

@justadityaraj

Copy link
Copy Markdown
Author

Hi @kaysond — checking in on this one after a while. Anything needed from my side to help move it along? Happy to rebase or adjust if useful.

@kaysond

kaysond commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Hi @kaysond — checking in on this one after a while. Anything needed from my side to help move it along? Happy to rebase or adjust if useful.

Haven't had time for testing. If you can post a screenshot/video of the fix I'll merge it

@justadityaraj

Copy link
Copy Markdown
Author

Sure @kaysond, will do it by this weekend, in hospital now so will do this once I'm back!

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.

Column sorting does not work in drive details page

3 participants