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.

Sign up for GitHub

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

Jump to bottom

Add support for Answer Options Toggle Extension #2034

Merged
merged 24 commits into from
Jul 27, 2023
Merged

Add support for Answer Options Toggle Extension #2034

merged 24 commits into from
Jul 27, 2023

Conversation

LZRS
Copy link
Collaborator

@LZRS LZRS commented Jun 13, 2023 •
edited
Loading

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

Fixes #1773

Description
Clear and concise code change description.

Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?

Type
Choose one: (Bug fix | Feature | Documentation | Testing | Code health | Builds | Releases | Other)

Screenshots (if applicable)

Checklist

  • I have read and acknowledged the Code of conduct.
  • I have read the Contributing page.
  • 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 demo app(s) to verify my change fixes the issue and/or does not break the demo app(s).

@google-cla
Copy link

google-cla bot commented Jun 13, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@LZRS LZRS marked this pull request as ready for review June 14, 2023 11:01
@LZRS LZRS requested review from a team and santosh-pingle as code owners June 14, 2023 11:01
@LZRS LZRS requested a review from omarismail94 June 14, 2023 11:01
@LZRS LZRS force-pushed the 1773-support-answer-options-toggle branch from 50024ac to 5799d2f Compare June 20, 2023 15:28
@LZRS LZRS force-pushed the 1773-support-answer-options-toggle branch from 6a89c2d to ed9eb82 Compare June 22, 2023 15:17
@FikriMilano
Copy link
Collaborator

@LZRS thanks for the PR :D
Is this ready for review?

@LZRS
Copy link
Collaborator Author

LZRS commented Jun 26, 2023

@LZRS thanks for the PR :D
Is this ready for review?

Yes, it's ready for review

jingtang10
jingtang10 requested changes Jun 27, 2023
View reviewed changes
Copy link
Collaborator

@jingtang10 jingtang10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this change - really good tests and in general high quality code!

The most important comment is my first comment in QuestionnaireViewItem.kt. Please take a look and address. I think as a result this will also improve the general code quality of the SDC library. At the moment we're incorrectly handling some answer option logic in the UI code - this should be changed.

Thanks!

datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireViewItem.kt Outdated Show resolved Hide resolved
datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt Outdated Show resolved Hide resolved
datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt Outdated Show resolved Hide resolved
...capture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireViewItemTest.kt Outdated Show resolved Hide resolved
datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireViewItem.kt Outdated Show resolved Hide resolved
...capture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireViewItemTest.kt Show resolved Hide resolved
...capture/src/test/java/com/google/android/fhir/datacapture/views/QuestionnaireViewItemTest.kt Outdated Show resolved Hide resolved
FikriMilano
FikriMilano suggested changes Jul 6, 2023
View reviewed changes
Copy link
Collaborator

@FikriMilano FikriMilano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the good work! :D
I left a comment.

datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt Outdated Show resolved Hide resolved
@MJ1998 MJ1998 mentioned this pull request Jul 7, 2023
Closed
@LZRS LZRS requested review from FikriMilano and jingtang10 July 11, 2023 07:02
@FikriMilano FikriMilano mentioned this pull request Jul 11, 2023
Merged
7 tasks
FikriMilano
FikriMilano approved these changes Jul 12, 2023
View reviewed changes
Copy link
Collaborator

@FikriMilano FikriMilano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks @LZRS :D

@jingtang10 jingtang10 mentioned this pull request Jul 20, 2023
Merged
7 tasks
@jingtang10
Copy link
Collaborator

jingtang10 commented Jul 21, 2023 •
edited
Loading

@omarismail94 @FikriMilano I want to bring this PR to your attention too. @LZRS managed to remove one call to initialize the questionnaire state whilst maintaining the correct behavior (I assume he tested this thoroughly 😀) - I wonder if this PR would have a positive impact on the performance. Can you two take a look at this?

Thanks
Jing

@LZRS LZRS requested a review from jingtang10 July 21, 2023 16:56
@jingtang10 jingtang10 enabled auto-merge (squash) July 24, 2023 14:30
auto-merge was automatically disabled July 24, 2023 14:37

Head branch was pushed to by a user without write access

@LZRS LZRS requested a review from jingtang10 July 24, 2023 14:39
@LZRS
Replace answerDisabledCallback with call to remove disabled answers
6595bcf 
After evaluation of enabled answer options, and therefore disabled questionnaire response answers
@LZRS LZRS requested a review from jingtang10 July 25, 2023 15:50
jingtang10
jingtang10 approved these changes Jul 26, 2023
View reviewed changes
Copy link
Collaborator

@jingtang10 jingtang10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this - just a really small comment otherwise good to merge

@jingtang10 jingtang10 enabled auto-merge (squash) July 26, 2023 16:32
@jingtang10 jingtang10 merged commit edfa4b2 into google:master Jul 27, 2023
@jingtang10 jingtang10 deleted the 1773-support-answer-options-toggle branch July 27, 2023 00:29
LZRS added a commit to opensrp/android-fhir that referenced this pull request Aug 11, 2023
@LZRS
Add support for Answer Options Toggle Extension (google#2034)
39a141a 
---------
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MJ1998 MJ1998 MJ1998 left review comments

@FikriMilano FikriMilano FikriMilano approved these changes

@jingtang10 jingtang10 jingtang10 approved these changes

@santosh-pingle santosh-pingle Awaiting requested review from santosh-pingle

@omarismail94 omarismail94 Awaiting requested review from omarismail94 omarismail94 was automatically assigned from google/android-fhir

Assignees
No one assigned
Labels
None yet
Projects
Android FHIR SDK
Status: Complete
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for Extension: Answer Options Toggle Expression
6 participants
@LZRS @FikriMilano @jingtang10 @MJ1998 @android-fhir-bot @f-odhiambo