carekit-apple / CareKit

CareKit is an open source software framework for creating apps that help people better understand and manage their health.
https://www.researchandcare.org
Other
2.42k stars 443 forks source link

OCKSimpleTaskViewController and taskView.completionButton target work differently with last merge #314

Closed olger closed 5 years ago

olger commented 5 years ago

Thanks for the updates, I am changing my app to make use of these and test #309 and #310

Leave me one question:

Somewhere I found a trick to get a SimpleTaskView to do something additional when it is marked as done. (It triggers a ResearchKit view with a question)

                      if let dayScoreTask = tasks.first(where: { $0.identifier == "dayscore" }) {
                            let dayScoreCard = OCKSimpleTaskViewController(
                                storeManager: self.storeManager,
                                task: dayScoreTask,
                                eventQuery: OCKEventQuery(for: date))
                        dayScoreCard.taskView.completionButton.removeTarget(nil, action: nil, for: .allEvents)
                            dayScoreCard.taskView.completionButton.addTarget(self, action: #selector(self.presentDayScoreSurvey), for: .touchUpInside)

This doesn't work anymore (and possibly rightly so as it uses internals) What is the advised way to get this functionality back to work ? OCKTaskDisplayable seems like a candidate but if so, do you have sample use somewhere ?

Originally posted by @olger in https://github.com/carekit-apple/CareKit/pull/313#issuecomment-530714334

gavirawson-apple commented 5 years ago

Great question @olger! The OCKSimpleTaskView has a tap gesture on the entire view which will toggle the button. If you would like to override the default view controller logic, you can create a custom view controller class like so:

class CustomSimpleTaskViewController<Store: OCKStoreProtocol>: OCKSimpleTaskViewController<Store> {

    // Override this method, which will be called whenever the view is tapped. Note that the superclass's version of this method will
    // save a new outcome to to the store. Overriding without calling super will ignore that funcitonality.
    override func eventView(_ eventView: UIView & OCKEventDisplayable, didCompleteEvent isComplete: Bool, sender: Any?) {

        // Here you can deselect the completion button, which is automatically toggled when the view is tapped.
        (eventView as? OCKSimpleTaskView)?.completionButton.isSelected = false

        // Perform your custom logic here. As an example, we can display an alert.
        let alert = UIAlertController(title: "Alert", message: nil, preferredStyle: .alert)
        let cancelAction = UIAlertAction(title: "Close", style: .default, handler: nil)
        alert.addAction(cancelAction)
        present(alert, animated: true, completion: nil)
    }
}

Then, rather than using the default OCKSimpleTaskViewController, you can instantiate your custom CustomSimpleTaskViewController. Does that solve the issue?

olger commented 5 years ago

Thanks @gavirawson-apple for your explanation. This is quite a nice way to build custom TaskViews. I am working on a (specific) task view that makes use of ResearchKit.

What is the advised way to store an OCKOutcome based on the input of a custom view (controller) ? My first 'try-out's' do not seem to be the right approach. (Could imagine some kind of delegate that gives back a list of OCKOutcome and the remainder of handling (storage, updating views) is arranged for you.

gavirawson-apple commented 5 years ago

There are a couple of ways you can go about this depending on the specific details of your use case. What is the intended UX flow here for the user? And what action triggers the completion of the event that will ultimately lead to storing an OCKOutcome?

olger commented 5 years ago

I have a ORKTaskViewControllerDelegate that gives back the result of an ORK ordered taks in this situation with a ORKAnswerFormat.continuousScale Step.

The user 'marks' the formerly SimpleTaskView and now the custom task view as 'done', this triggers the ResearchKit view showing the ORKTaskView(Controller). This triggers the delegate when the questionnaire of a single step is done.

Hope this gives enough idea of the user flow and intended use. The simpletask gives the option to have a single marked item that needs to be done and is done after the user entered the answer of the questionnaire.

This worked before the merge because I changed the way the 'button' worked (found that somewhere in the tickets here, I believe) But basically I was using an internal component and changed its' behaviour, I'd prefer to use a way that is more as expected. After the change, the the 'done' marker of my task doesn't work (yet) the completeness in the dates is not passing the 50%, al in all enough to learn on how the framework works and 'should' be used ;)

BTW: really appreciate your patience and support.

gavirawson-apple commented 5 years ago

No worries at all! We're happy to help with these types of questions.

That flow totally makes sense and is definitely achievable. One route is to use the ResearchKit delegate methods to receive a notification when the ORKTaskViewController finishes. When it is finished, you can create an OCKOutcome in the OCKStore. Details of this process are outlined here.

olger commented 5 years ago

The link doesn't seem right so I can't see the outline.

What I do is implementing the ORKTaskViewControllerDelegate

     func taskViewController(_ taskViewController: ORKTaskViewController, didFinishWith reason: ORKTaskViewControllerFinishReason, error: Error?) {
        switch reason {
            case .completed:
                taskViewController.dismiss(animated: true, completion: nil)
                guard let dayScore = taskViewController.result.results?.filter({ $0.identifier == "dayscorequestion" }).first as? ORKStepResult else { return }
                let dayScoreResult = dayScore.results![0] as! ORKScaleQuestionResult
                let dayScoreResultScore = dayScoreResult.answer as! Double
                storeManager.store.fetchTask(withIdentifier: "dayscore") { result in
                    let careTask = try! result.get()
                    let dayScoreOutcome = OCKOutcome(taskID: careTask.localDatabaseID, taskOccurenceIndex: 0, values: [OCKOutcomeValue(dayScoreResultScore)])
                    let customOutcome = Store.Outcome(dayScoreOutcome)
                    self.storeManager.store.addOutcomes([customOutcome], queue: .main)
                        { result in
                            self.log.debugMessage("Got store result \(result)")
                    }
                }

            case .discarded:

This isn't updating the store in a way that the task is marked as done and the outcome value is displayed in a OCKCartesianChartViewController, so must be something that is wrong in my code.

olger commented 5 years ago

@gavirawson-apple, storage was my issue.

the scheduleEvent.occurence needed a piece of code to decide what the value should be. Probably a bit of background read on the the way store now works will help me. With the code below, the full flow works, stores values and shows them in the ChartView.

              storeManager.store.fetchTask(withIdentifier: "dayscore") { result in
                    let task = try! result.get()
                    let thisMorning = Calendar.current.startOfDay(for: self.selectedDate)
                    let tonight = Calendar.current.date(byAdding: DateComponents(day: 1, second: -1), to: thisMorning)!
                    let scheduleEvent = task.convert().schedule.events(from: thisMorning, to: tonight).first!
                    let dayScoreOutcome = OCKOutcome(taskID: task.localDatabaseID, taskOccurenceIndex: scheduleEvent.occurence, values: [OCKOutcomeValue(dayScoreResultScore)])
                    let customOutcome = Store.Outcome(dayScoreOutcome)
                    self.storeManager.store.addOutcomes([customOutcome], queue: .main)
                        { result in
                            self.log.debugMessage("Got store result \(result)")
                    }
olger commented 5 years ago

Just to summarise the way to integrate in this user flow: A custom extended OCKSimpleTaskViewController with custom views interacting with the user will store (in the right way) the OCKOutcomes (and values) and the other parts of the App will work as expected.

erik-apple commented 5 years ago

@olger Glad you figured it out! The way the store works didn't change in the last update, so I'm not quite sure why it was working for you before. The issue with your code, as you correctly diagnosed, is that you had the taskOccurenceIndex set to 0 on the outcome.

Let's say your dayscorequestion task is defined to occur every day, starting on Sep 1, 2019.

The first occurrence of the task will be on Sunday, Sep 1. (taskOccurenceIndex = 0) The second occurrence will be on Monday, Sep 2. (taskOccurenceIndex = 1) The third occurrence will be on Tuesday, Sep 3. (taskOccurenceIndex = 2)

When you save an outcome, you specify which occurrence of that task to associate it with by setting the taskOccurenceIndex. In your code, you were setting it to 0, which means you were just continuously overwriting the first event every time. It's fixed now because you are retrieving the correct index from the task's schedule.

There are a couple way to retrieve the correct occurrence index. The first is the way you're using now, in which you query the task and pass a date range to the task's schedule. This method is useful when you only have access to a date, but not an event object itself.

If you are subclassing OCKEventViewController or one its subclasses, then you have access to a variable viewModel: Store.Event?. In that case, it's much easier because you already have all the info required. I recommend this approach for your use case.

func saveNewOutcome(value: OCKOutcomeValue) {
  let event = viewModel! // In the context of an OCKEventViewController, the view model is a Store.Event
  let occurenceIndex = event.scheduleEvent.occurence // We can get the index without querying since we already have the event
  let taskID = event.task.localDatabaseID

  // When we create the event, we have to associate it with both a specific task, and a specific occurrence of that task
  let outcome = OCKOutcome(taskID: taskID, taskOccurenceIndex: occurenceIndex, values: [value])

  // Persist the outcome
  manager.store.addOutcome(Store.Outcome(outcome)) { result in 
     // check for errors / handle result
  }
}

I hope this makes it more clear! We're working on adding more documentation right now. Thanks for your patience during the beta!

olger commented 5 years ago

Thanks @erik-apple , your sample on how the Outcomes are stored and the role of the occurrence index makes it all really clear. The '0' in the sample was an experiment to check my assumption that the Outcomes were stored based on the Date+occurenceindex as primary key. (and that is not the way it is)

The event being a specific type in the right context, is quite nice and certainly saves me from querying the store. I've changed my code accordingly.

Thanks again !