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

CHT External App Launcher #199

Merged
merged 13 commits into from
Sep 15, 2021
Merged

CHT External App Launcher #199

merged 13 commits into from
Sep 15, 2021
+1,489 −54

Conversation

latin-panda
Copy link
Contributor

@latin-panda latin-panda commented Jul 30, 2021 •
edited
Loading

Description

This PR:

  • Adds a way to launch any external android app from the CHT-Android.
    • Webapp triggers this feature and pass the information that an external app may need.
    • When the work in the external app is finished, the CHT-Android send the intent's response data (if any) to Webapp.
  • As first scope: it's going to support numbers, strings, bools, objects, arrays and images.

Ticket: medic/cht-core#6981
PR CHT-Docs
PR CHT-Core

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

@latin-panda latin-panda requested a review from jkuester July 30, 2021 10:26
@latin-panda
Copy link
Contributor Author

latin-panda commented Jul 30, 2021 •
edited
Loading

Hi @jkuester, I heard that you have experience with Android and Java, would you please review this draft?
I'd like to receive early feedback as I'm learning my way in Android 🙏
I still have concerns if this code is working in android 5 & 6, because of Java 8 features, so in the meantime I'm going figure out this piece with emulators 🙂
Thanks!

@latin-panda
Fixing image support for older android sdk
9bc4b2f
@latin-panda
Fixed Java 8 support in older version of android.
7f46ebd 
Disabled AndroidGradlePluginVersion lint rule as newer version of the lib requires Java 11 min.
@latin-panda
Unit tests for Utils new method
ab6358e
@latin-panda latin-panda marked this pull request as ready for review August 2, 2021 09:33
@latin-panda
Copy link
Contributor Author

Hi @jkuester, I think I've done all my pending items:

  • Fixed the image path support for older sdk
  • Fixed the inline errors for Java 8 code in older sdks (Android 5 & 6)
  • Disabled the gradle lint error because the next android.tool.gradle library is version 7, and this version requires JDK 11 👀 I recommend that it's fixed in a separate PR/Ticket so QA can target the testing.
  • Added more unit tests.

Please let me know any feedback 🙂
@garethbowen welcome to provide feedback too 🙂

I'll be working in the second piece of this feature which is the Enketo widget to process the form and connect with this Android feature.
Thank you!

jkuester
jkuester reviewed Aug 2, 2021
View reviewed changes
Copy link
Contributor

@jkuester jkuester left a comment

Choose a reason for hiding this comment

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

Overall these changes look great! I have added some minor suggestions around the structure/flow and some questions regarding some small parts of the logic.

I have not had a chance to look at the tests yet, but I wanted to at least get these comments in to start with.

@latin-panda latin-panda requested a review from jkuester August 4, 2021 10:12
@latin-panda
Copy link
Contributor Author

Hi @jkuester Thanks for the feedback and examples, it was good learning, can you please review again? Thanks!

jkuester
jkuester reviewed Aug 4, 2021
View reviewed changes
Copy link
Contributor

@jkuester jkuester left a comment

Choose a reason for hiding this comment

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

Great test cases!

@mrsarm
Copy link
Contributor

mrsarm commented Aug 4, 2021

@latin-panda I have created an issue for the linter issue here: #201 , working on that.

@latin-panda latin-panda requested a review from jkuester August 5, 2021 06:37
@latin-panda
Copy link
Contributor Author

@jkuester I've applied the feedback, please have a look :)

jkuester
jkuester approved these changes Aug 5, 2021
View reviewed changes
Copy link
Contributor

@jkuester jkuester left a comment

Choose a reason for hiding this comment

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

🚀

@latin-panda latin-panda mentioned this pull request Aug 12, 2021
Merged
5 tasks
@latin-panda latin-panda mentioned this pull request Aug 14, 2021
Merged
@latin-panda
Merge branch 'master' of https://github.com/medic/medic-android into …
a010066 
…6981-launching-external-apps

� Conflicts:
�	build.gradle
@latin-panda latin-panda mentioned this pull request Aug 16, 2021
Closed
@mrjones-plip mrjones-plip mentioned this pull request Aug 31, 2021
Closed
7 tasks
@mrsarm mrsarm added this to the 0.10.0 milestone Sep 10, 2021
@latin-panda latin-panda merged commit b7a7a8d into master Sep 15, 2021
@latin-panda latin-panda deleted the 6981-launching-external-apps branch September 15, 2021 15:18
@mrsarm mrsarm mentioned this pull request Sep 15, 2021
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garethbowen garethbowen

@jkuester jkuester

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@latin-panda @mrsarm @garethbowen @jkuester