Skip to content

Fix report $explain_ tooltips when $name_ is also set#3877

Merged
lunkwill42 merged 1 commit into5.17.xfrom
fix/report-explain-tooltip
Mar 23, 2026
Merged

Fix report $explain_ tooltips when $name_ is also set#3877
lunkwill42 merged 1 commit into5.17.xfrom
fix/report-explain-tooltip

Conversation

@lunkwill42
Copy link
Copy Markdown
Member

@lunkwill42 lunkwill42 commented Mar 19, 2026

Scope and purpose

Fixes #3876.

The $explain_ directive in the report configuration is supposed to add a tooltip to column headers, but the tooltip never appears when the column also has a $name_ override. The explain dict is keyed by the original SQL field name, but the lookup in make_table_headers happened after the field name had already been replaced by the display name from $name_. This caused the lookup to silently miss for any column that uses both directives (13 of 24 existing $explain_ usages are affected).

The fix is to look up the explanation before overriding the title.

Contributor Checklist

  • Added a changelog fragment for towncrier
  • Added/amended tests for new/changed code
  • Added/changed documentation
  • Linted/formatted the code with ruff, easiest by using pre-commit
  • Wrote the commit message so that the first line continues the sentence "If applied, this commit will ...", starts with a capital letter, does not end with punctuation and is 50 characters or less long. See https://cbea.ms/git-commit/
  • Based this pull request on the correct upstream branch: For a patch/bugfix affecting the latest stable version, it should be based on that version's branch (<major>.<minor>.x). For a new feature or other additions, it should be based on master.
  • If applicable: Created new issues if this PR does not fix the issue completely/there is further work to be done
  • If it's not obvious from a linked issue, described how to interact with NAV in order for a reviewer to observe the effects of this change first-hand (commands, URLs, UI interactions)
  • If this results in changes in the UI: Added screenshots of the before and after
  • If this adds a new Python source code file: Added the boilerplate header to that file

@lunkwill42
Fix report $explain_ tooltips when $name_ is also set
d8b37dd 
The `explain` dict is keyed by the original SQL field name, but the
lookup happened after `title` was already overridden by the display
name from `$name_`. This caused the lookup to always miss for columns
that have both directives.

Fixes #3876
@lunkwill42 lunkwill42 self-assigned this Mar 19, 2026
@lunkwill42 lunkwill42 requested a review from a team March 19, 2026 11:06
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 19, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 19, 2026

Test results

    20 files      20 suites   25m 24s ⏱️
 2 895 tests  2 895 ✅ 0 💤 0 ❌
16 542 runs  16 542 ✅ 0 💤 0 ❌

Results for commit d8b37dd.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 19, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.49%. Comparing base (bc4f032) to head (d8b37dd).
⚠️ Report is 6 commits behind head on 5.17.x.

Additional details and impacted files 
@@            Coverage Diff             @@
##           5.17.x    #3877      +/-   ##
==========================================
- Coverage   63.49%   63.49%   -0.01%     
==========================================
  Files         620      620              
  Lines       45923    45923              
  Branches       43       43              
==========================================
- Hits        29160    29158       -2     
- Misses      16753    16755       +2     
  Partials       10       10

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lunkwill42 lunkwill42 changed the title Fix report $explain_ tooltips when $name_ is also set Fix report $explain_ tooltips when $name_ is also set Mar 19, 2026
@lunkwill42 lunkwill42 linked an issue Mar 20, 2026 that may be closed by this pull request
[BUG] Report $explain_ tooltip broken when column also has $name_ override #3876
Closed
johannaengland
johannaengland approved these changes Mar 23, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@johannaengland johannaengland left a comment

Choose a reason for hiding this comment

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

Works great, but you might want to change the issue description, since the sysname column in /report/swport does not have $explain_ - it took me so long to figure it out until I saw that the to_netboxid column for example has a tooltip

@lunkwill42 lunkwill42 merged commit 0ec6133 into 5.17.x Mar 23, 2026
19 checks passed
@lunkwill42 lunkwill42 deleted the fix/report-explain-tooltip branch March 23, 2026 12:55
@lunkwill42 lunkwill42 mentioned this pull request Mar 23, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@johannaengland johannaengland johannaengland approved these changes

Assignees

@lunkwill42 lunkwill42

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Report $explain_ tooltip broken when column also has $name_ override

2 participants

@lunkwill42 @johannaengland