refactor: remove Supabase integration#700
Merged
utkarshdalal merged 1 commit intoutkarshdalal:masterfrom Mar 5, 2026
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughRemoved Supabase integration and CI credential injections; added a new worker-style REST API layer (GameNativeApi, GameRunApi, SupportersApi) and refactored feedback/supporters utilities to use the API instead of Supabase. Build and app initialization related Supabase wiring and dependencies were removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as App UI / Utils
participant GameNativeApi as GameNativeApi
participant PlayIntegrity as Play Integrity
participant OkHttp as OkHttpClient
participant Server as Worker API Server
Client->>GameNativeApi: buildPostRequest(url, body)
GameNativeApi->>PlayIntegrity: request token
PlayIntegrity-->>GameNativeApi: integrity token
GameNativeApi->>Client: Request (with headers)
Client->>GameNativeApi: executeRequest(request, parser)
GameNativeApi->>OkHttp: execute(request)
OkHttp->>Server: HTTP POST/GET
Server-->>OkHttp: Response
OkHttp-->>GameNativeApi: Response body
alt 2xx Success
GameNativeApi->>GameNativeApi: parse body
GameNativeApi-->>Client: ApiResult.Success(data)
else HTTP Error
GameNativeApi->>GameNativeApi: parse error details
GameNativeApi-->>Client: ApiResult.HttpError(code,message)
else Network/Error
GameNativeApi-->>Client: ApiResult.NetworkError(exception)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/app/gamenative/ui/component/dialog/SupportersDialog.kt (1)
50-62:⚠️ Potential issue | 🟠 MajorError handling is ineffective; API failures will silently show "no supporters" instead of error state.
Looking at
SupportersUtils.fetchKofiSupporters()in the relevant snippet, it catchesApiResult.HttpErrorandApiResult.NetworkErrorinternally, returningemptyList()rather than throwing. The try-catch here will never catch API failures, sohasErrorremainsfalseand users see the "no supporters" message instead of the error UI.Consider returning
ApiResultdirectly from the fetch and handling it here, or returning a result wrapper that distinguishes between "no data" and "error":Suggested approach: Handle ApiResult directly
+import app.gamenative.api.ApiResult +import app.gamenative.api.SupportersApi LaunchedEffect(Unit) { isLoading = true hasError = false - try { - val data = withContext(Dispatchers.IO) { - fetchKofiSupporters() - } - supporters = data - } catch (e: Exception) { - hasError = true + val result = withContext(Dispatchers.IO) { + SupportersApi.fetch() + } + when (result) { + is ApiResult.Success -> { + supporters = result.data.map { + KofiSupporter(name = it.name, oneOff = it.oneOff, total = it.total) + } + } + is ApiResult.HttpError, is ApiResult.NetworkError -> { + hasError = true + } } isLoading = false }Alternatively, modify
fetchKofiSupporters()to return a wrapper or nullable result that the caller can distinguish.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/component/dialog/SupportersDialog.kt` around lines 50 - 62, The current LaunchedEffect in SupportersDialog.kt calls SupportersUtils.fetchKofiSupporters() which swallows ApiResult errors and returns emptyList(), so the try/catch never triggers and hasError stays false; change the contract so fetchKofiSupporters() returns an ApiResult (or a sealed Result wrapper) instead of an empty list, then update the LaunchedEffect to call the new fetch (or fetchKofiSupportersResult()) and branch on the result setting supporters on success, hasError = true on ApiResult.HttpError/NetworkError, and isLoading = false in finally; update any callers or rename symbols accordingly so SupportersDialog.kt can distinguish "no supporters" vs "error".
🧹 Nitpick comments (2)
app/src/main/java/app/gamenative/api/GameNativeApi.kt (2)
19-24: Consider adding a write timeout.The client configures connect and read timeouts but no write timeout. For POST requests with larger payloads, this could cause indefinite blocking during upload.
Suggested improvement
val httpClient: OkHttpClient by lazy { OkHttpClient.Builder() .connectTimeout(10, TimeUnit.SECONDS) .readTimeout(10, TimeUnit.SECONDS) + .writeTimeout(10, TimeUnit.SECONDS) .build() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/api/GameNativeApi.kt` around lines 19 - 24, Add a write timeout to the OkHttp client configuration to avoid uploads blocking indefinitely: in the httpClient lazy block where OkHttpClient.Builder() is used (currently calling connectTimeout and readTimeout), add a writeTimeout call (e.g., .writeTimeout(10, TimeUnit.SECONDS) or an appropriate value) before .build() so POSTs and large payload uploads have an enforced timeout.
52-69: Redundant Content-Type header.Line 62 manually sets
Content-Type: application/json, but this is already handled bytoRequestBody(mediaType)on line 55. The header is redundant but harmless.Suggested cleanup
val builder = Request.Builder() .url(url) .post(requestBody) - .header("Content-Type", "application/json") if (integrityToken != null) { builder.header("X-Integrity-Token", integrityToken) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/api/GameNativeApi.kt` around lines 52 - 69, In buildPostRequest suspend function remove the manual .header("Content-Type", "application/json") call on the Request.Builder because requestBody created via bodyString.toRequestBody(mediaType) already sets the Content-Type; update the Request.Builder chain to rely on the requestBody and keep the integrity token header logic as-is (i.e., remove the redundant header call referencing "Content-Type" in the builder).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/api/GameNativeApi.kt`:
- Around line 42-49: The parser lambda's exceptions are being lumped into
ApiResult.NetworkError; add a dedicated parse error case and catch
parser-related exceptions separately: introduce an
ApiResult.ParseError(Throwable) variant (or similar) and, in GameNativeApi's
try/catch around ApiResult.Success(parser(body)), add a catch specifically for
parsing exceptions (e.g., com.squareup.moshi.JsonDataException /
JsonEncodingException / com.google.gson.JsonParseException /
org.json.JSONException or a general JsonParseException) that returns
ApiResult.ParseError(e), keep the existing catch(IOException) =>
ApiResult.NetworkError(e) and a final generic catch for other unexpected
exceptions.
---
Outside diff comments:
In `@app/src/main/java/app/gamenative/ui/component/dialog/SupportersDialog.kt`:
- Around line 50-62: The current LaunchedEffect in SupportersDialog.kt calls
SupportersUtils.fetchKofiSupporters() which swallows ApiResult errors and
returns emptyList(), so the try/catch never triggers and hasError stays false;
change the contract so fetchKofiSupporters() returns an ApiResult (or a sealed
Result wrapper) instead of an empty list, then update the LaunchedEffect to call
the new fetch (or fetchKofiSupportersResult()) and branch on the result setting
supporters on success, hasError = true on ApiResult.HttpError/NetworkError, and
isLoading = false in finally; update any callers or rename symbols accordingly
so SupportersDialog.kt can distinguish "no supporters" vs "error".
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/api/GameNativeApi.kt`:
- Around line 19-24: Add a write timeout to the OkHttp client configuration to
avoid uploads blocking indefinitely: in the httpClient lazy block where
OkHttpClient.Builder() is used (currently calling connectTimeout and
readTimeout), add a writeTimeout call (e.g., .writeTimeout(10, TimeUnit.SECONDS)
or an appropriate value) before .build() so POSTs and large payload uploads have
an enforced timeout.
- Around line 52-69: In buildPostRequest suspend function remove the manual
.header("Content-Type", "application/json") call on the Request.Builder because
requestBody created via bodyString.toRequestBody(mediaType) already sets the
Content-Type; update the Request.Builder chain to rely on the requestBody and
keep the integrity token header logic as-is (i.e., remove the redundant header
call referencing "Content-Type" in the builder).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/app-release-signed.yml.github/workflows/pluvia-pr-check.yml.github/workflows/tagged-release.ymlapp/build.gradle.ktsapp/src/main/java/app/gamenative/PluviaApp.ktapp/src/main/java/app/gamenative/api/ApiResult.ktapp/src/main/java/app/gamenative/api/GameNativeApi.ktapp/src/main/java/app/gamenative/api/GameRunApi.ktapp/src/main/java/app/gamenative/api/SupportersApi.ktapp/src/main/java/app/gamenative/ui/PluviaMain.ktapp/src/main/java/app/gamenative/ui/component/dialog/SupportersDialog.ktapp/src/main/java/app/gamenative/utils/GameFeedbackUtils.ktapp/src/main/java/app/gamenative/utils/SupportersUtils.kt
💤 Files with no reviewable changes (5)
- .github/workflows/app-release-signed.yml
- app/src/main/java/app/gamenative/PluviaApp.kt
- .github/workflows/tagged-release.yml
- app/build.gradle.kts
- .github/workflows/pluvia-pr-check.yml
Comment on lines
+42
to
+49
| ApiResult.Success(parser(body)) | ||
| } catch (e: IOException) { | ||
| Timber.tag("GameNativeApi").e(e, "Network error: ${e.message}") | ||
| ApiResult.NetworkError(e) | ||
| } catch (e: Exception) { | ||
| Timber.tag("GameNativeApi").e(e, "Unexpected error: ${e.message}") | ||
| ApiResult.NetworkError(e) | ||
| } |
Contributor
There was a problem hiding this comment.
Parser exceptions are incorrectly classified as NetworkError.
If the parser lambda throws an exception (e.g., JSON parsing failure on a successful response), it gets caught and wrapped as NetworkError, which is semantically misleading. Consider adding a dedicated error type for parsing failures or catching parser exceptions separately.
Suggested fix
inline fun <T> executeRequest(request: Request, parser: (String) -> T): ApiResult<T> {
return try {
val response = httpClient.newCall(request).execute()
val body = response.body?.string() ?: ""
if (!response.isSuccessful) {
val message = try {
val json = JSONObject(body)
json.optJSONObject("error")?.optString("message") ?: body
} catch (_: Exception) {
body
}
Timber.tag("GameNativeApi").w("HTTP ${response.code}: $message")
return ApiResult.HttpError(response.code, message)
}
- ApiResult.Success(parser(body))
+ try {
+ ApiResult.Success(parser(body))
+ } catch (e: Exception) {
+ Timber.tag("GameNativeApi").e(e, "Failed to parse response: ${e.message}")
+ ApiResult.NetworkError(e) // Or introduce ApiResult.ParseError
+ }
} catch (e: IOException) {
Timber.tag("GameNativeApi").e(e, "Network error: ${e.message}")
ApiResult.NetworkError(e)
} catch (e: Exception) {
Timber.tag("GameNativeApi").e(e, "Unexpected error: ${e.message}")
ApiResult.NetworkError(e)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ApiResult.Success(parser(body)) | |
| } catch (e: IOException) { | |
| Timber.tag("GameNativeApi").e(e, "Network error: ${e.message}") | |
| ApiResult.NetworkError(e) | |
| } catch (e: Exception) { | |
| Timber.tag("GameNativeApi").e(e, "Unexpected error: ${e.message}") | |
| ApiResult.NetworkError(e) | |
| } | |
| try { | |
| ApiResult.Success(parser(body)) | |
| } catch (e: Exception) { | |
| Timber.tag("GameNativeApi").e(e, "Failed to parse response: ${e.message}") | |
| ApiResult.NetworkError(e) | |
| } | |
| } catch (e: IOException) { | |
| Timber.tag("GameNativeApi").e(e, "Network error: ${e.message}") | |
| ApiResult.NetworkError(e) | |
| } catch (e: Exception) { | |
| Timber.tag("GameNativeApi").e(e, "Unexpected error: ${e.message}") | |
| ApiResult.NetworkError(e) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/gamenative/api/GameNativeApi.kt` around lines 42 - 49,
The parser lambda's exceptions are being lumped into ApiResult.NetworkError; add
a dedicated parse error case and catch parser-related exceptions separately:
introduce an ApiResult.ParseError(Throwable) variant (or similar) and, in
GameNativeApi's try/catch around ApiResult.Success(parser(body)), add a catch
specifically for parsing exceptions (e.g., com.squareup.moshi.JsonDataException
/ JsonEncodingException / com.google.gson.JsonParseException /
org.json.JSONException or a general JsonParseException) that returns
ApiResult.ParseError(e), keep the existing catch(IOException) =>
ApiResult.NetworkError(e) and a final generic catch for other unexpected
exceptions.
Owner
|
Looks reasonable. |
…sion to use new API
ObfuscatedVoid
force-pushed
the
fix/no-tea
branch
from
9e0fc5c to
0c471c8
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/api/SupportersApi.kt">
<violation number="1" location="app/src/main/java/app/gamenative/api/SupportersApi.kt:18">
P2: Strict JSON parsing can make supporters fetch brittle to backend response shape changes. Consider using `optJSONArray("supporters")` with null checks to gracefully degrade to an empty list instead of throwing `JSONException` on missing/malformed fields.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| GameNativeApi.executeRequest(request) { responseBody -> | ||
| val json = JSONObject(responseBody) | ||
| val array = json.getJSONArray("supporters") |
Contributor
There was a problem hiding this comment.
P2: Strict JSON parsing can make supporters fetch brittle to backend response shape changes. Consider using optJSONArray("supporters") with null checks to gracefully degrade to an empty list instead of throwing JSONException on missing/malformed fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/api/SupportersApi.kt, line 18:
<comment>Strict JSON parsing can make supporters fetch brittle to backend response shape changes. Consider using `optJSONArray("supporters")` with null checks to gracefully degrade to an empty list instead of throwing `JSONException` on missing/malformed fields.</comment>
<file context>
@@ -0,0 +1,29 @@
+
+ GameNativeApi.executeRequest(request) { responseBody ->
+ val json = JSONObject(responseBody)
+ val array = json.getJSONArray("supporters")
+ (0 until array.length()).map { i ->
+ val item = array.getJSONObject(i)
</file context>
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.
remove Supabase integration and update game feedback submission to use new API
Summary by cubic
Removed Supabase integration and migrated feedback submission and supporters fetching to the GameNative worker API with Play Integrity attestation. Added a small OkHttp-based API layer and cleaned up build/workflows.
New Features
Refactors
Written for commit 0c471c8. Summary will update on new commits.
Summary by CodeRabbit