Skip to content
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

Error when end date for user is null #64

Open
mpgxvii opened this issue Mar 4, 2024 · 6 comments
Open

Error when end date for user is null #64

mpgxvii opened this issue Mar 4, 2024 · 6 comments
Assignees

Comments

@mpgxvii
Copy link
Member

mpgxvii commented Mar 4, 2024

  • In the latest versions of rest-sources-auth, we now allow null end date (which may be applicable for some studies where data collection is ongoing and there is no specific end date)
  • However this causes an error in pulling Garmin data through the PushEndpoint, and this affects the processing and pulling of the data. Some error logs:
Failed to parse JSON: com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException: Instantiation of [simple type, class org.radarbase.push.integration.garmin.user.GarminUser] value failed for JSON property endDate due to missing (therefore NULL) value for creator parameter endDate which is a non-nullable type

@yatharthranjan Can we allow end date to be nullable or set it to a default value? What do you think?

@mpgxvii mpgxvii self-assigned this Mar 4, 2024
@afolarin
Copy link
Member

afolarin commented Mar 4, 2024 via email

@mpgxvii
Copy link
Member Author

mpgxvii commented Mar 4, 2024

I seem to recall we had this discussion before, and I'm not sure if there's a straightforward workaround. For now, perhaps the way to handle this is to use a very far in the advanced future end date?

@afolarin Yes, because of this error, it affected the whole application and wasn't able to process incoming data. So I just set the end date for that user and it got resolved. Yes either maybe a default max end date if it is null?

@afolarin
Copy link
Member

afolarin commented Mar 4, 2024 via email

@yatharthranjan
Copy link
Member

I think there are a few things to consider -

  1. If this is a use-case, where we want unbounded end dates, then we should add support for it in the push endpoint
  2. if we think this is not a valid use-case for this app, we can allow null dates in the data class but filter out the users in the user repository when they are being queried.

IMO number 1 but it might take a while to add this, we can add the fix mentioned number 2 in the short term.

@afolarin
Copy link
Member

afolarin commented Mar 4, 2024

For 1. What do we think the consequences of unbounded end dates are here (other than perhaps some info governance, though the main controls for dictating if data is collected should ideally be the Management Portal as this overides all IIRC).
For 2. I guess what is the overhead here and how much operational complexity is added by the filtering.

@yatharthranjan
Copy link
Member

For 1. What do we think the consequences of unbounded end dates are here (other than perhaps some info governance, though the main controls for dictating if data is collected should ideally be the Management Portal as this overides all IIRC).

Yes would be ideal if it was all centralised in the MP, but currently there are not controls in MP to specify the end date of a subject
For the consequences, i am not sure, apart from added code complexity (one more case to handle in all downstream apps that use rest-sources-auth). This functionality is already present for researchers tbh (like you said someone can just specify very high end date in the authorizer and it would be same). I think this just adds another human error component which is not really needed. Maybe something to discuss with the hyve too.

For 2. I guess what is the overhead here and how much operational complexity is added by the filtering.

Not much complexity i guess apart from us having to followup with people who have left unbounded dates when we believe the study should have ended (but it is similar to someone specifying dates that are too far in the future). This should be somehow limited if people are following instructions provided by us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants