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

Allow users to set cht-core URLs with leading or trailing spaces (and trailing slash) on unbranded app #178

Closed
mrjones-plip opened this issue May 24, 2021 · 2 comments · Fixed by #233
Assignees
Labels
Type: Bug Fix something that isn't working as intended
Milestone

Comments

@mrjones-plip
Copy link
Collaborator

mrjones-plip commented May 24, 2021

Overview

When entering in a URL on the unbranded app and accidentally entering space before or after the URL, errors are caused.

We should likely trim leading and trailing spaces for a better user experience since spaces at those positions in the URL are not legal chars (but are mid URL!)

Tested on

  • Android 11
  • Pixel 3a
  • Unbranded 0.8.0 webview

Error 1

The first error is that we give different messages:

  • https://example.com - leading space and NO trailing slash, error is: "must be a valid URL"
  • https://example.com/ - leading space and YES trailing slash, error is: "URL is not a medic-webapp instance"
  • https://example.com - trailing space and NO trailing slash, error is: "unable to contact server"
  • https://example.com/ - trailing space and YES trailing slash, error is: "URL is not a medic-webapp instance"

steps to reproduce:

  1. download unbranded android app
  2. enter url with leading or trailing space
  3. click "save"

expected: launch app connecting to URL
actual: errors per above

Error 2

After entering a URL with a space at the beginning and no trailing slash, the launch screen now has a URL entry that when you click the app crashes

steps to reproduce:

  1. download unbranded android app
  2. enter url with leading space and no trailing slash: https://example.com
  3. click "save"
  4. see error c
  5. use back button to to back to launch screen
  6. click top entry for the URL you just entered on step two

expected: launch app connecting to URL (ideally), or at least show error URL is not a medic-webapp instance"
actual: app crashes b/c Attempt to invoke virtual method 'void android.widget.TextView.setError(java.lang.CharSequence)' on a null object reference at org.medicmobile.webapp.mobile.SettingsDIalogActivity.a(SettingsDialogActivity.java:8) (sorry - manually transcribing from screenshot - there may be typos here - see attached)

Screenshots

error1
error2
error3
error6
error5

@garethbowen garethbowen added this to the 0.9.0 milestone Jun 2, 2021
@mrsarm mrsarm added the Type: Bug Fix something that isn't working as intended label Aug 4, 2021
@garethbowen garethbowen added this to Ready for dev in Allies Workstream - DEPRECATED via automation Aug 4, 2021
@mrsarm mrsarm modified the milestones: 0.9.0, 0.10.0 Aug 4, 2021
@mrsarm mrsarm modified the milestones: 0.10.0, 0.11.0 Sep 15, 2021
@mrsarm mrsarm self-assigned this Nov 24, 2021
@mrsarm mrsarm moved this from Ready for dev to Dev in progress in Allies Workstream - DEPRECATED Nov 25, 2021
@mrsarm
Copy link
Contributor

mrsarm commented Dec 1, 2021

Ready for AT, alpha release v0.11.0-alpha-url-fix, branch 178-fix-url-errors, PR #233.

Ways to test the fix are well described above in the ticket description.

@mrsarm mrsarm moved this from Dev in progress to Ready for AT in Allies Workstream - DEPRECATED Dec 1, 2021
@ngaruko
Copy link
Contributor

ngaruko commented Dec 2, 2021

LGTM.
Tested the following scenarios:

  1. URL with trailing slash, NO leading space and NO trailing space: https://server.medic.org/

  2. URL with trailing slash, leading space and NO trailing space: ..... https://server.medic.org/

  3. URL with trailing slash, NO leading space and trailing space: https://server.medic.org/....

  4. URL with trailing slash, leading space and trailing space: ..... https://server.medic.org/....

  5. URL with NO trailing slash, NO leading space and NO trailing space: https://server.medic.org

  6. URL with NO trailing slash, leading space and NO trailing space: . https://server.medic.org

  7. URL with NO trailing slash, NO leading space and trailing space: https://server.medic.org.

  8. URL with NO trailing slash, leading space and trailing space: . https://server.medic.org.

Feel free to merge @mrsarm .

Allies Workstream - DEPRECATED automation moved this from Ready for AT to Done Dec 2, 2021
@jkuester jkuester changed the title Spaces in URL on unbranded app cause errors Allow users to set cht-core URLs with leading or trailing spaces (and trailing slash) on unbranded app Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Fix something that isn't working as intended
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants