RADAR-base / RADAR-Rest-Source-Auth

A simple application to support authorization of wearable devices using OAuth using a webservice with REST Endpoints.
https://radar-base.org/
Apache License 2.0
1 stars 0 forks source link

Initial implementation for RESET of users #40

Closed yatharthranjan closed 4 years ago

yatharthranjan commented 4 years ago

Working demo is here Closes #33

afolarin commented 4 years ago

Looks fine to me.

If applicable to individual streams/topics -- this could be handy, but not essential (at least I don't think we're hitting the API limits at the moment). Certainly, we'd like to re-pull all data for individual participants with detected gaps in their data, or those exiting the study, as a way of trying to backfill any holes in the user data where there was a sync delay on the Fitbit-side (this is the working hypothesis for gaps).

On a related matter -- is there any reasonable sense to adding a backoff if we detect a given individual's data is gappy in the initial collection phase?

yatharthranjan commented 4 years ago

If applicable to individual streams/topics -- this could be handy, but not essential (at least I don't think we're hitting the API limits at the moment). Certainly, we'd like to re-pull all data for individual participants with detected gaps in their data, or those exiting the study, as a way of trying to backfill any holes in the user data where there was a sync delay on the Fitbit-side (this is the working hypothesis for gaps).

I looked into doing this per topic but it is not as trivial as it would seem. Firstly, the source-authoriser has no data about the topics and data streams so adding them here would take some thinking and further implementation (might not be worth it). Secondly, the connector also will have to be modified to filter this based on the topic and again would require additional effort to design and implement. I guess for now this should be sufficient. We can discuss in the next dev call for potential solutions

On a related matter -- is there any reasonable sense to adding a backoff if we detect a given individual's data is gappy in the initial collection phase?

I think there already is a back off but it is hard to set this interval since it will vary with every user and how they use the device. Also will vary per data stream. But again we should discuss this in the dev call next week. Perhaps @blootsvoets can also comment his thoughts on this.

nivemaham commented 4 years ago

Can't we append some incremented value to the version? so that we have some idea how often this has been reset? Ideally it would be nice to have auditing implemented for this, so we know whats happening. @yatharthranjan I also think, it is good to add a confirmation dialog to let user know and confirm that they want to reset. In another project, we implemented a pop-up which would confirm the operation and you can also optionally choose the start and end date for the reset. in this case we have more control over what data is being pulled and we can maybe avoid redundant pulling. This would address part of the requirements mentioned by @afolarin

yatharthranjan commented 4 years ago

Hi nivethika,

Can't we append some incremented value to the version? so that we have some idea how often this has been reset? Ideally it would be nice to have auditing implemented for this, so we know whats happening.

Do you mean like {number}-{Date} and the number is also incremented every time we reset? Maybe we can include another field like timesReset for brevity instead of adding it to the version? By auditing, do you mean auto-generated createDate and updateDate properties?

@yatharthranjan I also think, it is good to add a confirmation dialog to let user know and confirm that they want to reset.

Ok i will add a pop to both RESET and DELETE.

In another project, we implemented a pop-up which would confirm the operation and you can also optionally choose the start and end date for the reset. in this case we have more control over what data is being pulled and we can maybe avoid redundant pulling. This would address part of the requirements mentioned by @afolarin

ok sounds good. i will add editing the start and end date during reset popup. Amos' suggestion was more for specific topics which would be much harder to add i think.

afolarin commented 4 years ago

should we have a cancel on the Edit screen seems to only have a Submit

yatharthranjan commented 4 years ago

Hi @nivemaham and @afolarin ,

I have made most of the requested changes including-

  1. adding a timesReset property to signify the number of times a user has been reset
  2. Pop up confirmation for the RESET and DELETE Actions
  3. The RESET popup also contains functionality to update the start and end dates for data collection.
  4. Add a CANCEL button to the EDIT users page.

The only thing I have not yet added is the auditing as i was not exactly sure of the specifics. But it is another feature and can be added in a separate PR.

The live demo for the above changes is available here. Please take a look and provide feedback. Thanks.