-
Notifications
You must be signed in to change notification settings - Fork 73
Fix debug mode detection and update eCHIS app configuration #3828
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,3 +23,6 @@ keystore.properties | |
| gradle.properties | ||
|
|
||
| /sentry.properties | ||
| .kotlin/ | ||
| quest/echis/ | ||
| quest/echisSupervisor/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -439,6 +439,13 @@ constructor( | |
| */ | ||
| @Throws(UnknownHostException::class, HttpException::class) | ||
| suspend fun fetchNonWorkflowConfigResources() { | ||
| // Skip remote fetch when using /debug mode - resources are loaded from assets | ||
| // Check directly for /debug suffix in app ID, regardless of build variant | ||
| val appId = sharedPreferencesHelper.retrieveApplicationId()?.trim() | ||
| if (appId?.endsWith(DEBUG_SUFFIX, ignoreCase = true) == true) { | ||
| Timber.d("Skipping remote config fetch - app ID '$appId' has /debug suffix, using local assets") | ||
| return | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we make this optional/configurable?
Comment on lines
+444
to
+447
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This functionality is unnecessary.
|
||
| } | ||
| Timber.d("Triggered fetching application configurations remotely") | ||
| configCacheMap.clear() | ||
| sharedPreferencesHelper.read(SharedPreferenceKey.APP_ID.name, null)?.let { appId -> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,7 +56,7 @@ kotlinx-coroutines = "1.9.0" | |
| kotlinx-serialization-json = "1.6.3" | ||
| kt3k-coveralls-ver="2.12.0" | ||
| ktlint = "0.50.0" | ||
| kujaku-library = "0.10.8-SNAPSHOT" | ||
| kujaku-library = "0.9.0" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how big are the differences between 0.10.8 with 0.9.0?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure but 0.10.8 is not working.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like a downgrade. Are we able to trace where 0.10.8 was published from? We could republish an update from that |
||
| kujaku-mapbox-sdk-turf = "7.2.0" | ||
| leakcanary-android = "2.10" | ||
| lifecycle= "2.8.7" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <resources> | ||
| <string name="fhir_core_app" translatable="false">MoH eCHIS</string> | ||
| </resources> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,7 +98,9 @@ constructor( | |
| val appId = appId.value?.trim() | ||
| if (!appId.isNullOrEmpty()) { | ||
| when { | ||
| sharedPreferencesHelper.hasDebugSuffix() -> loadConfigurations(context) | ||
| // Check directly for /debug suffix, regardless of build variant | ||
| appId.endsWith(ConfigurationRegistry.DEBUG_SUFFIX, ignoreCase = true) -> | ||
| loadConfigurations(context) | ||
|
Comment on lines
+101
to
+103
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we make this optional? |
||
| else -> fetchRemoteConfigurations(appId, context) | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have you tested publishing some artifacts and make sure this url works?