feat: added shouldUpdate and checkForUpdate#307
Open
yuvrajjsingh0 wants to merge 2 commits into
Open
Conversation
yuvrajjsingh0
changed the title
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant RN as React Native
participant Module as Platform Module<br/>(Airborne/AirborneModule)
participant Core as Airborne Core
participant Net as Network Layer<br/>(HTTP)
participant Server as Release Config<br/>Server
RN->>Module: checkForUpdate(nameSpace)
Module->>Core: checkForUpdate()
Core->>Core: Read local release config
Core->>Core: Extract version & package info
Core->>Net: GET releaseConfigUrl<br/>(with version headers)
Net->>Server: HTTP GET request
Server-->>Net: HTTP 200 + JSON response
Net-->>Core: Response body
Core->>Core: Parse remote config<br/>Compare versions
Core-->>Module: JSON (updateAvailable,<br/>versions)
Module-->>RN: Promise resolved<br/>with result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
yuvrajjsingh0
force-pushed
the
airborne-shouldUpdate-and-check-for-update
branch
from
0a4f43d to
0342a7a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
airborne-react-native/example/android/app/src/main/java/airborne/example/MainApplication.kt (1)
57-62: Avoid tenant-specific hardcoding in the example bootstrap.Using concrete org/app values here makes the example less reusable and harder to run across environments. Also, the trailing
falseis not self-descriptive. Consider config-driven constants and a named argument for clarity.♻️ Proposed refactor
- "https://airborne.juspay.in/release/yuvraj_org12/my_app23", + BuildConfig.AIRBORNE_RELEASE_URL, @@ - return "my_app23" // Your app id + return BuildConfig.AIRBORNE_NAMESPACE @@ - }, false) + }, shouldUpdate = false)Also applies to: 85-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne-react-native/example/android/app/src/main/java/airborne/example/MainApplication.kt` around lines 57 - 62, Replace the hardcoded tenant/org/app values and the opaque trailing boolean with config-driven constants and a named argument: extract the URL string and namespace returned by getNamespace() from a configuration constant (e.g., AIRBORNE_BASE_URL and AIRBORNE_NAMESPACE) instead of literal "https://airborne.juspay.in/release/yuvraj_org12/my_app23" and "my_app23", and when constructing the AirborneInterface or its caller pass the boolean as a named parameter (e.g., enableFeature = false) to make its purpose clear; update references in the anonymous object overriding getNamespace() and any other occurrences (including the similar occurrence around line 85) to use these constants.airborne-react-native/android/src/main/java/in/juspay/airborneplugin/Airborne.kt (2)
120-122: Redundant header:x-release-config-versionandx-config-versionhave the same value.Both headers are set to
config.version. If this is intentional for backward compatibility with the server, consider adding a comment. Otherwise, one can be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne-react-native/android/src/main/java/in/juspay/airborneplugin/Airborne.kt` around lines 120 - 122, The code sets both headers["x-release-config-version"] and headers["x-config-version"] to the same value (localJson.optJSONObject("config")?.optString("version", "") ?: ""), which is redundant; either remove one of the assignments or, if both are required for backward compatibility, add a clarifying comment next to the duplicate assignment. Locate the three header assignments in Airborne.kt (the lines setting headers["x-release-config-version"], headers["x-package-version"], and headers["x-config-version"]) and either delete the redundant x-release-config-version (or x-config-version) assignment or add a comment like "kept for backward compatibility" above the duplicate to justify keeping both.
131-158: Network call is synchronous — ensure callers invoke from a background thread.
doGet()is a blocking call. The React Native bridge should handle this on a background thread, but consider documenting this requirement in the method's KDoc, or verify thatAirborneModuleImpldispatches this off the main thread.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne-react-native/android/src/main/java/in/juspay/airborneplugin/Airborne.kt` around lines 131 - 158, The network call in checkForUpdate uses the blocking netUtils.doGet; update the code so callers don't run this on the main thread: add KDoc to the checkForUpdate function documenting that doGet is blocking and must be invoked from a background thread, and/or modify AirborneModuleImpl to dispatch the checkForUpdate invocation off the main thread (e.g., using an IO dispatcher, Executor, or another background thread mechanism) so doGet is never executed on the UI thread; reference checkForUpdate, netUtils.doGet and AirborneModuleImpl when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@airborne-react-native/android/src/main/java/in/juspay/airborneplugin/Airborne.kt`:
- Around line 142-146: Android is currently reading root-level "version" while
iOS reads "config.version", and localRc is reparsed; update Android's comparison
to match iOS by reusing the already-parsed local JSON object (instead of
reparsing localRc) and extract versions from the nested "config" object.
Specifically, change how localVersion and localPkgVersion are obtained to use
localJson.optJSONObject("config")?.optString("version", "") and
localJson.optJSONObject("config")?.optJSONObject("package")?.optString("version",
"") (reusing the parsed local JSON stored earlier), and likewise derive
remoteVersion/remotePkgVersion from remoteJson.optJSONObject("config"), then set
updateAvailable by comparing these config-level versions.
In
`@airborne-react-native/android/src/main/java/in/juspay/airborneplugin/AirborneModuleImpl.kt`:
- Around line 72-75: In checkForUpdate, avoid resolving null when the namespace
key is missing: inspect Airborne.airborneObjectMap[namespace] first and if it is
null/undefined call promise.reject with a clear error code/message (e.g.,
"UNKNOWN_NAMESPACE", "Namespace not found: " + namespace); otherwise call the
object's checkForUpdate(), capture its result and promise.resolve(result) as
before (also ensure you catch and promise.reject any thrown exceptions from
checkForUpdate).
In `@airborne-react-native/ios/Airborne.m`:
- Around line 124-127: The current error branch in Airborne.m builds JSON with
stringWithFormat using error.localizedDescription which can break when the
message contains quotes/backslashes; update the code in the method that calls
completion(...) so you create an NSDictionary (e.g., @{@"updateAvailable": `@NO`,
@"error": error.localizedDescription ?: @""}) and serialize it with
NSJSONSerialization to NSData/NSString, handle serialization errors (fallback to
a safe JSON string) and pass that serialized JSON string to completion instead
of using stringWithFormat.
---
Nitpick comments:
In
`@airborne-react-native/android/src/main/java/in/juspay/airborneplugin/Airborne.kt`:
- Around line 120-122: The code sets both headers["x-release-config-version"]
and headers["x-config-version"] to the same value
(localJson.optJSONObject("config")?.optString("version", "") ?: ""), which is
redundant; either remove one of the assignments or, if both are required for
backward compatibility, add a clarifying comment next to the duplicate
assignment. Locate the three header assignments in Airborne.kt (the lines
setting headers["x-release-config-version"], headers["x-package-version"], and
headers["x-config-version"]) and either delete the redundant
x-release-config-version (or x-config-version) assignment or add a comment like
"kept for backward compatibility" above the duplicate to justify keeping both.
- Around line 131-158: The network call in checkForUpdate uses the blocking
netUtils.doGet; update the code so callers don't run this on the main thread:
add KDoc to the checkForUpdate function documenting that doGet is blocking and
must be invoked from a background thread, and/or modify AirborneModuleImpl to
dispatch the checkForUpdate invocation off the main thread (e.g., using an IO
dispatcher, Executor, or another background thread mechanism) so doGet is never
executed on the UI thread; reference checkForUpdate, netUtils.doGet and
AirborneModuleImpl when making the change.
In
`@airborne-react-native/example/android/app/src/main/java/airborne/example/MainApplication.kt`:
- Around line 57-62: Replace the hardcoded tenant/org/app values and the opaque
trailing boolean with config-driven constants and a named argument: extract the
URL string and namespace returned by getNamespace() from a configuration
constant (e.g., AIRBORNE_BASE_URL and AIRBORNE_NAMESPACE) instead of literal
"https://airborne.juspay.in/release/yuvraj_org12/my_app23" and "my_app23", and
when constructing the AirborneInterface or its caller pass the boolean as a
named parameter (e.g., enableFeature = false) to make its purpose clear; update
references in the anonymous object overriding getNamespace() and any other
occurrences (including the similar occurrence around line 85) to use these
constants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ca632cc-233a-4e99-a627-7e9a6fa2c884
📒 Files selected for processing (10)
airborne-react-native/android/src/main/java/in/juspay/airborneplugin/Airborne.ktairborne-react-native/android/src/main/java/in/juspay/airborneplugin/AirborneModule.ktairborne-react-native/android/src/main/java/in/juspay/airborneplugin/AirborneModuleImpl.ktairborne-react-native/android/src/main/java/in/juspay/airborneplugin/AirborneTurboModule.ktairborne-react-native/example/android/app/src/main/java/airborne/example/MainApplication.ktairborne-react-native/ios/Airborne.hairborne-react-native/ios/Airborne.mairborne-react-native/ios/AirborneReact.mmairborne-react-native/src/NativeAirborne.tsairborne-react-native/src/index.tsx
yuvrajjsingh0
force-pushed
the
airborne-shouldUpdate-and-check-for-update
branch
2 times, most recently
from
b48ea37 to
4d85de9
Compare
Yaswanth0606
requested changes
Apr 23, 2026
| self.airborne = [[AirborneServices alloc] initWithReleaseConfigURL:releaseConfigURL delegate:delegate ?: self]; | ||
| self.releaseConfigURL = releaseConfigURL; | ||
| self.delegate = delegate; | ||
| self.shouldUpdateEnabled = shouldUpdate; |
Contributor
There was a problem hiding this comment.
@yuvrajjsingh0 This doesn't have to be stored right
| } | ||
|
|
||
| // Get local manifest for version comparison | ||
| NSString *localRcJson = [self.airborne getReleaseConfig]; |
Contributor
There was a problem hiding this comment.
We can parse the json and fetch the values right.
Comment on lines
+100
to
+101
|
|
||
| NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:url]; |
Contributor
There was a problem hiding this comment.
Can we avoid having this code here. Can we reuse airborne's code to fetch release config
| } | ||
|
|
||
| return try { | ||
| val resp = netUtils.doGet(releaseConfigUrl, headers, null, null, null) |
Contributor
There was a problem hiding this comment.
Same here as well, can we re use airborne's code to fetch release config.
yuvrajjsingh0
force-pushed
the
airborne-shouldUpdate-and-check-for-update
branch
from
4d85de9 to
bd5500b
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary by CodeRabbit
Release Notes
checkForUpdate()method to check for available app updates, returning version comparison data and update availability status across Android and iOS platforms.