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

Anc issue fix 906 for the human readable values not updating in events when the translation is on #892

Merged
merged 14 commits into from
Oct 18, 2022

Conversation

junaidwarsivd
Copy link
Contributor

issue reference : opensrp/opensrp-client-anc#906

@SebaMutuku SebaMutuku changed the base branch from migrate-crashlytics to master October 5, 2022 09:59
@SebaMutuku SebaMutuku changed the base branch from master to migrate-crashlytics October 5, 2022 10:07
@junaidwarsivd junaidwarsivd changed the base branch from migrate-crashlytics to master October 11, 2022 12:31
@junaidwarsivd junaidwarsivd changed the base branch from master to migrate-crashlytics October 12, 2022 08:20
…' into anc-issue-fix-906

# Conflicts:
#	gradle.properties
Copy link
Member

@dubdabasoduba dubdabasoduba left a comment

Choose a reason for hiding this comment

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

Please add tests fro the new functionality on the JsonFormUtils

Comment on lines +401 to +408

public void saveUserId(String username, String userId) {
preferences.edit().putString(AllConstants.USER_ID_PREFIX + username, userId).apply();
}

public String getUserId(String userName) {
return StringUtils.isNotBlank(userName) ? preferences.getString(AllConstants.USER_ID_PREFIX + userName, null) : "";
}
Copy link
Member

Choose a reason for hiding this comment

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

@junaidwarsivd What does the user id do? Why are we concatenating it with the username?

Copy link
Contributor Author

@junaidwarsivd junaidwarsivd Oct 14, 2022

Choose a reason for hiding this comment

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

@dubdabasoduba these changes are from the https://github.com/opensrp/opensrp-client-core/pull/887/files merge as it has some resolved codacy issues as well as these changes also needs to be merged in the targeted branch issue ref# opensrp/opensrp-client-anc#878

Comment on lines +852 to +857

public void saveUserId(String userName, String baseEntityId) {
if (userName != null) {
allSharedPreferences.saveUserId(userName, baseEntityId);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Not sure what we are using the user ID for

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +356 to +363
if(valueOpenMRSAttribute.has(VALUE))
popupJson.put(VALUE, valueOpenMRSAttribute.getString(VALUE));
else
popupJson.put(VALUE, valueOpenMRSAttribute.getString(OPENMRS_ENTITY_ID));
if(valueOpenMRSAttribute.has(TEXT))
popupJson.put(TEXT,valueOpenMRSAttribute.getString(TEXT));
if(valueOpenMRSAttribute.has(OPTIONS_FIELD_NAME))
popupJson.put(OPTIONS_FIELD_NAME,valueOpenMRSAttribute.getJSONArray(OPTIONS_FIELD_NAME));
Copy link
Member

Choose a reason for hiding this comment

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

Please share a sample JSON for the popupJson

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dubdabasoduba
{"key":"hiv_test_status","openmrs_entity":"concept","openmrs_entity_id":"165383AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","value":"done_today","text":"Done today","options":[{"key":"done_today","text":"Done today","type":"done_today","openmrs_entity_parent":"","openmrs_entity":"concept","openmrs_entity_id":"165383AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"}],"type":"extended_radio_button","openmrs_entity_parent":""}

in options only the selected key is added to avoid the extra looping when filling up the human readable values

@dubdabasoduba dubdabasoduba merged commit 5fdb3dd into migrate-crashlytics Oct 18, 2022
@dubdabasoduba dubdabasoduba deleted the anc-issue-fix-906 branch October 18, 2022 10:19
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 this pull request may close these issues.

3 participants