Skip to content
This repository has been archived by the owner on Apr 9, 2021. It is now read-only.

[MRG] Add reset settings option. #300

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

Conversation

huangyz0918
Copy link
Contributor

@huangyz0918 huangyz0918 commented Aug 15, 2019

Closes #299

What has been done to verify that this works as intended?

Test using my Nexus 6P

Screenshots,

Why is this the best possible solution? Were any other approaches considered?

I only reset the settings, reset related to forms can be done by Collect, so we don't need to build a duplicate feature.

Note, the arrange of preference items will be improved by #290 , that includes some groups.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

N/A

Before submitting this PR, please make sure you have:

  • run ./gradlew checkCode and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.

@huangyz0918 huangyz0918 changed the title Add reset settings option. [MRG] Add reset settings option. Aug 15, 2019
@huangyz0918
Copy link
Contributor Author

Now the dialog looks like

Copy link
Contributor

@lakshyagupta21 lakshyagupta21 left a comment

Choose a reason for hiding this comment

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

Everything looks good to me, there are few string resources which needs improvements, please fix those.

skunkworks_crow/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
skunkworks_crow/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
skunkworks_crow/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
skunkworks_crow/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
@lakshyagupta21
Copy link
Contributor

@huangyz0918 I've tested this and it looks like whenever I clear the saved forms the forms are still shown in the sent and received tabs of the blank forms

@huangyz0918
Copy link
Contributor Author

@huangyz0918 I've tested this and it looks like whenever I clear the saved forms the forms are still shown in the sent and received tabs of the blank forms

In the code you can see I just deleted all the files in share folder and create a new folder. The share related databases should be reset successfully. According to you comment, do you mean we should also clear the odk folder?

@lakshyagupta21
Copy link
Contributor

In the code you can see I just deleted all the files in share folder and create a new folder. The share related databases should be reset successfully. According to you comment, do you mean we should also clear the odk folder?

No, skunkworks-crow should not delete any of the things present in ODK folder.

I'm testing it again, it could be some problem at my end because I took a pull from master in your branch while testing because bluetooth was not working.

@lakshyagupta21
Copy link
Contributor

I'm testing it again, it could be some problem at my end because I took a pull from master in your branch while testing because bluetooth was not working.

There seems to be some issue with InMemory map that we've related to forms information.
Steps to reproduce the problem.

  1. Transfer any filled form from one device to another.
  2. After the transfer is a successful tap on a form which you sent.
  3. Go to sent tab.
  4. You'll see the form information is still shown in that tab.

I checked the tables and db are deleted.

@huangyz0918
Copy link
Contributor Author

be some issue with InMemory map

So it seems we should clear the cache in memory after resetting the forms? @lakshyagupta21

@lakshyagupta21
Copy link
Contributor

So it seems we should clear the cache in memory after resetting the forms? @lakshyagupta21

Hmm, or lets reset the state of variables in onResume, anyways we're not doing any high memory usage operation in MainActivity.

@huangyz0918
Copy link
Contributor Author

huangyz0918 commented Aug 18, 2019

Hmm, or lets reset the state of variables in onResume, anyways we're not doing any high memory usage operation in MainActivity.

Sure, let me have a try.

@huangyz0918
Copy link
Contributor Author

huangyz0918 commented Aug 19, 2019

@lakshyagupta21 How about restarting the application after resetting the application?

Restart code like

 Intent mainIntent = new Intent(this, MainActivity.class);
                    int mPendingIntentId = 0x130;
                    PendingIntent mPendingIntent = PendingIntent.getActivity(this, mPendingIntentId, mainIntent, PendingIntent.FLAG_CANCEL_CURRENT);
                    AlarmManager mgr = (AlarmManager) getSystemService(Context.ALARM_SERVICE);
                    mgr.set(AlarmManager.RTC, System.currentTimeMillis() + 100, mPendingIntent);
                    System.exit(0);

I updated the PR, and did some tests, after a restart we can get the right behavior.

@lakshyagupta21
Copy link
Contributor

lakshyagupta21 commented Aug 19, 2019

@lakshyagupta21 How about restarting the application after resetting the application?

Restart code like

@huangyz0918 , Restarting the application is not the solution we've to modify the code in MainActivity, this could be a solution in the short term but it's not going to help us in the long term.

@huangyz0918
Copy link
Contributor Author

Restarting the application is not the solution we've to modify the code in MainActivity, this could be a solution in the short term but it's not going to help us in the long term.

I think a quick restart looks like exiting and starting MainActivity again, and can refresh the whole application, it's a good option, handling in MainActivity is not a good solution and I think we should not do that, that can only make the future refactoring harder.

@lakshyagupta21
Copy link
Contributor

I think a quick restart looks like exiting and starting MainActivity again, and can refresh the whole application, it's a good option, handling in MainActivity is not a good solution and I think we should not do that, that can only make the future refactoring harder.

I would still not advice you to write code for app restart, if this was a solution then it would be there in ODK-Collect right? There are some consequences that we should take care when we're thinking in the direction of restarting the app, for high-end device this could be fast but for low-end devices restarting the app may take some time because that would take some time to load resources, activity, creating the storage directory(in our case its not required but in future we may have some use case)
What if there is some other activity on top of MainActivity? Then how restarting is going to help, for now, this looks the easiest solution but not the perfect solution.

@lakshyagupta21
Copy link
Contributor

@huangyz0918 While testing this PR, I was able to see the crash in Android 4.4

08-25 18:24:46.806 2023-2023/org.odk.share E/AndroidRuntime: FATAL EXCEPTION: main
    Process: org.odk.share, PID: 2023
    java.lang.RuntimeException: Cannot create directory: /storage/sdcard0/share/metadata
        at org.odk.share.application.Share.createODKDirs(Share.java:68)
        at org.odk.share.views.ui.settings.SettingsActivity.resetData(SettingsActivity.java:291)
        at org.odk.share.views.ui.settings.SettingsActivity.startReset(SettingsActivity.java:255)
        at org.odk.share.views.ui.settings.SettingsActivity.lambda$resetApplication$4(SettingsActivity.java:238)
        at org.odk.share.views.ui.settings.-$$Lambda$SettingsActivity$2ZWiFMifxwXM4EoK8yD3UbokPPo.onClick(lambda)
        at androidx.appcompat.app.AlertController$ButtonHandler.handleMessage(AlertController.java:167)
        at android.os.Handler.dispatchMessage(Handler.java:110)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:5304)
        at java.lang.reflect.Method.invokeNative(Native Method)
        at java.lang.reflect.Method.invoke(Method.java:515)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:824)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:640)
        at dalvik.system.NativeStart.main(Native Method)

@huangyz0918
Copy link
Contributor Author

Cannot create directory: /storage/sdcard0/share/metadata

It seems we missed a / between sdcard and 0? @lakshyagupta21

@lakshyagupta21
Copy link
Contributor

@huangyz0918 Can you fix the above issue in the same PR.

@huangyz0918
Copy link
Contributor Author

huangyz0918 commented Oct 4, 2019

Can you fix the above issue in the same PR.

Yes will do.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an option in Settings to reset the application
2 participants