Validation API for SDC #548

joiskash · 2021-06-14T11:23:44Z

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Fixes #374

Description
Expose external api for developers to validate the questionnaire response using the SDC library

Type
Feature

Checklist

I have read and acknowledged the Code of conduct
I have read How to Contribute
I have read the Developer's guide
I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA
I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach
I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project
I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally
I have built and run the reference app(s) to verify my change fixes the issue and/or does not break the reference app(s)

…ting

datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt

datacapture/src/main/java/com/google/android/fhir/datacapture/validation/ConstraintValidator.kt

...in/java/com/google/android/fhir/datacapture/validation/QuestionnaireResponseItemValidator.kt

…validation/ConstraintValidator.kt Co-authored-by: Jing Tang <jing.t86@gmail.com>

Tarun-Bhardwaj · 2021-06-18T05:33:40Z

Hi @joiskash, it seems changes are pending for this PR. Is there an ETA by when these can be ready for review?

joiskash · 2021-06-18T07:17:42Z

Hi @joiskash, it seems changes are pending for this PR. Is there an ETA by when these can be ready for review?

I have actually requested a review after the last round of review. Not sure if its not showing up properly

jingtang10 · 2021-06-18T13:51:08Z

Hi @joiskash, it seems changes are pending for this PR. Is there an ETA by when these can be ready for review?

I have actually requested a review after the last round of review. Not sure if its not showing up properly

Thanks @joiskash

Not sure if you need to dismiss or approve the previous review etc. Odd.

joiskash · 2021-06-18T13:52:41Z

Hi @joiskash, it seems changes are pending for this PR. Is there an ETA by when these can be ready for review?

I have actually requested a review after the last round of review. Not sure if its not showing up properly

Thanks @joiskash

Not sure if you need to dismiss or approve the previous review etc. Odd.

Yea.. i maybe doing something wrong. Ill pay more attention next time. Sorry for the inconvenience

jingtang10 · 2021-06-18T13:54:03Z

...pture/src/main/java/com/google/android/fhir/datacapture/validation/QuestionnaireValidator.kt

+import org.hl7.fhir.r4.model.Questionnaire
+import org.hl7.fhir.r4.model.QuestionnaireResponse
+
+object QuestionnaireValidator {


Questionnaire Response Validator maybe? Since it's validating the response.

jingtang10 · 2021-06-21T10:18:58Z

datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt

@@ -66,6 +66,9 @@ class QuestionnaireFragment : Fragment() {
  // Returns the current questionnaire response
  fun getQuestionnaireResponse() = viewModel.getQuestionnaireResponse()

+  // Returns the current questionnaire
+  fun getQuestionnaire() = viewModel.getQuestionnaire()


feel this can be a property ie

val questionnaire = viewModel.questionnaire

jingtang10 · 2021-06-21T10:19:23Z

datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt

@@ -88,6 +88,9 @@ internal class QuestionnaireViewModel(state: SavedStateHandle) : ViewModel() {
  internal val questionnaireItemViewItemListFlow: Flow<List<QuestionnaireItemViewItem>> =
    modificationCount.map { questionnaireItemViewItemList }

+  /** The current [Questionnaire] . */
+  fun getQuestionnaire(): Questionnaire = questionnaire


just change the property to internal?

you can remove this getter function altogether!

jingtang10 · 2021-06-21T10:25:38Z

...pture/src/main/java/com/google/android/fhir/datacapture/validation/QuestionnaireValidator.kt

+        QuestionnaireResponseItemValidator.validate(questionnaireItem, questionnaireResponseItem)
+      )
+      if (questionnaireItem.hasNestedItemsWithinAnswers) {
+        // TODO(https://github.com/google/android-fhir/issues/487): Validates all answers.


this doesn't seem very difficult to do in this PR? can you just do it?

Well the issue is that I am reusing the validators which are used in QuestionnaireResponseItemValidator which all handle only single answer. I can make those changes in this PR if you want.

eg: Have a look at ValueConstraintExtensionValidator

oh yeah that's right -- sure this makes sense.

jingtang10 · 2021-06-21T10:26:41Z

.../test/java/com/google/android/fhir/datacapture/validation/MaxValueConstraintValidatorTest.kt

@@ -55,7 +55,7 @@ class MaxValueConstraintValidatorTest {
      MaxValueConstraintValidator.validate(questionnaireItem, questionnaireResponseItem)

    assertThat(validationResult.isValid).isFalse()
-    assertThat(validationResult.message.equals("Maximum value allowed is:200000")).isTrue()
+    assertThat(validationResult.message?.equals("Maximum value allowed is:200000")).isTrue()


Suggested change

assertThat(validationResult.message?.equals("Maximum value allowed is:200000")).isTrue()

assertThat(validationResult.message!!).isEqualTo("Maximum value allowed is:200000")

jingtang10 · 2021-06-21T10:29:16Z

...e/src/test/java/com/google/android/fhir/datacapture/validation/QuestionnaireValidatorTest.kt

+            )
+        )
+    val result = QuestionnaireValidator.validate(questionnaire.item, questionnaireResponse.item)
+    assertEquals(


use truth:

assertThat(...).isEqualTo()

I have addressed the comments. It still shows as changes requested.

jingtang10

Thanks @joiskash! 🎉

jingtang10 · 2021-06-22T08:59:25Z

datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt

@@ -88,6 +88,9 @@ internal class QuestionnaireViewModel(state: SavedStateHandle) : ViewModel() {
  internal val questionnaireItemViewItemListFlow: Flow<List<QuestionnaireItemViewItem>> =
    modificationCount.map { questionnaireItemViewItemList }

+  /** The current [Questionnaire] . */
+  fun getQuestionnaire(): Questionnaire = questionnaire


you can remove this getter function altogether!

jingtang10 · 2021-06-22T09:00:49Z

...c/main/java/com/google/android/fhir/datacapture/validation/QuestionnaireResponseValidator.kt

+  fun validate(
+    questionnaireItemList: List<Questionnaire.QuestionnaireItemComponent>,
+    questionnaireResponseItemList: List<QuestionnaireResponse.QuestionnaireResponseItemComponent>
+  ): Map<String, List<ValidationResult>> {
+    val questionnaireItemListIterator = questionnaireItemList.iterator()
+    val questionnaireResponseItemListIterator = questionnaireResponseItemList.iterator()
+    while (questionnaireItemListIterator.hasNext() &&
+      questionnaireResponseItemListIterator.hasNext()) {
+      val questionnaireItem = questionnaireItemListIterator.next()
+      val questionnaireResponseItem = questionnaireResponseItemListIterator.next()
+      linkIdToValidationResultMap[questionnaireItem.linkId] = mutableListOf()
+      linkIdToValidationResultMap[questionnaireItem.linkId]?.add(
+        QuestionnaireResponseItemValidator.validate(questionnaireItem, questionnaireResponseItem)
+      )
+      if (questionnaireItem.hasNestedItemsWithinAnswers) {
+        // TODO(https://github.com/google/android-fhir/issues/487): Validates all answers.
+        validate(questionnaireItem.item, questionnaireResponseItem.answer[0].item)
+      }
+      validate(questionnaireItem.item, questionnaireResponseItem.item)
+    }
+    return linkIdToValidationResultMap
+  }


don't have to do this in this PR - but you can probably create an iterator for questionnaire item + questionnaire response item. this logic is repeated in the questionnaire view model as well.

sure ill put a comment so that it is done while handling repeated answers

jingtang10 · 2021-06-22T10:06:32Z

...st/java/com/google/android/fhir/datacapture/validation/QuestionnaireResponseValidatorTest.kt

+    assertThat(result.get("a-question"))
+      .isEqualTo(
+        listOf(
+          ValidationResult(
+            false,
+            listOf("The maximum number of characters that are permitted in the answer is: 3")
+          )
+        )
+      )
+  }


Suggested change

assertThat(result.get("a-question"))

.isEqualTo(

listOf(

ValidationResult(

false,

listOf("The maximum number of characters that are permitted in the answer is: 3")

)

)

)

}

assertThat(result.get("a-question"))

.containsExactly(

ValidationResult(

false,

listOf("The maximum number of characters that are permitted in the answer is: 3")

)

)

}

...st/java/com/google/android/fhir/datacapture/validation/QuestionnaireResponseValidatorTest.kt

Co-authored-by: Jing Tang <jing.t86@gmail.com>

Added link id to validation result

14ad806

joiskash requested review from aditya-07, deepankarb, jingtang10, kevinmost, pankajkatoch and yigit as code owners June 14, 2021 11:23

joiskash added 3 commits June 15, 2021 15:53

Merge branch 'master' of https://github.com/google/android-fhir

bc96a50

Added function to validate Questionnaire response items before submit…

962a864

…ting

Fixed UT

a9f7bf3

joiskash force-pushed the kj/validation-api branch from 0da4fd1 to a9f7bf3 Compare June 15, 2021 10:25

jingtang10 requested changes Jun 15, 2021

View reviewed changes

joiskash and others added 6 commits June 16, 2021 12:17

Update datacapture/src/main/java/com/google/android/fhir/datacapture/…

8f7be59

…validation/ConstraintValidator.kt Co-authored-by: Jing Tang <jing.t86@gmail.com>

Reverted all changes to validation package

6b59724

Reverted changes and added function to return the current Questionnaire

4049c4a

Reverted changes to questionnaireFragment

87fec8e

Added new questionnaire Validator

5d99679

Ran spotless

5c3855d

joiskash requested a review from jingtang10 June 16, 2021 11:18

joiskash added 2 commits June 16, 2021 17:00

UT fix

41d335d

Added unit tests

167ad15

Tarun-Bhardwaj assigned joiskash Jun 18, 2021

joiskash requested review from jingtang10 and removed request for jingtang10 June 18, 2021 07:25

Tarun-Bhardwaj added this to the Q2 June 2021 milestone Jun 19, 2021

jingtang10 previously requested changes Jun 21, 2021

View reviewed changes

joiskash added 6 commits June 21, 2021 23:08

Renamed Class to QuestionnaireResponseValidator.kt

20e0ad2

Removed function and made questionnaire a property

70ce98a

Changed to isEqualTo

803120f

Using assertThat from truth instead of assertEquals

932681e

Ran spotless

6a498c1

Merge branch 'master' into kj/validation-api

379a1e4

joiskash requested a review from jingtang10 June 21, 2021 18:06

Tarun-Bhardwaj assigned jingtang10 and unassigned joiskash Jun 22, 2021

jingtang10 approved these changes Jun 22, 2021

View reviewed changes

joiskash added 3 commits June 22, 2021 15:02

Merge branch 'master' into kj/validation-api

c01b055

removed getQuestionnaire function

f1a0062

added Todo note to cover in the next PR

2c9f422

joiskash enabled auto-merge (squash) June 22, 2021 09:41

jingtang10 requested changes Jun 22, 2021

View reviewed changes

joiskash disabled auto-merge June 22, 2021 10:11

joiskash and others added 2 commits June 22, 2021 15:42

Updated test

6f50d8b

Co-authored-by: Jing Tang <jing.t86@gmail.com>

Merge branch 'master' into kj/validation-api

7e515ce

jingtang10 approved these changes Jun 22, 2021

View reviewed changes

jingtang10 enabled auto-merge (squash) June 22, 2021 10:47

jingtang10 merged commit b6950ae into master Jun 22, 2021

jingtang10 deleted the kj/validation-api branch June 22, 2021 10:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validation API for SDC #548

Validation API for SDC #548

joiskash commented Jun 14, 2021 •

edited

Loading

Tarun-Bhardwaj commented Jun 18, 2021

joiskash commented Jun 18, 2021

jingtang10 commented Jun 18, 2021

joiskash commented Jun 18, 2021

jingtang10 Jun 18, 2021

joiskash Jun 21, 2021

jingtang10 Jun 21, 2021

joiskash Jun 21, 2021

jingtang10 Jun 21, 2021

joiskash Jun 21, 2021

jingtang10 Jun 22, 2021

jingtang10 Jun 21, 2021

joiskash Jun 21, 2021

jingtang10 Jun 21, 2021

jingtang10 Jun 21, 2021

joiskash Jun 21, 2021

jingtang10 Jun 21, 2021

joiskash Jun 21, 2021

jingtang10 left a comment

jingtang10 Jun 22, 2021

jingtang10 Jun 22, 2021

joiskash Jun 22, 2021

jingtang10 Jun 22, 2021

	assertThat(validationResult.message?.equals("Maximum value allowed is:200000")).isTrue()
	assertThat(validationResult.message!!).isEqualTo("Maximum value allowed is:200000")

Validation API for SDC #548

Validation API for SDC #548

Conversation

joiskash commented Jun 14, 2021 • edited Loading

Tarun-Bhardwaj commented Jun 18, 2021

joiskash commented Jun 18, 2021

jingtang10 commented Jun 18, 2021

joiskash commented Jun 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jingtang10 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joiskash commented Jun 14, 2021 •

edited

Loading