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

Issue fix sync pr1855 #2111

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

Issue fix sync pr1855 #2111

wants to merge 6 commits into from

Conversation

junaidwarsivd
Copy link

added the missng code from the Pr 1855 in ChwSyncIntentService

org.smartregister.util.Utils.getAllSharedPreferences().saveLastSyncDate(serverVersion);

// flag all contentious events as unsynced
EventDao.markEventsForReUpload();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not add a test for this?

Copy link
Author

Choose a reason for hiding this comment

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

@ekigamba this is the Protected method writing a unit test might not be applicable for it

@ekigamba
Copy link
Contributor

Kindly update the issue title

@ekigamba
Copy link
Contributor

  • Update the CHW app versions
  • In another PR, we'll need to update this for android 12 compatibility. This will happen later once the security bugs on client core are fixed


@Test
public void testHandleSync() {
chwSyncIntentService.handleSync();

Choose a reason for hiding this comment

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

@junaidwarsivd I am not sure if testHandleSync() method is complete

@Test
public void testGoToChildProfile() {
AllClientsUtils.goToChildProfile(mockActivity, mockPatient, mockBundle);
verify(mockActivity).startActivity(ArgumentMatchers.any());
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Kindly update this test to match the expected activity class here
  2. Check that the intent has the correct extras as expected


verify(database).execSQL("ALTER TABLE ec_family ADD COLUMN entity_type VARCHAR; UPDATE ec_family SET entity_type = 'ec_family' WHERE id is not null;");

verify(database).execSQL("ALTER TABLE ec_family ADD COLUMN entity_type VARCHAR; UPDATE ec_family SET entity_type = 'ec_family' WHERE id is not null;");
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Kindly remove the duplicated verification here
  2. Test/assert that the fields are added to the FTS table

verify(mockBottomNavigationView).setLabelVisibilityMode(LabelVisibilityMode.LABEL_VISIBILITY_LABELED);
verify(mockBottomNavigationView).inflateMenu(R.menu.bottom_nav_menu);
verify(mockBottomNavigationHelper).disableShiftMode(mockBottomNavigationView);
verify(mockBottomNavigationView).setOnNavigationItemSelectedListener(mockListener);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we verify if the QR, job aids and reports items are removed if the flavor does not have them

// fetch the last downloaded serverVersion before any unsyced data
Long serverVersion = EventDao.getMinimumVerifiedServerVersion();
if (serverVersion != null)
org.smartregister.util.Utils.getAllSharedPreferences().saveLastSyncDate(serverVersion);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this try to sync already synced events and clients? I just want to understand this

@ekigamba
Copy link
Contributor

ekigamba commented Mar 9, 2023

Kindly rename the PR so that it's easy to search

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