Fix:Failure of log-out button #790

AkshGautam · 2017-12-30T09:09:34Z

Log Out Button is working fine!

AkshGautam · 2017-12-30T18:43:15Z

@tarun0 Please review when you get time :)

tarun0 · 2017-12-31T15:58:01Z

Squash the commits into a single commit
Follow commit style guideline (find it on the wiki of the project, here at Github)
Don't add unnecessary blank lines
Add a GIF in the PR as a comment which demonstrates the changes you've added

tarun0 · 2017-12-31T17:04:07Z

mifosng-android/src/main/java/com/mifos/mifosxdroid/core/MifosBaseActivity.java

@@ -5,6 +5,7 @@

 package com.mifos.mifosxdroid.core;

+


What I meant by unnecessary blank lines were these - added by you and not those which Android Studio automatically inserts to distinguish imports from different packages. Please keep the blank lines between the imports as they were.

tarun0 · 2017-12-31T17:04:44Z

mifosng-android/src/main/java/com/mifos/mifosxdroid/passcode/PassCodeActivity.java

@@ -67,6 +65,7 @@ protected void onCreate(Bundle savedInstanceState) {
            //enabling passCodeListener only when user has already setup PassCode
            passCodeView.setPassCodeListener(this);
        }
+


Remove all the changes in this file except the finish() statement.

tarun0 · 2017-12-31T17:15:50Z

You may find a tutorial for Squashing the commits here: https://github.com/todotxt/todo.txt-android/wiki/Squash-All-Commits-Related-to-a-Single-Issue-into-a-Single-Commit

I'd recommend squashing the commits (even if there is just single commit) once the conversation has started on any PR because amending the commit will lead to deletion of the comments on the PR.

AkshGautam · 2017-12-31T18:12:44Z

@tarun0 i hope, it satisfies every requirement.I got stuck in some git error. I was not able to push my changes.If anything else is required,i would make the necessary changes.Thanks

AkshGautam · 2017-12-31T18:42:37Z

@tarun0 After exploring a lot with git rebase -i HEAD~X ,i hope i have finally squashed all commits into one. Don't know why i was not able to push though. I had to use git push -f. Thanks for your patience.

tarun0 · 2018-01-02T09:30:50Z

mifosng-android/src/main/java/com/mifos/mifosxdroid/passcode/PassCodeActivity.java

            } else {
                counter++;
                passCodeView.clearPasscodeField();
                Toaster.show(clRootview, R.string.incorrect_passcode);
            }
        }
+


...or this one?

I removed the blanked line as well. Would take care of it from now onwards.Sorry for the mistake

tarun0 · 2018-01-02T09:31:10Z

mifosng-android/src/main/java/com/mifos/mifosxdroid/passcode/PassCodeActivity.java

@@ -67,6 +67,7 @@ protected void onCreate(Bundle savedInstanceState) {
            //enabling passCodeListener only when user has already setup PassCode
            passCodeView.setPassCodeListener(this);
        }
+


Any specific purpose of this blank line?

tarun0 · 2018-01-02T10:10:42Z

mifosng-android/src/main/java/com/mifos/mifosxdroid/passcode/PassCodeActivity.java

            } else {
                counter++;
                passCodeView.clearPasscodeField();
                Toaster.show(clRootview, R.string.incorrect_passcode);
            }
        }
+


You haven't added the screenshot/GIF of the changes yet... Add it so that the reviewers (or other org members) don't have to build the app from your repo to observe the changes.

Done. I am sorry for carelessness.I will be cautious from next time.

AkshGautam · 2018-01-02T10:44:33Z

tarun0 · 2018-01-03T09:34:28Z

mifosng-android/src/main/java/com/mifos/mifosxdroid/core/MifosBaseActivity.java

+        builder.setMessage(R.string.logout_message)
+                .setPositiveButton(R.string.action_yes, new DialogInterface.OnClickListener() {
+                    public void onClick(DialogInterface dialog, int id) {
+                        PrefManager.clearToken();


Remove all the sharedpreferences here instead of using clearToken() method.

When one logs out, the application should clear all the saved 'SharedPreference values'. clearToken() will only save a blank passcode value and all the remaining sharedpreferences will remain intact. You can save the Instance Domain to make it convenient for the other users to login but it is already handled in LoginActivity.

@tarun0 I thought of clearing all the shared preferences at first but then I realized that if a user logs out and the logs in again his identity is lost and he will be a new user to the application making him to set the 4-digit passcode on subsequent logins.I will clear all the preferences for now and would start working on the problem of preventing the reset of password. I have already created an issue for that in mifos mobile as well.

if a user logs out and the logs in again his identity is lost and he will be a new user to the application making him to set the 4-digit passcode on subsequent logins
Isn't it exactly how it should be? Why do you think passcode should be retained after logout?

@tarun0 Let's take an example of Paytm,The user creates a 4 digit PIN which is asked before any operation involving banking.The 4 digit passcode can be used instead of a longer PIN to ensure authenticity. Once the user logs-out the application(does not mean he wont use the app again,so why need to reset the password ) the password is still saved since the user is expected to use the app again.That's my opinion.If you feel this is irrelavent i would amend the code.

Your point is not irrelevant or wrong. However, you are mistaking the passcode used in the Mifos App with the PIN used in banking apps.

While the PIN is an added security measure in the baking apps, the passcode in Mifos is a tad inclined towards convenience as compared to security. It provides a way for the user to log into the app with a 4 digit passcode saving him from entering username and password every time he opens the app. Our intention to add such a feature is that unauthorized user (who has physical access to the phone) shouldn't use be allowed to use the app when the authenticated user has already exited it (in the future versions, it's supposed to evolve into a feature that would ask for passcode every time before the user performs any writable action, e.g. apply for a loan).

The PIN used in banking apps is stored on the server side to verify the authenticity whereas, in our app, the Passcode is saved in SharedPreference (hashed form) only and the verification is done locally, without any kind of intervention from the server.

Still, there's some flaw in your proposition. If the user logs in from device 1 and saves the passcode and then logs out, assuming that the passcode still refers that particular user, what will happen if he logs in from device 2? How will device 1 get information that a new passcode has been generated on some other device? The solution to this intervention of the server.

Your proposition was something in the pipeline but it might affect offline functionality and would add to more network calls. As I'm not the right person to confirm or reject any possibility whether this feature will be added, I can only tell you that it's not present currently.

I hope it provides some clarification. :)

@tarun0 Alright,I understood the functionality of the passcode now. Moreover retaining the passcode would be consistent only if the application is hosted on a universally available database on cloud. I have made the changes by clearing the preferences on logout.Thanks

tarun0 · 2018-01-03T09:43:07Z

mifosng-android/src/main/java/com/mifos/mifosxdroid/core/MifosBaseActivity.java

+        builder.setMessage(R.string.logout_message)
+                .setPositiveButton(R.string.action_yes, new DialogInterface.OnClickListener() {
+                    public void onClick(DialogInterface dialog, int id) {
+                        PrefManager.clearToken();


Also, could you double check whether this issue of white text is present on other devices as well? My device is Motorola G5S Plus, Stock Nougat (7.1.1) - API 25.

@tarun0 I would check this in some other device as well and post the update.

@tarun0 I have checked the application on Android 6.0.1,API 23 and on Android 5.0.2 API 21 and I did not find the white text problem.

AkshGautam · 2018-01-08T08:56:58Z

@tarun0 Could you please review and suggest any improvements if any so it could be merged.Thanks

therajanmaurya · 2018-01-10T11:22:02Z

@AkshGautam Good work, Can you please make little changes, We already have Material Dialog Please make changes in PR and use this https://github.com/openMF/android-client/blob/master/mifosng-android/src/main/java/com/mifos/mifosxdroid/core/MaterialDialog.java

AkshGautam · 2018-01-10T11:29:23Z

@therajanmaurya sure! I would make the changes.

AkshGautam · 2018-01-13T06:10:08Z

@therajanmaurya @tarun0 done the changes by adding material dialog as well.

AkshGautam · 2018-01-17T14:43:49Z

@therajanmaurya Issue Number #787

Contributor made requested changes, No need to wait.

tarun0 reviewed Dec 31, 2017

View reviewed changes

AkshGautam force-pushed the master branch from 14bbfb1 to 047bc84 Compare December 31, 2017 18:06

AkshGautam force-pushed the master branch 2 times, most recently from f54a044 to 0e8adcf Compare December 31, 2017 18:40

AkshGautam force-pushed the master branch 2 times, most recently from 7be3239 to f9c0dae Compare January 2, 2018 05:00

tarun0 requested changes Jan 2, 2018

View reviewed changes

AkshGautam force-pushed the master branch from f9c0dae to ef17ba8 Compare January 2, 2018 10:40

tarun0 previously requested changes Jan 3, 2018

View reviewed changes

AkshGautam force-pushed the master branch 2 times, most recently from b40f956 to aa472c5 Compare January 3, 2018 13:42

AkshGautam changed the title ~~log out button working~~ Fix:Failure of log-out button Jan 4, 2018

AkshGautam force-pushed the master branch 4 times, most recently from 87b815b to 3bf781d Compare January 13, 2018 05:53

Fix: Failure of log-out button

581d5ba

AkshGautam force-pushed the master branch from 3bf781d to 581d5ba Compare January 13, 2018 06:01

AkshGautam mentioned this pull request Jan 17, 2018

Log out button not working #787

Closed

therajanmaurya merged commit 42c82a6 into openMF:master Jan 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix:Failure of log-out button #790

Fix:Failure of log-out button #790

AkshGautam commented Dec 30, 2017 •

edited

Loading

AkshGautam commented Dec 30, 2017

tarun0 commented Dec 31, 2017

tarun0 Dec 31, 2017

AkshGautam Dec 31, 2017

tarun0 Dec 31, 2017

AkshGautam Dec 31, 2017

tarun0 commented Dec 31, 2017

AkshGautam commented Dec 31, 2017 •

edited

Loading

AkshGautam commented Dec 31, 2017

tarun0 Jan 2, 2018

AkshGautam Jan 2, 2018

tarun0 Jan 2, 2018

tarun0 Jan 2, 2018

AkshGautam Jan 2, 2018

AkshGautam commented Jan 2, 2018

tarun0 Jan 3, 2018

AkshGautam Jan 3, 2018

tarun0 Jan 3, 2018 •

edited

Loading

AkshGautam Jan 3, 2018

tarun0 Jan 3, 2018

AkshGautam Jan 3, 2018

tarun0 Jan 3, 2018

AkshGautam Jan 3, 2018

AkshGautam Jan 4, 2018

AkshGautam commented Jan 8, 2018 •

edited

Loading

therajanmaurya commented Jan 10, 2018

AkshGautam commented Jan 10, 2018

AkshGautam commented Jan 13, 2018

AkshGautam commented Jan 17, 2018

Fix:Failure of log-out button #790

Fix:Failure of log-out button #790

Conversation

AkshGautam commented Dec 30, 2017 • edited Loading

AkshGautam commented Dec 30, 2017

tarun0 commented Dec 31, 2017

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

tarun0 commented Dec 31, 2017

AkshGautam commented Dec 31, 2017 • edited Loading

AkshGautam commented Dec 31, 2017

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

AkshGautam commented Jan 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarun0 Jan 3, 2018 • edited Loading

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

AkshGautam commented Jan 8, 2018 • edited Loading

therajanmaurya commented Jan 10, 2018

AkshGautam commented Jan 10, 2018

AkshGautam commented Jan 13, 2018

AkshGautam commented Jan 17, 2018

AkshGautam commented Dec 30, 2017 •

edited

Loading

AkshGautam commented Dec 31, 2017 •

edited

Loading

tarun0 Jan 3, 2018 •

edited

Loading

AkshGautam commented Jan 8, 2018 •

edited

Loading