Process skipped clients #506

junaidwarsivd · 2023-01-26T14:33:54Z

issue ref : https://github.com/opensrp/support/issues/267

ekigamba · 2023-01-26T15:25:20Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/job/GizReProcessJob.java

+import org.smartregister.giz.service.ReProcessSyncIntentService;
+import org.smartregister.job.BaseJob;
+
+public class GizReProcessJob extends BaseJob {


Rename this to GIZClientReprocessJob for now

ekigamba · 2023-01-26T15:28:31Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/recievers/SyncStatusReciever.java

+            if (fetchStatusSerializable instanceof FetchStatus) {
+                FetchStatus fetchStatus = (FetchStatus) fetchStatusSerializable;
+                    if (fetchStatus.equals(FetchStatus.fetched)) {
+                        GizReProcessJob.scheduleJobImmediately(GizReProcessJob.TAG);


I believe we want to call the job once sync is complete eg sync is done fetching all the latest records and not successfully fetched one batch. I'm not sure if this is what is currently implemented

ekigamba · 2023-01-26T15:29:03Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/repository/GizEventRepository.java

+
+
+        try {
+            if(formSubmissionIDs.size()>0)


Reformat the code to fix spacing in the if statement

ekigamba · 2023-01-26T15:29:21Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/repository/GizEventRepository.java

+            e.printStackTrace();
+        }
+    }
+


Use Timber to print exceptions. It sends exceptions to Crashlytics

ekigamba · 2023-01-26T15:29:40Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/repository/GizEventRepository.java

+        try {
+            if(formSubmissionIDs.size()>0)
+            GizUtils.initiateEventProcessing(formSubmissionIDs);
+        } catch (Exception e) {


I don't think we should be generally catching Exception here

ekigamba · 2023-01-26T15:30:51Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/repository/GizEventRepository.java

+    {
+        String missingClientRegister = "SELECT DISTINCT baseEntityId, formSubmissionId AS formSubmissionId FROM event WHERE baseEntityId IN (SELECT baseEntityId FROM client WHERE baseEntityId NOT IN (SELECT DISTINCT base_entity_id FROM client_register_type)) AND eventType LIKE '%Registration%'";
+        String missingECClient = "SELECT DISTINCT baseEntityId, formSubmissionId AS formSubmissionId FROM event WHERE baseEntityId IN (SELECT baseEntityId FROM client WHERE baseEntityId NOT IN (SELECT DISTINCT base_entity_id FROM ec_client)) AND eventType LIKE '%Registration%'";
+        List<String> formSubmissionIDs = new ArrayList<>();


This list allows duplicates which might not be handled

ekigamba · 2023-01-27T07:54:08Z

Fix codacy and build checks. Kindly make sure you fix this before requesting for a review

ekigamba · 2023-01-27T12:26:40Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/recievers/SyncStatusReciever.java

+
+import static org.smartregister.receiver.SyncStatusBroadcastReceiver.EXTRA_FETCH_STATUS;
+
+public class SyncStatusReciever  extends BroadcastReceiver {


Rename this to GizReprocessSyncStatusReceiver since this is only being used by the GizClientReprocessJob

ekigamba · 2023-01-27T12:33:01Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/repository/GizEventRepository.java

+        SQLiteDatabase database = getReadableDatabase();
+        Cursor cursor = database.rawQuery(query, null);
+        int columnIndex = cursor.getColumnIndex("formSubmissionId");
+        if(cursor.getCount() >  0) {


Reformat the code file to fix the spacing

ekigamba · 2023-01-27T12:35:36Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/repository/GizEventRepository.java

@@ -21,4 +25,37 @@ public boolean hasEvent(@NonNull String baseEntityId, @NonNull String eventType)
        }
        return hasEvent;
    }
+
+    public void ReprocessClients()


😄 camelCase please and let's rename this to processSkippedClients or something a bit more descriptive enough that only unprocessed clients are being processed here

ekigamba · 2023-01-27T12:36:03Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/service/ReProcessSyncIntentService.java

+
+public class ReProcessSyncIntentService extends IntentService {
+
+    public static final String TAG = "ReValidateSyncIntentService";


Let's have the tag similar to the name of the service

ekigamba · 2023-02-01T10:01:32Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/job/GIZClientReprocessJob.java

+
+public class GIZClientReprocessJob extends BaseJob {
+
+    public static final String TAG = "GizReValidationJob";


Change this tag also to match the Job

…e action added a check for existing formsubmission id while population the list corrected the job tag

ekigamba · 2023-02-02T06:19:54Z

...giz-malawi/src/main/java/org/smartregister/giz/recievers/GizReprocessSyncStatusReceiver.java

+            if(isComplete)
+            {
+                GIZClientReprocessJob.scheduleJobImmediately(GIZClientReprocessJob.TAG);
+                Timber.d("fetch completed");


This log might be misleading

ekigamba · 2023-02-02T06:34:23Z

opensrp-giz-malawi/build.gradle

@@ -404,6 +404,8 @@ dependencies {

    implementation "androidx.multidex:multidex:2.0.1"
    testImplementation "androidx.work:work-testing:2.7.1"
+    androidTestImplementation 'androidx.test:runner:1.3.0'


These two are also applied for instrumentation tests that are not changed in this PR. Confirm that we need these and probably change this to testImplementation

ekigamba · 2023-02-02T06:38:17Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/repository/GizEventRepository.java

+        if (cursor.getCount() > 0) {
+            cursor.moveToFirst();
+            do {
+                if (!ids.contains(cursor.getString(columnIndex)))


Time complexity for contains is n. I still believe that a HashSet is better here

hilpitome · 2023-02-06T08:28:21Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/repository/GizEventRepository.java

@@ -21,4 +27,33 @@ public boolean hasEvent(@NonNull String baseEntityId, @NonNull String eventType)
        }
        return hasEvent;
    }
+
+    public void processSkippedClients() {
+        String missingClientRegister = "SELECT DISTINCT baseEntityId, formSubmissionId AS formSubmissionId FROM event WHERE baseEntityId IN (SELECT baseEntityId FROM client WHERE baseEntityId NOT IN (SELECT DISTINCT base_entity_id FROM client_register_type)) AND eventType LIKE '%Registration%'";


I am not sure we need to alias formSubmissionId here

hilpitome · 2023-02-06T08:28:51Z

opensrp-giz-malawi/src/main/java/org/smartregister/giz/repository/GizEventRepository.java

+
+    public void processSkippedClients() {
+        String missingClientRegister = "SELECT DISTINCT baseEntityId, formSubmissionId AS formSubmissionId FROM event WHERE baseEntityId IN (SELECT baseEntityId FROM client WHERE baseEntityId NOT IN (SELECT DISTINCT base_entity_id FROM client_register_type)) AND eventType LIKE '%Registration%'";
+        String missingECClient = "SELECT DISTINCT baseEntityId, formSubmissionId AS formSubmissionId FROM event WHERE baseEntityId IN (SELECT baseEntityId FROM client WHERE baseEntityId NOT IN (SELECT DISTINCT base_entity_id FROM ec_client)) AND eventType LIKE '%Registration%'";


check previous comment on formSubmissionId

ekigamba · 2023-03-07T12:58:17Z

opensrp-giz-malawi/src/main/AndroidManifest.xml

-        <service android:name="org.smartregister.sync.intent.SettingsSyncIntentService" />
-        <service android:name="org.smartregister.sync.intent.SyncIntentService" />
-        <service android:name=".service.ReProcessSyncIntentService"/>
+        <service android:name="org.smartregister.immunization.service.intent.VaccineIntentService" android:exported="true" />


Why are these services set as exported and why is this explicitly declared?

ekigamba · 2023-03-07T13:01:27Z

...giz-malawi/src/main/java/org/smartregister/giz/recievers/GizReprocessSyncStatusReceiver.java

+            }
+        }
+
+        Timber.d("broadcast Recieved");


I think the log is too vague and it would be easier to understand if it was a bit more specific on the broadcast that was received

… clients - Update versionCode and versionName to 47 and 0.4.7

- Update immunization library to fix duplicate vaccines and services - Fix client-core version

- Add config to close ec_client records when an OPD Close event is encountered - Filter out closed records from the OPD register

- Update child register query to use ec_client.is_removed flag

Fixes #510

- These events had not been previously processed due to a bug

added broadcast reciever and service to reprocess the client

b5b5dbb

junaidwarsivd requested a review from ekigamba January 26, 2023 14:33

ekigamba suggested changes Jan 26, 2023

View reviewed changes

ekigamba suggested changes Jan 27, 2023

View reviewed changes

junaidwarsivd added 2 commits January 31, 2023 05:21

added the corrections in the PR review

21b014b

added UnitTest for GizEventRepository

16470ec

ekigamba suggested changes Feb 1, 2023

View reviewed changes

junaidwarsivd added 3 commits February 1, 2023 18:21

added more corrections to the PR changed the job trigger to isComplet…

443625a

…e action added a check for existing formsubmission id while population the list corrected the job tag

corrected the condition

b322055

removed unused imports

77e7122

junaidwarsivd requested a review from ekigamba February 2, 2023 06:16

reformatted the code

c405d49

ekigamba suggested changes Feb 2, 2023

View reviewed changes

junaidwarsivd added 3 commits February 2, 2023 13:31

updated according to the PR feedback

51d9247

changed androidTestImplemenation to testImplementation

0402130

codacy issues removed

d3d2cfd

hilpitome reviewed Feb 6, 2023

View reviewed changes

added android 12 compatibility

277e50c

ekigamba suggested changes Mar 7, 2023

View reviewed changes

ekigamba added 2 commits March 14, 2023 11:11

Code cleanup

89fb705

Log skipped clients for reprocessing

35ca072

ekigamba changed the title ~~added broadcast reciever and service to reprocess the client~~ Process skipped client processing Mar 14, 2023

ekigamba changed the title ~~Process skipped client processing~~ Process skipped clients Mar 14, 2023

ekigamba added 5 commits March 21, 2023 16:11

Process skipped clients events in-order of eventDate after processing…

fa8589f

… clients - Update versionCode and versionName to 47 and 0.4.7

Append APK file name with version and variant

eb00e3d

Separate oauth client id and secret for different variants

e2a4098

Comment out code in client processor

0c4106b

Fix commented out code in client processor

2f54b4b

ekigamba added 12 commits March 21, 2023 16:12

Update ANC version to fix next button in form

5c5cf23

- Update immunization library to fix duplicate vaccines and services - Fix client-core version

Fix OPD Close event processing

b4bb240

- Add config to close ec_client records when an OPD Close event is encountered - Filter out closed records from the OPD register

Mark deceased children as closed in ec_client

6bd8dfa

- Update child register query to use ec_client.is_removed flag

Update README setup instructions

a834abb

Remove unused imports

07c67a8

Fix PNC registration form cannot move to the next page

4626b37

Fixes #510

Process death and OPD close events

7e93ddf

- These events had not been previously processed due to a bug

Fix All Clients register search to remove dead clients

27b7787

Code cleanup

502694f

Update versionName and versionCode to 0.4.8 and 48

eeab005

Fix Child register search showing deceased clients

57aa2d3

Update versionName and versionCode to 0.4.9 and 49

af4d112

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Process skipped clients #506

Process skipped clients #506

junaidwarsivd commented Jan 26, 2023

ekigamba Jan 26, 2023

ekigamba Jan 26, 2023

ekigamba Jan 26, 2023

ekigamba Jan 26, 2023

ekigamba Jan 26, 2023

ekigamba Jan 26, 2023

ekigamba commented Jan 27, 2023

ekigamba Jan 27, 2023

ekigamba Jan 27, 2023

ekigamba Jan 27, 2023

ekigamba Jan 27, 2023

ekigamba Feb 1, 2023

ekigamba Feb 2, 2023

ekigamba Feb 2, 2023

ekigamba Feb 2, 2023

hilpitome Feb 6, 2023

hilpitome Feb 6, 2023

ekigamba Mar 7, 2023

ekigamba Mar 7, 2023


		import static org.smartregister.receiver.SyncStatusBroadcastReceiver.EXTRA_FETCH_STATUS;

		public class SyncStatusReciever extends BroadcastReceiver {


		public class ReProcessSyncIntentService extends IntentService {

		public static final String TAG = "ReValidateSyncIntentService";


		public class GIZClientReprocessJob extends BaseJob {

		public static final String TAG = "GizReValidationJob";

Process skipped clients #506

Are you sure you want to change the base?

Process skipped clients #506

Conversation

junaidwarsivd commented Jan 26, 2023

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

ekigamba commented Jan 27, 2023

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