Remove kujaku library #3285
Merged
Remove kujaku library #3285
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📝 WalkthroughWalkthroughThis change removes all Mapbox and Kujaku-based GIS and mapping features from the application. It deletes related activities ( Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant GeoPointMapActivity
User->>App: Initiates map-based action (e.g., select location)
App->>GeoPointMapActivity: Launches legacy map activity (always)
GeoPointMapActivity->>User: Presents map UI for interaction
User->>GeoPointMapActivity: Selects location/area
GeoPointMapActivity->>App: Returns selected data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~30-45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/src/org/commcare/location/CommCareFusedLocationController.kt (1)
21-22: Parameter type and null check inconsistency.The parameter type was changed to non-nullable
LocationResult, but the null checkresult ?: returnremains. If the API contract guarantees non-null results, the null check is redundant and should be removed. If null results are still possible, the parameter type should remain nullable.override fun onLocationResult(result: LocationResult) { - result ?: return mCurrentLocation = result.lastLocation mListener?.onLocationResult(mCurrentLocation!!) }Or revert to nullable if the API still allows null:
-override fun onLocationResult(result: LocationResult) { +override fun onLocationResult(result: LocationResult?) { result ?: return mCurrentLocation = result.lastLocation mListener?.onLocationResult(mCurrentLocation!!) }app/build.gradle (1)
168-168: Consider removing unused Mapbox API key references.Since the Mapbox library is being completely removed, the
MAPBOX_SDK_API_KEYreferences in the build configuration may no longer be needed. Consider removing these to fully clean up Mapbox-related configuration.- MAPBOX_SDK_API_KEY = project.properties['MAPBOX_SDK_API_KEY'] ?: ''- buildConfigField 'String', 'MAPBOX_SDK_API_KEY', "\"${project.ext.MAPBOX_SDK_API_KEY}\""Also applies to: 306-306
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
app/AndroidManifest.xml(0 hunks)app/assets/locales/android_translatable_strings.txt(0 hunks)app/build.gradle(2 hunks)app/res/color/button_colored_text.xml(0 hunks)app/res/color/button_green_background.xml(0 hunks)app/res/color/button_orange_background.xml(0 hunks)app/res/color/button_red_background.xml(0 hunks)app/res/layout/activity_drawing_boundary.xml(0 hunks)app/res/layout/activity_entity_kujaku_map_info_view.xml(0 hunks)app/res/layout/activity_entity_mapbox.xml(0 hunks)app/res/layout/activity_mapbox_location_picker.xml(0 hunks)app/res/values-es/strings.xml(0 hunks)app/res/values-fr/strings.xml(0 hunks)app/res/values-hi/strings.xml(0 hunks)app/res/values-pt/strings.xml(0 hunks)app/res/values-sw/strings.xml(0 hunks)app/res/values-ti/strings.xml(0 hunks)app/res/values/strings.xml(0 hunks)app/res/values/styles.xml(0 hunks)app/src/org/commcare/activities/EntitySelectActivity.java(1 hunks)app/src/org/commcare/gis/AreaCalculator.kt(0 hunks)app/src/org/commcare/gis/BaseMapboxActivity.kt(0 hunks)app/src/org/commcare/gis/DrawingBoundaryActivity.kt(0 hunks)app/src/org/commcare/gis/EntityMapUtils.kt(0 hunks)app/src/org/commcare/gis/EntityMapboxActivity.kt(0 hunks)app/src/org/commcare/gis/MapEntity.kt(0 hunks)app/src/org/commcare/gis/MapboxLocationPickerActivity.kt(0 hunks)app/src/org/commcare/gis/MapboxLocationPickerViewModel.kt(0 hunks)app/src/org/commcare/location/CommCareFusedLocationController.kt(1 hunks)app/src/org/commcare/preferences/HiddenPreferences.java(0 hunks)app/src/org/commcare/views/widgets/GeoPointWidget.java(1 hunks)
💤 Files with no reviewable changes (27)
- app/res/color/button_orange_background.xml
- app/res/color/button_colored_text.xml
- app/res/color/button_red_background.xml
- app/res/color/button_green_background.xml
- app/assets/locales/android_translatable_strings.txt
- app/res/layout/activity_entity_kujaku_map_info_view.xml
- app/res/layout/activity_entity_mapbox.xml
- app/res/values-es/strings.xml
- app/src/org/commcare/gis/MapEntity.kt
- app/src/org/commcare/preferences/HiddenPreferences.java
- app/res/layout/activity_mapbox_location_picker.xml
- app/src/org/commcare/gis/AreaCalculator.kt
- app/res/values/strings.xml
- app/res/values-sw/strings.xml
- app/src/org/commcare/gis/EntityMapboxActivity.kt
- app/res/values-fr/strings.xml
- app/res/layout/activity_drawing_boundary.xml
- app/src/org/commcare/gis/EntityMapUtils.kt
- app/res/values-hi/strings.xml
- app/res/values-ti/strings.xml
- app/res/values/styles.xml
- app/res/values-pt/strings.xml
- app/AndroidManifest.xml
- app/src/org/commcare/gis/DrawingBoundaryActivity.kt
- app/src/org/commcare/gis/MapboxLocationPickerActivity.kt
- app/src/org/commcare/gis/BaseMapboxActivity.kt
- app/src/org/commcare/gis/MapboxLocationPickerViewModel.kt
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: shubham1g5
PR: dimagi/commcare-android#2935
File: app/src/org/commcare/gis/EntityMapActivity.java:134-142
Timestamp: 2025-01-13T11:29:50.055Z
Learning: Google Maps SDK internally manages bitmap lifecycle for marker icons created through BitmapDescriptorFactory, so manual bitmap recycling is not needed in EntityMapActivity.java.
📚 Learning: google maps sdk internally manages bitmap lifecycle for marker icons created through bitmapdescripto...
Learnt from: shubham1g5
PR: dimagi/commcare-android#2935
File: app/src/org/commcare/gis/EntityMapActivity.java:134-142
Timestamp: 2025-01-13T11:29:50.055Z
Learning: Google Maps SDK internally manages bitmap lifecycle for marker icons created through BitmapDescriptorFactory, so manual bitmap recycling is not needed in EntityMapActivity.java.
Applied to files:
app/src/org/commcare/activities/EntitySelectActivity.javaapp/src/org/commcare/views/widgets/GeoPointWidget.java
📚 Learning: viewmodels should not store view or activity references as this can cause memory leaks. unlike fragm...
Learnt from: shubham1g5
PR: dimagi/commcare-android#3042
File: app/src/org/commcare/fragments/BreadcrumbBarViewModel.java:50-55
Timestamp: 2025-04-21T15:02:17.492Z
Learning: ViewModels should not store View or Activity references as this can cause memory leaks. Unlike Fragments with setRetainInstance(true), ViewModels don't have automatic view detachment mechanisms. When migrating from Fragments to ViewModels, view references should be replaced with data-only state in the ViewModel.
Applied to files:
app/src/org/commcare/activities/EntitySelectActivity.java
📚 Learning: in commcaresetupactivity.java, the call to installfragment.showconnecterrormessage() after fragment ...
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3121
File: app/src/org/commcare/activities/CommCareSetupActivity.java:360-364
Timestamp: 2025-05-22T14:28:35.959Z
Learning: In CommCareSetupActivity.java, the call to installFragment.showConnectErrorMessage() after fragment transactions is intentionally unguarded with null checks. This follows the app's design pattern where critical error paths prefer immediate crashes over silent failures, making potential issues immediately visible during development rather than hiding them with defensive programming.
Applied to files:
app/src/org/commcare/activities/EntitySelectActivity.javaapp/src/org/commcare/views/widgets/GeoPointWidget.java
📚 Learning: never instantiate android activity classes directly with 'new'. activities should only be created th...
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3037
File: app/src/org/commcare/connect/ConnectIDManager.java:233-243
Timestamp: 2025-04-22T15:48:29.346Z
Learning: Never instantiate Android Activity classes directly with 'new'. Activities should only be created through the Android framework using Intents.
Applied to files:
app/src/org/commcare/activities/EntitySelectActivity.javaapp/src/org/commcare/views/widgets/GeoPointWidget.java
📚 Learning: never instantiate android activity classes directly with 'new' as it bypasses the android component ...
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3037
File: app/src/org/commcare/connect/ConnectIDManager.java:233-243
Timestamp: 2025-04-22T15:48:29.346Z
Learning: Never instantiate Android Activity classes directly with 'new' as it bypasses the Android component lifecycle. Activities should only be created by the system through Intents. Non-static instance fields don't need manual resetting as they'll be initialized with their default values when a new activity instance is properly created.
Applied to files:
app/src/org/commcare/activities/EntitySelectActivity.java
📚 Learning: in selectinstallmodefragment.java, the showconnecterrormessage method intentionally omits null check...
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3121
File: app/src/org/commcare/fragments/SelectInstallModeFragment.java:201-205
Timestamp: 2025-05-22T14:26:41.341Z
Learning: In SelectInstallModeFragment.java, the showConnectErrorMessage method intentionally omits null checks because it's called at a specific point in the startup flow where UI is guaranteed to be loaded. It's designed to crash if activity or view is null to make potential issues immediately visible rather than hiding them with defensive programming.
Applied to files:
app/src/org/commcare/activities/EntitySelectActivity.java
📚 Learning: in connectunlockfragment.java, the user prefers to let getarguments() potentially throw nullpointere...
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3108
File: app/src/org/commcare/fragments/connect/ConnectUnlockFragment.java:62-64
Timestamp: 2025-06-04T19:17:21.213Z
Learning: In ConnectUnlockFragment.java, the user prefers to let getArguments() potentially throw NullPointerException rather than adding null checks, as the arguments are required for proper navigation flow and their absence indicates a programming error that should fail fast.
Applied to files:
app/src/org/commcare/activities/EntitySelectActivity.java
📚 Learning: in the commcare android codebase, use org.jetbrains.annotations.notnull for null-safety annotations....
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#2912
File: app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java:81-89
Timestamp: 2025-01-21T17:27:58.754Z
Learning: In the CommCare Android codebase, use org.jetbrains.annotations.NotNull for null-safety annotations. Empty strings are acceptable for unspecified values, but null values should be prevented using NotNull.
Applied to files:
app/src/org/commcare/activities/EntitySelectActivity.javaapp/src/org/commcare/location/CommCareFusedLocationController.kt
📚 Learning: the connect base url and api version in build.gradle are intentionally hardcoded as these values do ...
Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/build.gradle:294-295
Timestamp: 2025-01-27T15:18:25.903Z
Learning: The Connect base URL and API version in build.gradle are intentionally hardcoded as these values do not change with variants.
Applied to files:
app/build.gradle
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (3)
app/src/org/commcare/activities/EntitySelectActivity.java (1)
777-779: LGTM! Clean simplification removing Mapbox dependency.The conditional logic for selecting between Mapbox and non-Mapbox map activities has been properly removed, now always using
EntityMapActivity. This change aligns well with the overall Mapbox library removal effort.app/src/org/commcare/views/widgets/GeoPointWidget.java (1)
139-141: LGTM! Method simplified by removing Mapbox conditional logic.The
getMapActivityIntent()method has been appropriately simplified to always returnGeoPointMapActivity, removing the previous conditional logic that selected between Mapbox and non-Mapbox implementations. This change is consistent with the overall Mapbox library removal effort.app/build.gradle (1)
89-89: Google Play Services Location dependency is up-to-date (21.3.0) with no known security advisories
Verified that 21.3.0 is the latest stable version as of August 2025 and no documented vulnerabilities exist. No changes needed—continue monitoring Google’s release notes for future updates.
shubham1g5
approved these changes
Aug 8, 2025
Contributor
shubham1g5
left a comment
shubham1g5
left a comment
There was a problem hiding this comment.
thanks for the detailed description over there.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Product Description
Since the Kujaku library is no longer being actively maintained and the related feature have very low usage, we have decided to remove it from CommCare and retire all related features. This decision was also supported by issues observed during the Android 15 update, where accessing such features would cause CommCare to crash, it seems that Kujaku relies on native libraries that are not compatible with that version of Android.
With this removal, when users try to access features that rely on this feature they will see:
Notes from reviewers:
commcare-corethat I'm reluctant to remove as it can causes issues to existing apps:Tickets: https://dimagi.atlassian.net/browse/SAAS-18188, https://dimagi.atlassian.net/browse/SAAS-18042
Feature Flag
mapscc-use-mapbox-mapSafety Assurance
Safety story
Successfully tested locally.
QA Plan
This will undergo regression testing as always, but it will require the QA team to update the relevant test cases with the behavior described above.
Labels and Review