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 Launch and Submission Timestamps to QR #2400

Open
wants to merge 51 commits into
base: master
Choose a base branch
from
Open

Add Launch and Submission Timestamps to QR #2400

wants to merge 51 commits into from

Conversation

hamza-vd
Copy link

@hamza-vd hamza-vd commented Jan 4, 2024 •
edited
Loading

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

Fixes #2372

Description
PR adds QuestionnaireItemComponent and QuestionnaireResponseItemComponent for form launch and submission timestamps

Alternative(s) considered
N/A

Type
Feature

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).

cc: @f-odhiambo @qiarie

@google-cla Google CLA
Copy link

google-cla bot commented Jan 4, 2024

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.

@hamza-vd hamza-vd marked this pull request as ready for review January 5, 2024 10:22
@hamza-vd hamza-vd requested review from santosh-pingle and a team as code owners January 5, 2024 10:22
@aditya-07
Copy link
Collaborator

@hamza-vd Please update the branch to latest.

@hamza-vd
Copy link
Author

@aditya-07 this is done

MJ1998
MJ1998 requested changes Jan 22, 2024
View reviewed changes
Copy link
Collaborator

@MJ1998 MJ1998 left a comment

Choose a reason for hiding this comment

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

I think we should not implicitly do this for all Questionnaires as it might be undesirable to receive unexpected items in QuestionnaireResponse.

We should consider adding the flag to either DataCaptureConfig or via QuestionnaireFragment.args!

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
@hamza-vd
Refactor static constant
0e9fcff
@hamza-vd
Merge branch 'master' into add-launch-and-submission-timestamps
bb44a5b 
# Conflicts:
#	datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireFragment.kt
@hamza-vd
Refactor toTimeZoneString() to MoreDates.kt
e9ba791
jingtang10
jingtang10 requested changes Jun 11, 2024
View reviewed changes
...e/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt Outdated Show resolved Hide resolved
...e/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt Outdated
internal fun QuestionnaireResponse.addLaunchTimestamp() {
val noLaunchTimeStampExists =
this.extension.none {
it.url == "http:https://github.com/google-android/questionnaire-launch-timestamp"
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't hardcode values like this

Copy link
Author

Choose a reason for hiding this comment

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

This is addressed

...e/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt Show resolved Hide resolved
jingtang10
jingtang10 requested changes Jul 9, 2024
View reviewed changes
...e/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt
}
set(value) {
val noLaunchTimeStampExists = this.extension.none { it.url == EXTENSION_LAUNCH_TIMESTAMP }
if (noLaunchTimeStampExists) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this extension already exists we probably should update the value?

Copy link
Author

Choose a reason for hiding this comment

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

We need to track the metric from the point the user has opened the Questionnaire until submission. The original timestamp should remain as is even if user navigates away from the questionnaire and later resumes on it. cc: @qiarie

...c/test/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponsesTest.kt Outdated Show resolved Hide resolved
...c/test/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponsesTest.kt Outdated
}

@Test
fun `test setLaunchTimestamp should not override previously added extension if any`() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we want this behavior?

Copy link
Author

Choose a reason for hiding this comment

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

Discussed here

...c/test/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponsesTest.kt Outdated Show resolved Hide resolved
hamza-vd and others added 2 commits July 10, 2024 14:08
@hamza-vd hamza-vd force-pushed the add-launch-and-submission-timestamps branch from 7c1534d to 8121c88 Compare July 11, 2024 06:41
@hamza-vd hamza-vd requested a review from jingtang10 July 29, 2024 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MJ1998 MJ1998 MJ1998 requested changes

@santosh-pingle santosh-pingle Awaiting requested review from santosh-pingle santosh-pingle is a code owner

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

@qiarie qiarie Awaiting requested review from qiarie

@ellykits ellykits Awaiting requested review from ellykits

@aditya-07 aditya-07 Awaiting requested review from aditya-07 aditya-07 is a code owner

@jingtang10 jingtang10 Awaiting requested review from jingtang10 jingtang10 is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

@aditya-07 aditya-07

@ellykits ellykits

Labels
None yet
Projects
Android FHIR SDK
Status: PR under Review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ability to Capture Questionnaire Launch and Submit Timestamps