fix(overrides): address gaps in attribute override system (#345)#346
Merged
fix(overrides): address gaps in attribute override system (#345)#346
Conversation
- Add migration m20260411000000 to enforce unique constraint on (protocol, attribute_id, wwn) in attribute_overrides, deduplicating any existing rows before creating the index - Update AttributeOverride model to use uniqueIndex tag to match - Add GetAllOverridesForDisplay repo method and mock; GET /api/settings/overrides now returns both UI and config file overrides so users can see everything active in the settings panel - Extract validation from SaveAttributeOverride into helpers to keep cognitive complexity within limits; add validation for: at least one threshold required in custom mode, warn_above < fail_above, non-negative thresholds, and WWN format - Fix misleading comment in applier.go that implied thresholds can be combined with force_status; they are mutually exclusive at the call site - Default new override form action to custom threshold so warn/fail fields are visible without requiring an extra dropdown interaction - Add 18 handler unit tests covering GET, POST validation paths, and DELETE
Ensure m20260410000000 (notify_on_collector_error setting) appears before m20260411000000 (attribute_overrides unique index) in the migrations slice, matching chronological order.
Starosdev
added
bug
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
m20260411000000) to enforce a unique constraint on(protocol, attribute_id, wwn)inattribute_overrides, deduplicating any existing rows firstAttributeOverridemodel GORM tags to useuniqueIndexto match the new constraintGetAllOverridesForDisplayrepository method that merges UI overrides (DB) with config-file overrides, so the settings panel shows everything currently active — config overrides appear as read-only (source:config, ID: 0)GetAllOverridesForDisplaySaveAttributeOverrideto extract validation helpers (validateAttributeOverride,validateForceStatus,validateThresholds) to keep cognitive complexity in boundswarn_above < fail_above, non-negative values, and WWN hex-format check for all action typesapplier.gothat implied thresholds can be combined withforce_status— they are mutually exclusive at the call siteignoreGetAttributeOverridesandSaveAttributeOverride(mock-based, no InfluxDB required)Linked Issues
Closes #345
Test plan
go build ./...passes cleanlygo test -v -run TestAttributeOverride ./webapp/backend/pkg/web/handler/...)go test -v ./webapp/backend/pkg/overrides/...)warn >= failcorrectly rejected)