Open
Conversation
Undo local-publish hacks from the workspace-support feature commit: re-enable the HTTPS guard in Formbricks.setup and revert the version back to 1.2.0 so a release can be cut cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughThis pull request migrates the Android SDK from environment-based to workspace-based models. The core identifier changes from 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 please review this PR |
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
android/src/main/java/com/formbricks/android/network/FormbricksRetrofitBuilder.kt (1)
14-57: 🛠️ Refactor suggestion | 🟠 MajorRemove dead code: commented-out HTTPS validation and the now-unused
HttpsOnlyInterceptor.HTTPS is now enforced earlier in
Formbricks.setup()(seeFormbricks.ktL72-77), so the validation/interceptor here is obsolete. Keeping commented-out logic (L16-20, L26) and an unreferenced private class (L43-57) is confusing — readers will wonder whether the enforcement is intentionally disabled or temporarily disabled.Either delete them outright, or, if you want a safety net at the transport layer, re-enable the interceptor. Dead-in-place is the worst of both worlds.
♻️ Proposed cleanup
package com.formbricks.android.network -import com.formbricks.android.logger.Logger -import okhttp3.Interceptor import okhttp3.OkHttpClient -import okhttp3.Response import okhttp3.logging.HttpLoggingInterceptor import retrofit2.Retrofit import retrofit2.converter.gson.GsonConverterFactory -import java.io.IOException import java.util.concurrent.TimeUnit class FormbricksRetrofitBuilder(private val baseUrl: String, private val loggingEnabled: Boolean) { fun getBuilder(): Retrofit.Builder? { - // Validate base URL is HTTPS -// if (!baseUrl.startsWith("https://", ignoreCase = true)) { -// val error = RuntimeException("Only HTTPS URLs are allowed. HTTP URLs are not permitted for security reasons. Provided URL: $baseUrl") -// Logger.e(error) -// return null -// } - val clientBuilder = OkHttpClient.Builder() .connectTimeout(CONNECT_TIMEOUT_MS.toLong(), TimeUnit.MILLISECONDS) .readTimeout(READ_TIMEOUT_MS.toLong(), TimeUnit.MILLISECONDS) .followSslRedirects(true) -// .addInterceptor(HttpsOnlyInterceptor()) - + if (loggingEnabled) { val logging = HttpLoggingInterceptor() logging.setLevel(HttpLoggingInterceptor.Level.BODY) clientBuilder.addInterceptor(logging) } return Retrofit.Builder() .baseUrl(baseUrl) .addConverterFactory(GsonConverterFactory.create()) .client(clientBuilder.build()) } - /** - * Interceptor that ensures all requests use HTTPS protocol - */ - private class HttpsOnlyInterceptor : Interceptor { - `@Throws`(IOException::class) - override fun intercept(chain: Interceptor.Chain): Response { - val request = chain.request() - val url = request.url - - if (!url.isHttps) { - val error = RuntimeException("HTTP request blocked. Only HTTPS requests are allowed. Attempted URL: $url") - Logger.e(error) - throw IOException("HTTP request blocked. Only HTTPS requests are allowed. Attempted URL: $url") - } - - return chain.proceed(request) - } - } - companion object { private const val CONNECT_TIMEOUT_MS = 30 * 1000 // 30 seconds private const val READ_TIMEOUT_MS = 30 * 1000 // 30 seconds } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/src/main/java/com/formbricks/android/network/FormbricksRetrofitBuilder.kt` around lines 14 - 57, Remove the dead commented HTTPS validation and the unused transport-layer interceptor: delete the commented-out baseUrl HTTPS check block in getBuilder() (the commented lines referencing baseUrl) and the commented .addInterceptor(HttpsOnlyInterceptor()) line, and remove the private HttpsOnlyInterceptor class entirely; if you prefer to keep a transport safety-net instead, re-enable the addInterceptor(HttpsOnlyInterceptor()) call in getBuilder() and ensure HttpsOnlyInterceptor is referenced and used, otherwise remove HttpsOnlyInterceptor to avoid unused/private dead code (see getBuilder(), baseUrl, loggingEnabled, HttpsOnlyInterceptor and Formbricks.setup()).android/src/main/java/com/formbricks/android/manager/SurveyManager.kt (1)
158-158: 🧹 Nitpick | 🔵 TrivialStale
SDKErrorname after workspace rename.The log messages and method were renamed to workspace, but
SDKError.unableToRefreshEnvironmentstill carries the old environment-centric name. Consider renaming the error (or adding a workspace-scoped alias) to keep the naming consistent with the rest of the migration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/src/main/java/com/formbricks/android/manager/SurveyManager.kt` at line 158, Replace the stale enum/member SDKError.unableToRefreshEnvironment with a workspace-consistent name (e.g., SDKError.unableToRefreshWorkspace) or add a workspace-scoped alias in the SDKError definition; update the reference in SurveyManager.kt (the val error assignment) and any other usages to use the new name so logs and errors match the renamed workspace terminology, and ensure the SDKError declaration includes the new constant or an alias mapping from unableToRefreshEnvironment to the new unableToRefreshWorkspace to preserve backward compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@android/build.gradle.kts`:
- Line 98: The commented out signAllPublications() call removes artifact signing
and will cause Maven Central publishing to fail; restore signAllPublications()
in the android/build.gradle.kts or make it conditional so CI signs and local
builds can skip: implement a Gradle property check (e.g.,
project.hasProperty("sign") or -Psign) around signAllPublications() so signing
runs by default in CI but can be disabled locally, and ensure any CI pipeline
passes the property (or leave it enabled and revert the commented line).
In
`@android/src/androidTest/java/com/formbricks/android/MockFormbricksApiService.kt`:
- Around line 20-23: The test asset filename is still "Environment.json" while
the variables are named workspace/workspaceJson; update the test fixture
reference in MockFormbricksApiService by renaming the asset file to
"Workspace.json" (or add a copy) and change the code that opens the asset to use
"Workspace.json" so that workspaceJson and workspace (parsed into
WorkspaceResponse) match the file name and intent.
In `@android/src/main/java/com/formbricks/android/Formbricks.kt`:
- Around line 38-42: The internal environmentId getter currently directly
returns the lateinit workspaceId and will throw
UninitializedPropertyAccessException if accessed before setup; change the getter
on environmentId to check ::workspaceId.isInitialized and return a safe default
(e.g., empty string or null) or otherwise document the pre-init contract so
callers won't crash — locate the environmentId property and the lateinit
workspaceId in the Formbricks object (and consider referencing Formbricks.setup
initialization flow) and implement the isInitialized guard or explicit contract
as the fix.
- Around line 87-89: The deprecation notice is currently logged at debug level;
change the call that checks config.usedDeprecatedEnvironmentId in Formbricks
(the block using Logger.d(...)) to use Logger.w(...) so the message is emitted
as a warning; keep the same message text about environmentId being deprecated
and reference the same config.usedDeprecatedEnvironmentId check and surrounding
init/constructor in Formbricks.kt when making the change.
In `@android/src/main/java/com/formbricks/android/manager/SurveyManager.kt`:
- Around line 51-79: The getter for workspaceDataHolderJson performs
shared-preferences writes (migrating legacy key) which is surprising; extract
the migration into a one-shot method (e.g. migrateLegacyCacheIfNeeded()) that
runs once during initialization or first use, make workspaceDataHolderJson.get()
return only prefManager.getString(PREF_FORMBRICKS_WORKSPACE_DATA_HOLDER, null)
with no writes, and move all logic that writes
PREF_FORMBRICKS_WORKSPACE_DATA_HOLDER and removes
PREF_LEGACY_ENVIRONMENT_DATA_HOLDER into the new migration function; ensure the
existing setter still removes the legacy key after writing to the new key.
In
`@android/src/main/java/com/formbricks/android/model/workspace/WorkspaceData.kt`:
- Around line 14-17: The WorkspaceData.settings property is declared
non-nullable but can be absent at runtime when normalizeWorkspaceKeys returns
early; change the property signature in WorkspaceData from Settings to a
nullable type (Settings?) and update any constructors/serializers if necessary
so Gson/Kotlin serialization allows null, keeping existing annotations
(`@SerializedName`, `@SerialName`, `@JsonNames`) intact; verify call sites that
already use null-safe access (settings?....) still compile and remove any
non-null assertions assuming settings may be null.
In
`@android/src/main/java/com/formbricks/android/network/FormbricksApiService.kt`:
- Around line 60-73: The normalizeWorkspaceKeys function mutates a read-only Map
via unchecked casts which can silently no-op if the deserializer returns an
immutable Map and also alters the caller's original payload stored in
WorkspaceDataHolder.originalResponseMap; change the signature of
normalizeWorkspaceKeys to accept a MutableMap<String, Any> (so callers must
provide a mutable map) or, better, stop in-place mutation by creating a shallow
copy of the outer and inner maps, perform the selection/rename on that copy
(select inner["settings"] || inner["workspace"] || inner["project"], remove
workspace/project, set settings), and return the normalized map to the caller so
WorkspaceDataHolder can store the original response separately and receive the
normalized copy instead of having its originalResponseMap mutated.
In
`@android/src/main/java/com/formbricks/android/network/FormbricksRetrofitBuilder.kt`:
- Line 14: Change the getBuilder() signature to return a non-null
Retrofit.Builder (remove the `?`) because there is no code path returning null;
update the function declaration `fun getBuilder(): Retrofit.Builder` and any
related type usages. Then remove the unnecessary null-check in the caller
`FormbricksApiService.initialize()` (where it currently handles a nullable
builder) and use the builder directly. Ensure imports/types remain consistent
and run a quick compile to catch any downstream nullable expectations.
In `@android/src/main/java/com/formbricks/android/webview/FormbricksViewModel.kt`:
- Line 136: The call using first { it.id == surveyId } can throw
NoSuchElementException if no survey matches; change it to firstOrNull { it.id ==
surveyId } when computing matchedSurvey from
workspaceDataHolder.data?.data?.surveys, then handle the null case early (e.g.,
return from getJson/loadHtml or log and skip rendering) so downstream nullable
checks remain valid and the WebView render flow won't crash; reference
matchedSurvey, workspaceDataHolder, surveyId, getJson and loadHtml when making
the change.
---
Outside diff comments:
In `@android/src/main/java/com/formbricks/android/manager/SurveyManager.kt`:
- Line 158: Replace the stale enum/member SDKError.unableToRefreshEnvironment
with a workspace-consistent name (e.g., SDKError.unableToRefreshWorkspace) or
add a workspace-scoped alias in the SDKError definition; update the reference in
SurveyManager.kt (the val error assignment) and any other usages to use the new
name so logs and errors match the renamed workspace terminology, and ensure the
SDKError declaration includes the new constant or an alias mapping from
unableToRefreshEnvironment to the new unableToRefreshWorkspace to preserve
backward compatibility.
In
`@android/src/main/java/com/formbricks/android/network/FormbricksRetrofitBuilder.kt`:
- Around line 14-57: Remove the dead commented HTTPS validation and the unused
transport-layer interceptor: delete the commented-out baseUrl HTTPS check block
in getBuilder() (the commented lines referencing baseUrl) and the commented
.addInterceptor(HttpsOnlyInterceptor()) line, and remove the private
HttpsOnlyInterceptor class entirely; if you prefer to keep a transport
safety-net instead, re-enable the addInterceptor(HttpsOnlyInterceptor()) call in
getBuilder() and ensure HttpsOnlyInterceptor is referenced and used, otherwise
remove HttpsOnlyInterceptor to avoid unused/private dead code (see getBuilder(),
baseUrl, loggingEnabled, HttpsOnlyInterceptor and Formbricks.setup()).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 74f75c06-23b6-46a9-84d5-8126b4caec54
📒 Files selected for processing (31)
android/build.gradle.ktsandroid/src/androidTest/java/com/formbricks/android/FormbricksInstrumentedTest.ktandroid/src/androidTest/java/com/formbricks/android/MockFormbricksApiService.ktandroid/src/androidTest/java/com/formbricks/android/manager/SurveyManagerInstrumentedTest.ktandroid/src/androidTest/java/com/formbricks/android/manager/WorkspaceDataMigrationInstrumentedTest.ktandroid/src/androidTest/java/com/formbricks/android/network/FormbricksApiServiceInstrumentedTest.ktandroid/src/androidTest/java/com/formbricks/android/webview/FormbricksViewModelInstrumentedTest.ktandroid/src/main/java/com/formbricks/android/Formbricks.ktandroid/src/main/java/com/formbricks/android/api/FormbricksApi.ktandroid/src/main/java/com/formbricks/android/extensions/DateExtensions.ktandroid/src/main/java/com/formbricks/android/helper/FormbricksConfig.ktandroid/src/main/java/com/formbricks/android/manager/SurveyManager.ktandroid/src/main/java/com/formbricks/android/model/environment/EnvironmentData.ktandroid/src/main/java/com/formbricks/android/model/environment/EnvironmentResponse.ktandroid/src/main/java/com/formbricks/android/model/workspace/ActionClass.ktandroid/src/main/java/com/formbricks/android/model/workspace/ActionClassReference.ktandroid/src/main/java/com/formbricks/android/model/workspace/BrandColor.ktandroid/src/main/java/com/formbricks/android/model/workspace/Segment.ktandroid/src/main/java/com/formbricks/android/model/workspace/SegmentFilterResourceDeserializer.ktandroid/src/main/java/com/formbricks/android/model/workspace/Settings.ktandroid/src/main/java/com/formbricks/android/model/workspace/Styling.ktandroid/src/main/java/com/formbricks/android/model/workspace/Survey.ktandroid/src/main/java/com/formbricks/android/model/workspace/Trigger.ktandroid/src/main/java/com/formbricks/android/model/workspace/WorkspaceData.ktandroid/src/main/java/com/formbricks/android/model/workspace/WorkspaceDataHolder.ktandroid/src/main/java/com/formbricks/android/model/workspace/WorkspaceResponse.ktandroid/src/main/java/com/formbricks/android/model/workspace/WorkspaceResponseData.ktandroid/src/main/java/com/formbricks/android/network/FormbricksApiService.ktandroid/src/main/java/com/formbricks/android/network/FormbricksRetrofitBuilder.ktandroid/src/main/java/com/formbricks/android/network/FormbricksService.ktandroid/src/main/java/com/formbricks/android/webview/FormbricksViewModel.kt
💤 Files with no reviewable changes (2)
- android/src/main/java/com/formbricks/android/model/environment/EnvironmentData.kt
- android/src/main/java/com/formbricks/android/model/environment/EnvironmentResponse.kt
|
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
1 participant
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.
Adds workspace support to the android sdk and allows backwards compat with environmentId