-
-
Notifications
You must be signed in to change notification settings - Fork 175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create a setting for user to indicate humanized or hours on dashboard/device detail #634
Conversation
{ | ||
SettingKeyName: "powered_on_hours_unit", | ||
SettingKeyDescription: "Presentation format for device powered on time ('humanize' | 'device_hours')", | ||
SettingDataType: "string", | ||
SettingValueString: "humanize", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has to be added as a new migration, otherwise it will be skipped for users with an existing database..
sure, I can get behind this. Added comment about the db settings change - it needs to be in it's own migration |
Thanks @AnalogJ - I've never developed in Go, any specific naming conventions or versioning? Does it need to be removed from the current script? |
Basically just copy paste a migration entry like this: https://github.com/bauzer714/scrutiny/blob/addDeviceHoursSetting/webapp/backend/pkg/database/scrutiny_repository_migrations.go#L379C3-L393C5 The ID should start with "m" and be followed by a timestamp in YYYYMMDDHHMMSS format
Thanks @bauzer714 ! |
@AnalogJ - thanks for the guidance. Pushed the latest changes. Let me know if this needs further changes. |
import { DeviceHoursPipe } from './device-hours.pipe'; | ||
|
||
|
||
describe('DeviceHoursPipe', () => { | ||
it('create an instance', () => { | ||
const pipe = new DeviceHoursPipe(); | ||
expect(pipe).toBeTruthy(); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one last nit. Can you add some tests to show how this pipe behaves when called with:
- 'humanize'
- 'device_hours'
- null
- a invalid string
thanks! @bauzer714 this is great work
@AnalogJ - updated. I added tests for "configuration" of each of the 4 requested. Additionally I added null input values (since null is a number in TS) for the configuration "device_hours" and "humanize". What's your thoughts on "null hours" vs "0 hours" as the expected output of the "null" input tests? A few options I thought of:
|
if no data is present, maybe "No Data" or "Unknown" |
@AnalogJ - Updated. Let me know any additional changes. |
LGTM. Thanks for the PR @bauzer714 ! 🥇 |
Default is "humanize" creates a non-breaking change.
Setting:
Dashboard:
Detail:
Detail tooltip:
Implements original request of #628.