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.

Sign up for GitHub

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

Jump to bottom

feat(#334): add apk for cares malawi #334

Merged
merged 2 commits into from
Dec 7, 2023
Merged

feat(#334): add apk for cares malawi #334

merged 2 commits into from
Dec 7, 2023

Conversation

freefony
Copy link
Contributor

No description provided.

jkuester
jkuester approved these changes Oct 31, 2023
View reviewed changes
Copy link
Contributor

@jkuester jkuester left a comment •
edited
Loading

Choose a reason for hiding this comment

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

All the code changes look great!

Regarding the test failure, I am completely baffled. I believe it is failing here when it is waiting for the Gamma login screen to show up.

'has element with id: form' doesn't match: <?xml version="1.0" encoding="UTF-8"?><html xmlns:html="http://www.w3.org/1999/xhtml">

You can see from the error message that the actual page displayed is just the initial loading page for the webapp (with the loading spinner), but for some reason the loading never finishes and the login page is not displayed. Currently the test is waiting 7sec for the login screen to load. I tried increasing that to 2min but the test still fails.

I have not been able to reproduce this issue locally, however. The test runs fine for me, both on its own when I run it in Android Studio and when I run it via make test-ui.

I can reproduce the issue on top of master though. So, it is clearly not related to this PR at all...

If nobody has any brilliant ideas on what could be going wrong with this test, we may just have to log a new issue to get it fixed and skip it in the code for now. There is not much sense in it being a huge blocker for this PR, but I am fresh out of ideas for what could be causing the problem....

src/cares_malawi_kch/res/values/strings.xml Outdated
@@ -0,0 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<string name="app_name">Cares Malawi(KCH) App</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor, but should there be a space before the parenthesis? Not sure how this is usually rendered, but normally I would expect something like: Cares Malawi (KCH) App

src/cares_malawi_mpc/res/values/strings.xml Outdated
@@ -0,0 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<string name="app_name">Cares Malawi(MPC) App</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, not sure if we want a space before the parenthesis? Cares Malawi (MPC) App

@garethbowen
Copy link
Member

I agree with @jkuester - the failure probably has nothing to do with this PR. Let me know when it's ready and I can force merge this to get past the blocking required build.

@jkuester Would you mind raising an issue about the test failure? So far what I have is...

  • Slightly sus looking [DDMLIB]: An unexpected packet was received before the handshake. errors. But these are probably not causing this failure.
  • It's trying to log in to gamma-cht.dev (I think) which is in a bad state. Firstly the admin password in 1pass doesn't work so I can't log in. Secondly the /api/v1/monitoring URL 404s which implies it's a really old instance. It should be wiped and reset to 4.4 or deleted from the android app picker. A quick test would be to change the test to try logging in to "gamma.dev" instead (list item 2).
  • As Josh pointed out it's loading the spinner page. To clarify, it's the spinner page from the webapp not the login page. What should happen is that page loads, the JS downloads, and then the JS executes, which detects there's no session, and redirects to the login page. Any one of these steps could be failing. It's possible the instance is just bricked, or that we need to enable javascript, or we need to allow redirects, or we just need to give it more time.

@jkuester jkuester mentioned this pull request Nov 9, 2023
Closed
@jkuester
Copy link
Contributor

jkuester commented Nov 9, 2023

#337

I do not see how it could be a Gamma problem since it works fine locally. (And even after a 2min wait on the CI, it never made it to the login page...)

@kennsippell kennsippell changed the title add-cares-malawi feat(#334): add apk for cares malawi Dec 7, 2023
@kennsippell kennsippell merged commit 97b3576 into medic:master Dec 7, 2023
8 of 9 checks passed
@kennsippell kennsippell mentioned this pull request Dec 12, 2023
Merged
@sugat009 sugat009 mentioned this pull request Feb 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkuester jkuester jkuester approved these changes

@latin-panda latin-panda Awaiting requested review from latin-panda

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@freefony @garethbowen @jkuester @kennsippell