Skip to content

refactor(reports): use AttributeEvaluatedValue for ATA alert values - #1104

Merged
Starosdev merged 1 commit into
Starosdev:developfrom
RyoKusnadi:refactor/report-use-evaluated-value
Nov 5, 2026
Merged

Starosdev merged 1 commit into
Starosdev:developfrom
RyoKusnadi:refactor/report-use-evaluated-value

Conversation

@RyoKusnadi

Copy link
Copy Markdown

Summary

Follow-up to #1085 and #1086, as @Starosdev asked in the approval comment. The report's alert line now takes its ATA value from measurements.AttributeEvaluatedValue, and the extra exported SmartAtaAttribute.ThresholdValue is removed. No change in what the report prints.

Problem: None visible to users. This is a cleanup. Until #1086 landed, the report code had its own copy of "which ATA field does the status use", through an exported method added only for it.

How it happens: Not a bug. It was a stop-gap, noted in a comment in alertAttributeDetails.

Cause: #1085 merged before AttributeEvaluatedValue existed, so it called ThresholdValue, which gives the same result.

Type of Change

  • Refactoring (no functional changes)

Related Issues

Follow-up to #1085 and #1086. No issue.

Changes Made

  • generator.go: for an ATA attribute with metadata, value comes from AttributeEvaluatedValue. The ATA-without-metadata rule (normalized value, as the detail page shows) stays, as the helper would give the raw value there. Comment updated.
  • smart_ata_attribute.go: removed ThresholdValue. Nothing else called it (searched the repo). The private thresholdValue stays.

Scope: The nullable threshold is a separate PR (#1103). It touches nearby lines in alertAttributeDetails, so the second to merge may need a small rebase.

Testing

  • I have tested this locally
  • I have added/updated tests that prove my fix/feature works

Added TestAlertAttributeDetails_ATAValueMatchesEvaluatedValue: for every ATA attribute with metadata, the alert value equals AttributeEvaluatedValue. As this is a refactor it passes before and after; the existing TestAlertAttributeDetails cases pass unchanged.

Commands: go vet and go test on webapp/backend/pkg/reports and webapp/backend/pkg/models/measurements (pass), gofmt -l on the changed files (clean). go vet also on pkg/notify and pkg/web/.... I did not run the full suite.

Replace the report's call to SmartAtaAttribute.ThresholdValue with measurements.AttributeEvaluatedValue and remove the exported method, which nothing else used.

Signed-off-by: Ryo Kusnadi <ryokusnadi@gmail.com>
@RyoKusnadi
RyoKusnadi requested a review from Starosdev as a code owner November 4, 2026 22:54
@RyoKusnadi

Copy link
Copy Markdown
Author

@Starosdev here is the follow-up you asked for in #1085 (switch to AttributeEvaluatedValue and drop ThresholdValue). Could you please review it when you have a moment? Thank you!

@Starosdev Starosdev left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Approving at head 526bc954. Source-traced; CI green. AttributeEvaluatedValue resolves AtaMetadata[id].DisplayType exactly as the old ThresholdValue(metadata.DisplayType) call did, so output is unchanged, including the RawValue fallback for 174, 204, 227 and 228. ThresholdValue has no callers left. Two optional nits, no change needed: line 120 of the doc comment is wider than the rest of the block, and a case asserting RawValue for attribute 174 would pin the claim in that comment directly. Merging this first; #1103 will rebase onto it.

@Starosdev
Starosdev merged commit aa4e266 into Starosdev:develop Nov 5, 2026
20 checks passed
Starosdev pushed a commit that referenced this pull request Nov 5, 2026
## [1.79.3](v1.79.2...v1.79.3) (2026-11-05)

### Bug Fixes

* **release:** restore changelog bodies and add a release dry run ([#1113](#1113)) ([ee517f2](ee517f2)), closes [#1112](#1112)
* **reports:** count warning devices from active failures in report generator ([#1107](#1107)) ([1c80d1a](1c80d1a)), closes [#1105](#1105)

### Refactoring

* **reports:** use AttributeEvaluatedValue for ATA alert values ([#1104](#1104)) ([aa4e266](aa4e266)), closes [#1085](#1085) [#1086](#1086) [#1086](#1086) [#1085](#1085) [#1103](#1103)
@RyoKusnadi
RyoKusnadi deleted the refactor/report-use-evaluated-value branch November 5, 2026 12:37
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.

2 participants