AC-424:Do not require login location if no locations are configured. #391
Conversation
| @@ -353,7 +360,7 @@ private Snackbar createSnackbar(String message) { | |||
| @Override | |||
| public void setLocationErrorOccurred(boolean errorOccurred) { | |||
| this.loginValidatorWatcher.setLocationErrorOccurred(errorOccurred); | |||
| mLoginButton.setEnabled(!errorOccurred); | |||
There was a problem hiding this comment.
Why don't you want to disable the login button when errors occur?
There was a problem hiding this comment.
Will make necessary changes .
| @@ -116,17 +116,20 @@ public void onNothingSelected(AdapterView<?> adapterView) { | |||
|
|
|||
| private boolean isAllDataValid() { | |||
|
|
|||
| boolean result = validateNotEmpty(mUsername) && validateNotEmpty(mPassword) && validateNotEmpty(mLocation) && !urlChanged; | |||
There was a problem hiding this comment.
I would think that the validateNotEmpty() check stays, but in its body, we simply return valid of there are no login locations.
There was a problem hiding this comment.
@dkayiwa why would we even validateNotEmpty for location , because if location is not configured we don't care about location . Right ?
My point is why to have validateNotEmpty(mLocation) when it does nothing . ie. Returns true no matter what .
| mDropdownLocation.setVisibility(View.GONE); | ||
| mLoginButton.setEnabled(true); | ||
| }else { | ||
| mDropdownLocation.setVisibility(View.VISIBLE); |
There was a problem hiding this comment.
Do we need the else? I thought it was already disabled by default.
There was a problem hiding this comment.
@dkayiwa when the url / API is updated , the visibility of the mDropdownLocation should change accordingly .
|
The screenshot should be on the JIRA ticket instead of pull request. |
| @@ -353,7 +360,7 @@ private Snackbar createSnackbar(String message) { | |||
| @Override | |||
| public void setLocationErrorOccurred(boolean errorOccurred) { | |||
| this.loginValidatorWatcher.setLocationErrorOccurred(errorOccurred); | |||
| mLoginButton.setEnabled(!errorOccurred); | |||
| mDropdownLocation.setVisibility(View.GONE); | |||
There was a problem hiding this comment.
Isn't the above supposed to depend on the value of errorOccurred?
|
@anonymous-ME can you squash these commits into one as advised at? https://wiki.openmrs.org/display/docs/Pull+Request+Tips |
|
@dkayiwa squashed all commits into one. |
|
Found a bug in this PR in validating the location dropdown. Please refer to this PR for this issue. #395 |
JIRA TICKET NAME:
AC-424:Do not require login location if no locations are configured.
Summary:
Do not require login location if no locations are configured.