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

Updating OCKContact, OCKPatient, etc. creates duplicates in CareStore #357

Closed
cbaker6 opened this issue Feb 10, 2020 · 13 comments · Fixed by #368 or #373
Closed

Updating OCKContact, OCKPatient, etc. creates duplicates in CareStore #357

cbaker6 opened this issue Feb 10, 2020 · 13 comments · Fixed by #368 or #373

Comments

@cbaker6
Copy link
Contributor

cbaker6 commented Feb 10, 2020

After adding a new contact or patient to the CareStore, calling an update method such as "store.updateContact" creates an additional contact in the CareStore. If I'm understanding the documentation correctly, this should simply update the current contact and create a new version (allowing you go back to the older version if needed).

The problem is that querying contacts then returns each contact as a new one with the same id.

Is if you make 10 updates to contact, such as updates to address, etc. you will end up with 11 different contacts.

If tested this on OCKPatient and the same thing is happening.

The version of CareKit is the latest commit: 78a6ce5

@cbaker6
Copy link
Contributor Author

cbaker6 commented Feb 10, 2020

My assumption for a workaround is to use the contact with the latest version or probably the one with the newest "effectiveDate"

@erik-apple
Copy link
Collaborator

Each time you update one of the versioned entities (patent, care plan, contact, and task), a new entry is created in the table, and the old and new versions are linked together. The intended behavior is that when you query contacts in an interval, the store should return only the newest version existing in the given interval.

You've discovered a bug in which not passing any date results in all versions being returned.

We'll prioritize fixing this bug first as it will affect more users and doesn't have as simple of a work around as the other bug you found for us. You're assumed workaround is probably the best option available until we can issue a patch.

We appreciate your help ironing out these bugs!

@erik-apple
Copy link
Collaborator

erik-apple commented Feb 10, 2020

We did some work to fix this, but upon further reflection, are now leaning toward saying that the present behavior is correct, and that if you only want the latest version, you need to specify a date on the query.

Consider a case in which a patient is taking a medication, and the dose for that medication is periodically updated. Now let's imagine that we'd like to create a view controller which shows the prescription history for that medication. In order to do that, we need to be able to query all versions of the associated OCKTask. If queries only return the latest version of the task, then there is no way to gather all past versions without knowing a-priori when updates were made.

The present behavior allows you to ask for all versions by not passing a date, or to get the valid version in a date window.

// This query will return all versions, past, present, and future, of the task "doxylamine"
var query = OCKTaskQuery()
query.ids = ["doxylamine"]

// This query will return the newest version of the task as of today
var query = OCKTaskQuery(for: Date())
query.ids = ["doxylamine"]

// This query will return the current version of the task as it would have appeared a week ago
var query = OCKTaskQuery(for: aWeekAgo)
query.ids = ["doxylamine"]

So in your case, you'll probably get the results you desire if you create the query by passing in Date().

We'll update the documentation to explain this in more detail if it resolves your problem. We'll also consider making API changes in future versions to make this behavior more self-documenting.

@cbaker6
Copy link
Contributor Author

cbaker6 commented Feb 10, 2020

I agree with your example in terms of tasks. In that case, it is beneficial to see all versions/changes to the task. Each of these tasks will have different outcomes and outcomevalues attached to them. Should the changes to outcome values behave in a similar way?

What about when attempting to find all contacts related to a CarePlan? In this scenario, the time the contact was created/changed may not be important, I just need the latest contacts attached to the CarePlan. I also may not know the appropriate date range for this case.

@erik-apple
Copy link
Collaborator

erik-apple commented Feb 10, 2020

Should the changes to outcome values behave in a similar way?

Are you asking if outcomes should be a versioned entity?

What about when attempting to find all contacts related to a CarePlan? ...I also may not know the appropriate date range for this case. I just need the latest contacts attached to the CarePlan.

Many CareKit users pre-program new contacts and tasks to appear in the future, so there can be cases where there are versions of contacts that aren't intended to show up for a few more weeks. Getting the newest version (which would include future versions) isn't usually what you actually want. Typically what people are actually after is "the newest version as of the present".

You may not have future entities pre-loaded in your app, but OCKContactQuery(for:) is probably what you should use nonetheless.

// Get today's version of the "heartDisease" care plan
store.fetchAnyCarePlan(id: "heartDisease", callbackQueue: .main) { result in 
    
    // This is the latest version as of today
    let carePlan = try! result.get()

    // Create a query that finds the contacts on today's version of the care plan
    var query = OCKContactQuery(for: Date())
    query.carePlanIDs = [carePlan.localDatabaseID!]

    self.store.fetchAnyContacts(query: query, callbackQueue: .main) { result in 

        // These are the latest versions of the contacts for the latest version of the care plan
        let contacts = try! result.get()
    }
}

@cbaker6
Copy link
Contributor Author

cbaker6 commented Feb 10, 2020

Based on the use-cases you mentioned, I now understand why the queries are returning the way they are.

It seems your solution:

// Create a query that finds the contacts on today's version of the care plan
    var query = OCKContactQuery(for: Date())

Provides what I was looking for even though I created a Patient on 2/9/20 that has multiple versions that were created while debugging on 2/9, when I use Date() on 2/10/20, it gives me the newest version (2/9/20), even though that version isn't today. For some reason, I thought Date() would only provide me with the entities for today.

@erik-apple
Copy link
Collaborator

I find it helpful to visualize it on a timeline. The versions you created on 2/9 are exist extending into the future. When you take a future snip of the timeline (by querying) you will get the last version you created on 2/9.

timeline

@cbaker6
Copy link
Contributor Author

cbaker6 commented Feb 11, 2020

@erik-apple thanks the for the illustration and details! So whenever we want the latest versions, query with Date() and If we need to look at all versions, query without a date. If we need to get whatever versions were available on a specific date, query for the respective date/range...

I'm willing to close this issue if needed. Maybe in the future there can be some version of the visualization you made in the documentation?

@erik-apple
Copy link
Collaborator

Yep, exactly!

We’ll update the readme with more details. You can leave this issue open for now and we’ll close it after we’ve tweaked the docs!

@cbaker6
Copy link
Contributor Author

cbaker6 commented Feb 21, 2020

@erik-apple I think there may be a breaking change that was introduced by the 2/11/20 commit that causes all versions to show up when querying by using Date() to get the latest OCKPatient the way we discussed here.

Basically the same piece of code shows multiple versions in the new version above, but this doesn't occur with the previous 1/30/20 commit. Note that in order to see the error, you need to make an update to OCKPatient using a committed version of CareKit from 2/11 or higher.

I have updated my previous sample project to show the issue. One the main branch of the sample code, every time you hit the "Care Team" button, it will add an "s" onto the patient's last name and update the patient. The working branch of the code has the 1/30 commit which only shows 1 patient with the appended "s" no matter how many updates occur. The "working" branch has the 1/30 commit.

My assumption is something going on with the versioning where it's duplicating as when I upgrade from the older commit to the newer, on first run, I don't see multiple Patients, but whenever I make an update on Patient using the new version, I see multiple versions.

@erik-apple
Copy link
Collaborator

Thanks for doing the footwork to isolate this!

I'll start trying to reproduce the problem now, which should hopefully be pretty easy since you've provided a sample repo and determined the guilty commit.

@erik-apple
Copy link
Collaborator

Fortunately this was an easy fix! It was indeed a regression produced by the commit you identified. And it was much easier to locate it thanks to your sleuth work! Cheers!

@cbaker6
Copy link
Contributor Author

cbaker6 commented Feb 22, 2020

Happy to help!

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

Successfully merging a pull request may close this issue.

2 participants