Skip to content

Fix adding sensor widget to main page#3394

Merged
johannaengland merged 1 commit intoUninett:masterfrom
johannaengland:bugfix/add-sensor-widget
Jul 30, 2025
Merged

Fix adding sensor widget to main page#3394
johannaengland merged 1 commit intoUninett:masterfrom
johannaengland:bugfix/add-sensor-widget

Conversation

@johannaengland
Copy link
Copy Markdown
Contributor

@johannaengland johannaengland commented Jun 17, 2025

Some sensors have no alert_type specified, which would make this fail. Luckily we already have specified the property alert_type_class which we can simply use here.

https://github.com/Uninett/nav/blob/master/python/nav/models/manage.py#L2609-L2613

Found this bug while working on #3387.

@johannaengland johannaengland requested a review from a team June 17, 2025 12:05
@johannaengland johannaengland self-assigned this Jun 17, 2025
@johannaengland johannaengland mentioned this pull request Jun 17, 2025
Merged
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Jun 17, 2025

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

@johannaengland johannaengland mentioned this pull request Jun 18, 2025
Merged
jorund1
jorund1 reviewed Jul 22, 2025
View reviewed changes
@johannaengland johannaengland requested a review from jorund1 July 23, 2025 06:41
lunkwill42
lunkwill42 approved these changes Jul 29, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

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

The patch looks absolutely fine, but I don't seem to be able to reproduce the mentioned bug, and there's no mention of how to do it...

@johannaengland
Copy link
Copy Markdown
Contributor Author

johannaengland commented Jul 29, 2025 •
edited
Loading

The patch looks absolutely fine, but I don't seem to be able to reproduce the mentioned bug, and there's no mention of how to do it...

If you find a sensor that does not have alert_type specified, then you can reproduce this by going to http://localhost/ipdevinfo/sensor/1485 and then click on "Add to dashboard"

@lunkwill42
Copy link
Copy Markdown
Member

lunkwill42 commented Jul 29, 2025 •
edited
Loading

The patch looks absolutely fine, but I don't seem to be able to reproduce the mentioned bug, and there's no mention of how to do it...

If you find a sensor that does not have alert_type specified, then you can reproduce this by going to http://localhost/ipdevinfo/sensor/1485 and then click on "Add to dashboard"

My test data contains 11998 sensors without an alert_type. I've selected several ones by random, visited their URLs and clicked the Add to dashboard button. The button turns green, and the network tab of web developer tools indicate a succesful POST request to /navlets/add-user-navlet/sensor/?sensor_id=<sensor_id_here>.

Still not sure what I'm looking for... (this is on the latest master branch)

@johannaengland
Copy link
Copy Markdown
Contributor Author

johannaengland commented Jul 30, 2025

The patch looks absolutely fine, but I don't seem to be able to reproduce the mentioned bug, and there's no mention of how to do it...

If you find a sensor that does not have alert_type specified, then you can reproduce this by going to http://localhost/ipdevinfo/sensor/1485 and then click on "Add to dashboard"

My test data contains 11998 sensors without an alert_type. I've selected several ones by random, visited their URLs and clicked the Add to dashboard button. The button turns green, and the network tab of web developer tools indicate a succesful POST request to /navlets/add-user-navlet/sensor/?sensor_id=<sensor_id_here>.

Still not sure what I'm looking for... (this is on the latest master branch)

This only applies to boolean sensors, for all others it works as expected

nav/python/nav/web/navlets/__init__.py

Line 431 in ac4924c

if sensor.unit_of_measurement == sensor.UNIT_TRUTHVALUE:

@johannaengland
Fix adding sensor widget to main page
c517896 
Some sensors have no alert_type specified, which would make this fail for boolean sensors
@johannaengland johannaengland force-pushed the bugfix/add-sensor-widget branch from ac4924c to c517896 Compare July 30, 2025 13:15
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Jul 30, 2025

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

@johannaengland johannaengland merged commit c17008a into Uninett:master Jul 30, 2025
13 checks passed
@johannaengland johannaengland deleted the bugfix/add-sensor-widget branch July 30, 2025 13:23
@lunkwill42
Copy link
Copy Markdown
Member

lunkwill42 commented Jul 31, 2025

This only applies to boolean sensors, for all others it works as expected

Indeed, I suspected this in the end. I'll try to make a mental note of amending the changelog to that effect :)

@johannaengland
Copy link
Copy Markdown
Contributor Author

johannaengland commented Jul 31, 2025

This only applies to boolean sensors, for all others it works as expected

Indeed, I suspected this in the end. I'll try to make a mental note of amending the changelog to that effect :)

I already added that caveat to the news fragment 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jorund1 jorund1 Awaiting requested review from jorund1

@lunkwill42 lunkwill42 Awaiting requested review from lunkwill42

Assignees

@johannaengland johannaengland

Labels

widget

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@johannaengland @lunkwill42 @jorund1