"Active" state means that all data collected by the data source will be
sent to a server.
This change aims to give a user more control of what data they want to
send. Without this functionality the user might decide to reduce
telemetry level because of one small part of it. This means that we can
collect less statistic without a really good reason.
Details
Diff Detail
- Repository
- R849 User Feedback Collection Framework
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
For most of users, I think so.
And there are a few ways to implement this:
- Hard way (decorator-like approach): de-virtualize store/load/reset methods. Add new virtual methods with with the same names, but suffix "Impl". These methods will be used by children. De-virtualized methods will be implemented like this:
load(...) { // Custom logic loadImpl(...); }
- Easy way: change ProviderPrivate::store/load to also write/read additional settings like "active" flag.
- Leave it as is. If user need this functionality, he can easily implement it in code around.
Personally I would prefer the first option, but on the other hand, the second one involves less changes. What do you think is the best?
I had option (2) in mind initially, assuming that this doesn't hurt even for scenarios where this functionality isn't needed. It would also be consistent with how the current settings are persisted, avoiding the surprise that disabled elements get automatically activated again if you don't implement persistence manually in your application. (1) would achieve the same thing I think, so I'd be ok with that too. I just don't like option (3) :)
Ok, I'm going to implement option (1) then, because it looks like more universal solution and will produce the same results for persisting settings via Provider and DataSource itself.
"Active" state is stored in the settings now.
Also added unit tests covers new functionality.
Thanks! Almost there, but I think this is too aggressive in reset(). reset() is meant to reset internal counters etc between submissions, not to reset user settings, so that should not touch the active flag.
"Active" state is not reset in between data submissions anymore.
Also fixed unit test for this piece of functionality.