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

Optimize native forms v2 #522

Open
wants to merge 35 commits into
base: develop
Choose a base branch
from
Open

Optimize native forms v2 #522

wants to merge 35 commits into from

Conversation

bennsimon
Copy link
Member

@bennsimon bennsimon commented May 29, 2020

#511

  • Make number of contact schedules shown on summary configurable

  • Add contact_no to globals for quick check, when its opened from register Activity

  • Fix null GA

  • Fix incorrect count of profile contact container relevant required fields

  • Optimized native forms

  • Support use of boolean is_first_contact as due check strategy

  • Make ProfileActivity.class configurable

@@ -130,6 +133,25 @@ protected void onResumption() {
}
}

private void populatePreviousContactMissingEssentials(HashMap<String, String> clientDetails) {
Copy link

Choose a reason for hiding this comment

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

Method populatePreviousContactMissingEssentials has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.

}
}

private static void updateLocationTree(@NonNull JSONArray fields,
Copy link

Choose a reason for hiding this comment

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

Method updateLocationTree has a Cognitive Complexity of 15 (exceeds 5 allowed). Consider refactoring.

@@ -187,6 +241,28 @@ public static JSONObject getFieldJSONObject(JSONArray jsonArray, String key) {
}
}

protected static void processLocationFields(@NonNull JSONArray fields) throws JSONException {
Copy link

Choose a reason for hiding this comment

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

Method processLocationFields has a Cognitive Complexity of 17 (exceeds 5 allowed). Consider refactoring.

@@ -222,31 +302,60 @@ private static void getDobStrings(JSONArray fields) throws JSONException {
}
}

private static void initializeFirstContactValues(JSONArray fields) throws JSONException {
private static String initializeFirstContactValues(JSONArray fields) throws JSONException {
Copy link

Choose a reason for hiding this comment

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

Method initializeFirstContactValues has a Cognitive Complexity of 10 (exceeds 5 allowed). Consider refactoring.

@@ -222,31 +302,60 @@ private static void getDobStrings(JSONArray fields) throws JSONException {
}
}

private static void initializeFirstContactValues(JSONArray fields) throws JSONException {
private static String initializeFirstContactValues(JSONArray fields) throws JSONException {
Copy link

Choose a reason for hiding this comment

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

Method initializeFirstContactValues has 28 lines of code (exceeds 25 allowed). Consider refactoring.

return getProperties(AncLibrary.getInstance().getApplicationContext()).getProperty(ConstantsUtils.Properties.DUE_CHECK_STRATEGY, "");
}

public static HashMap<String, HashMap<String, String>> buildRepeatingGroupValues(@NonNull JSONArray fields, String fieldName) throws JSONException {
Copy link

Choose a reason for hiding this comment

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

Method buildRepeatingGroupValues has a Cognitive Complexity of 18 (exceeds 5 allowed). Consider refactoring.

return getProperties(AncLibrary.getInstance().getApplicationContext()).getProperty(ConstantsUtils.Properties.DUE_CHECK_STRATEGY, "");
}

public static HashMap<String, HashMap<String, String>> buildRepeatingGroupValues(@NonNull JSONArray fields, String fieldName) throws JSONException {
Copy link

Choose a reason for hiding this comment

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

Method buildRepeatingGroupValues has 27 lines of code (exceeds 25 allowed). Consider refactoring.


}

public static void createPreviousVisitFromGroup(@NonNull String strGroup, @NonNull String baseEntityId) throws JSONException {
Copy link

Choose a reason for hiding this comment

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

Method createPreviousVisitFromGroup has 30 lines of code (exceeds 25 allowed). Consider refactoring.

@codeclimate
Copy link

codeclimate bot commented Jun 24, 2020

Code Climate has analyzed commit dd5c4c3 and detected 16 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 16

View more on Code Climate.

@bennsimon bennsimon changed the base branch from master to develop June 24, 2020 09:19
@bennsimon bennsimon self-assigned this Jun 25, 2020
@bennsimon bennsimon marked this pull request as draft June 25, 2020 13:14
@coveralls
Copy link

coveralls commented Jul 1, 2020

Pull Request Test Coverage Report for Build 3231

  • 163 of 406 (40.15%) changed or added relevant lines in 27 files are covered.
  • 45 unchanged lines in 8 files lost coverage.
  • Overall coverage decreased (-0.01%) to 45.932%

Changes Missing Coverage Covered Lines Changed/Added Lines %
opensrp-anc/src/main/java/org/smartregister/anc/library/activity/ProfileActivity.java 0 1 0.0%
opensrp-anc/src/main/java/org/smartregister/anc/library/model/ContactSummaryModel.java 3 4 75.0%
opensrp-anc/src/main/java/org/smartregister/anc/library/repository/PreviousContactRepository.java 5 6 83.33%
opensrp-anc/src/main/java/org/smartregister/anc/library/presenter/ContactPresenter.java 0 2 0.0%
opensrp-anc/src/main/java/org/smartregister/anc/library/task/LoadContactSummaryDataTask.java 3 5 60.0%
opensrp-anc/src/main/java/org/smartregister/anc/library/AncLibrary.java 0 3 0.0%
opensrp-anc/src/main/java/org/smartregister/anc/library/event/PatientRemovedEvent.java 0 3 0.0%
opensrp-anc/src/main/java/org/smartregister/anc/library/interactor/ContactInteractor.java 0 3 0.0%
opensrp-anc/src/main/java/org/smartregister/anc/library/adapter/LastContactAdapter.java 18 22 81.82%
opensrp-anc/src/main/java/org/smartregister/anc/library/util/AncMetadata.java 1 5 20.0%
Files with Coverage Reduction New Missed Lines %
opensrp-anc/src/main/java/org/smartregister/anc/library/activity/ContactSummarySendActivity.java 1 39.39%
opensrp-anc/src/main/java/org/smartregister/anc/library/interactor/RegisterInteractor.java 1 50.35%
opensrp-anc/src/main/java/org/smartregister/anc/library/presenter/ContactWizardJsonFormFragmentPresenter.java 1 0%
opensrp-anc/src/main/java/org/smartregister/anc/library/util/ConstantsUtils.java 1 0%
opensrp-anc/src/main/java/org/smartregister/anc/library/domain/Contact.java 2 36.84%
opensrp-anc/src/main/java/org/smartregister/anc/library/util/Utils.java 2 52.74%
opensrp-anc/src/main/java/org/smartregister/anc/library/util/ANCJsonFormUtils.java 4 65.19%
opensrp-anc/src/main/java/org/smartregister/anc/library/activity/MainContactActivity.java 33 56.0%
Totals Coverage Status
Change from base Build 3120: -0.01%
Covered Lines: 3856
Relevant Lines: 8395

💛 - Coveralls

@dubdabasoduba
Copy link
Member

@bennsimon is this ready for review?

@bennsimon bennsimon marked this pull request as ready for review July 3, 2020 07:41
android.debug.obsoleteApi=true

## PUBLISHING VARS
VERSION_NAME=2.0.1-SNAPSHOT
VERSION_NAME=2.0.3.13-LOCAL-SNAPSHOT
Copy link
Member

Choose a reason for hiding this comment

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

Change this version name

@@ -143,7 +143,7 @@ dependencies {

implementation fileTree(dir: 'libs', include: ['*.jar'])
implementation 'com.android.support:appcompat-v7:28.0.0'
implementation('org.smartregister:opensrp-client-native-form:1.7.28-SNAPSHOT@aar') {
implementation('org.smartregister:opensrp-client-native-form:1.9.2-r117-OPTIMIZED-LOCAL-SNAPSHOT@aar') {
Copy link
Member

Choose a reason for hiding this comment

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

Update to the correct version name

@@ -90,6 +93,12 @@ private void initializeMainContactContainers() {
process(contactForms);
requiredFieldsMap.put(ConstantsUtils.JsonFormUtils.ANC_TEST_TASKS_ENCOUNTER_TYPE, 0);

if (StringUtils.isNotBlank(formInvalidFields) && contactNo > 1 && !PatientRepository.isFirstVisit(baseEntityId)) {
String[] pair = formInvalidFields.split(":");
Copy link
Member

Choose a reason for hiding this comment

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

Should check the size of this string array. Otherwise, you will get an out of bounds exception

@@ -318,7 +328,7 @@ private void processRequiredStepsField(JSONObject object) throws Exception {
Iterator<String> keys = object.keys();
while (keys.hasNext()) {
String key = keys.next();
if (key.startsWith(RuleConstant.STEP)) {
if (key.startsWith(RuleConstant.STEP) && !object.getJSONObject(key).has("skipped")) {
Copy link
Member

Choose a reason for hiding this comment

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

Move the skipped string to constants

@Override
protected void onActivityResult(int requestCode, int resultCode, Intent data) {
if (resultCode == RESULT_OK && data != null) {
formInvalidFields = data.getStringExtra("formInvalidFields");
Copy link
Member

Choose a reason for hiding this comment

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

Move this string to constants

@@ -130,7 +130,7 @@ android {
}

debug {
resValue "string", 'opensrp_url', '"https://anc-stage.smartregister.org/opensrp/"'
resValue "string", 'opensrp_url', '"https://opensrp-jembi-eregister.smartregister.org/opensrp/"' //TODO should be changed to anc staging url
Copy link
Member

Choose a reason for hiding this comment

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

Update this to use the ANC reference app URL

Comment on lines 224 to 234
// implementation('org.smartregister:opensrp-client-native-form:1.7.30-OPTIMIZE_FORMS-SNAPSHOT@aar') {
// transitive = true
// exclude group: 'com.android.support', module: 'recyclerview-v7'
// exclude group: 'com.android.support', module: 'appcompat-v7'
// exclude group: 'com.android.support', module: 'cardview-v7'
// exclude group: 'com.android.support', module: 'support-media-compat'
// exclude group: 'com.android.support', module: 'support-v4'
// exclude group: 'com.android.support', module: 'design'
// exclude group: 'org.yaml', module: 'snakeyaml'
// exclude group: 'io.ona.rdt-capture', module: 'lib'
// }
Copy link
Member

Choose a reason for hiding this comment

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

Why have you disabled this?

add check got contact summar model to avoid duplicates
@keymane
Copy link

keymane commented Jul 14, 2020

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- opensrp-anc/src/main/java/org/smartregister/anc/library/model/ContactSummaryModel.java  1
- opensrp-anc/src/main/java/org/smartregister/anc/library/activity/MainContactActivity.java  1
- opensrp-anc/src/test/java/org/smartregister/anc/library/activity/BaseActivityUnitTest.java  1
- opensrp-anc/src/main/java/org/smartregister/anc/library/task/LoadContactSummaryDataTask.java  5
- opensrp-anc/src/main/java/org/smartregister/anc/library/fragment/ProfileContactsFragment.java  1
- opensrp-anc/src/main/java/org/smartregister/anc/library/activity/ContactSummarySendActivity.java  3
- opensrp-anc/src/main/java/org/smartregister/anc/library/presenter/ContactSummaryPresenter.java  2
         

See the complete overview on Codacy

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.

5 participants