Add support for disabling repeat notifications if the values haven't changed

[kaysond]

fixes #504
fixes #364
fixes #514
related #553

Still needs testing and some refactoring

[kaysond]

@AnalogJ - would you mind reviewing before I update the tests?

[AnalogJ]

Fantastic PR! 🥳 thanks for making such a meaningful contribution.

I like the idea, and the implementation. Anything that touches the notification system needs alot of tests (which you mentioned you're working on).
I also called out that I'd like to see some more comments in the filtering logic as well.

Thanks again!

[kaysond]

Addressed the comments and tested the image (it works). Will add/fix tests

[kaysond]

@AnalogJ - I started looking into the tests, but I got stuck pretty quickly because I am a total golang noob.

The main issue is that I need to pass a deviceRepo to ShouldNotify now, which depends on a database that doesn't exist. I see some mocking libraries in other tests, so I'm guessing this might be the right approach? Would you mind pointing me in the direction of the right libraries/documentation/strategy to use?

Thanks!

[kaysond]

Bump. Pinging @AnalogJ for some help with tests. Thanks!

[AnalogJ]

I was going to say that it's super easy, but it seems like I forgot to generate a mock for the database. Let me generate one and push to your branch

[AnalogJ]

So I added a new file: webapp/backend/pkg/database/scrutiny_repository_device_smart_attributes.go

You can now use that to mock the database in your tests (similar to what I do with the Config mock):

	//setup
	mockCtrl := gomock.NewController(t)
	defer mockCtrl.Finish()
	fakeDatabase := mock_config.NewMockDeviceRepo(mockCtrl)

	// mock/fake out data and calls to the `fakeDatabase`, providing expected arguments and expected return parameters. 
    // you can mock any function in the interface: https://github.com/kaysond/scrutiny/blob/master/webapp/backend/pkg/database/interface.go
	fakeDatabase.EXPECT().GetDevices(context.Background()).Return([]models.Device{}, nil).AnyTimes()


    // if your inputs vary, you can use `gomock.Any()` as the argument
	fakeDatabase.EXPECT().HealthCheck(gomock.Any()).Return(fmt.Errorf("my error").AnyTimes()


	// once you finish setting up the mock, you can then pass it into your function under test:
	ShouldNotify(... , fakeDatabase) 
	
	// and internally when it calls `deviceRepo.GetSmartAttributeHistory(...)`, you can fake the response data. 

[AnalogJ]

you should be good to write tests now @kaysond

[kaysond]

you should be good to write tests now @kaysond

Thank you! I'll take a crack at it tonight if I can.

[kaysond]

@AnalogJ fixed the existing tests and added a few more for the case where repeat notifications are turned off. Looking at the updated shouldNotify code, this gives pretty good coverage of all the branching and looping.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 31.48148% with 74 lines in your changes missing coverage. Please review.

Project coverage is 30.34%. Comparing base (0febe3f) to head (09f4b34).
Report is 125 commits behind head on master.

Files with missing lines	Patch %	Lines
...ase/scrutiny_repository_device_smart_attributes.go	0.00%	51 Missing ⚠️
...end/pkg/database/scrutiny_repository_migrations.go	0.00%	12 Missing ⚠️
webapp/backend/pkg/notify/notify.go	87.17%	3 Missing and 2 partials ⚠️
...end/pkg/models/measurements/smart_ata_attribute.go	0.00%	2 Missing ⚠️
...nd/pkg/models/measurements/smart_nvme_attribute.go	0.00%	2 Missing ⚠️
...d/pkg/models/measurements/smart_scsci_attribute.go	0.00%	2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #547      +/-   ##
==========================================
- Coverage   32.54%   30.34%   -2.21%     
==========================================
  Files          54       28      -26     
  Lines        3045     2755     -290     
  Branches       66        0      -66     
==========================================
- Hits          991      836     -155     
+ Misses       2018     1889     -129     
+ Partials       36       30       -6     

Flag	Coverage Δ
unittests	30.34% <31.48%> (-2.21%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kaysond]

Bump. Pinging @AnalogJ for review

[AnalogJ]

hey @kaysond apologies for the delay.
As far as I can tell everything looks good! thanks so much for pushing this through to the finish-line.
I'm happy to merge it if you're ready.

[kaysond]

hey @kaysond apologies for the delay. As far as I can tell everything looks good! thanks so much for pushing this through to the finish-line. I'm happy to merge it if you're ready.

@AnalogJ please go ahead. And if you don't mind doing a release too then I can switch off my fork :)

[AnalogJ]

will do!