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

Syncing with KnowledgeVector additions #437

Closed
cbaker6 opened this issue May 22, 2020 · 3 comments · Fixed by #457
Closed

Syncing with KnowledgeVector additions #437

cbaker6 opened this issue May 22, 2020 · 3 comments · Fixed by #457

Comments

@cbaker6
Copy link
Contributor

cbaker6 commented May 22, 2020

I made additions to sync OCKPatient, OCKCarePlan, and OCKContact in CareKit on this branch. It's mostly a port of the functionality from syncing Tasks and Outcomes with the exception of OCKContact as a required an extra step to delete during a mergeRevision. For OCKPatient, OCKCarePlan, and OCKContact, I currently do nothing in confirmUpdateWillNotCauseDataLoss, but I can image there needs to be something there when a Contact or CarePlan is related to a Patient who has been deleted...

I didn't create a PR on this since the KnowledgeVector part is still in beta, but I did create some additional test cases in TestStore+BuildRevisions.swift, TestStore+ConsumeRevisions.swift, and TestStore+Sync.swift to make sure everything was working, so they may be useful.

If you think a part of it fits a PR, let me know and I can set it up.

@cbaker6
Copy link
Contributor Author

cbaker6 commented May 23, 2020

I created a PR on my master which mirrors careKit's master so the additions/changes can easily be seen. I'm not proposing to merge the podspec files, I use them for ParseCareKit. Also, the branch removes the patient restriction I mentioned in #438

https://github.com/cbaker6/CareKit/pull/1/files

@erik-apple
Copy link
Collaborator

This is a great start Corey, thanks!

The confirmUpdatesWillNotCauseDataLoss method exists for Tasks because it's possible that updating to a new version of a task could cause future outcomes (if they exist already) to be lost.

I'm not sure that an analogous method will be required for patients, care plans, or contacts, as those entities aren't as sensitive to a notion of time or schedules. We've still got a bit of thinking to do on that front.

It seems like the code for syncing patients, care plans, contacts, and tasks will all be pretty similar -- as you've demonstrated here. I'd like to explore what our options are for sending all this code down a single code path instead of having several similar code paths. That way we won't have to propagate changes made to one through all of the others manually.

It might turn out that it's too difficult or obfuscates the intent too much, in which case we could land right back here, but let's see explore the possibilities and see what we can do.

@cbaker6
Copy link
Contributor Author

cbaker6 commented Jun 1, 2020

I definitely agree, I was trying to see if I could sync all data leveraging KnowledgeVectors, but definitely replicated code with tweaks. I'm sure there are some ways to simplify

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