Add freemium model with 30-day trial, Google Play IAP, and write-access gating#630
Open
tobihagemann wants to merge 58 commits intodevelopfrom
Open
Add freemium model with 30-day trial, Google Play IAP, and write-access gating#630tobihagemann wants to merge 58 commits intodevelopfrom
tobihagemann wants to merge 58 commits intodevelopfrom
Conversation
…ial info, fix subscription state
…itles, rename section to Premium
…eContentViewBinder
…ove double enforcement
SailReal
requested changes
Apr 17, 2026
SailReal
requested changes
Apr 17, 2026
Member
SailReal
left a comment
SailReal
left a comment
There was a problem hiding this comment.
As we now show an activity when writing is prohibited, we can remove the toast because the toast does not add any info
SailReal
requested changes
Apr 17, 2026
Member
SailReal
left a comment
SailReal
left a comment
There was a problem hiding this comment.
This should not be shown when screen lock is set
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
presentation/src/testPlaystoreiap/java/org/cryptomator/presentation/service/PurchaseManagerTest.kt (1)
20-20: Nit:acknowledgedTokenscan beval.The field is only reassigned via
.clear()/.add()on the list itself, never re-bound, so it can be declaredvalto matchacknowledgePurchasebelow.♻️ Proposed change
- private var acknowledgedTokens = mutableListOf<String>() + private val acknowledgedTokens = mutableListOf<String>()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/testPlaystoreiap/java/org/cryptomator/presentation/service/PurchaseManagerTest.kt` at line 20, The field acknowledgedTokens is only mutated via its methods (clear/add) and never re-assigned, so change its declaration from var to val to make it immutable; update the declaration of acknowledgedTokens in PurchaseManagerTest (referencing the acknowledgedTokens symbol) to use val to match acknowledgePurchase and ensure no other code rebinds it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@presentation/src/main/res/values/strings.xml`:
- Around line 110-112: Remove the three unused string resources from the XML:
delete the <string> entries named screen_settings_license_unlock_prompt,
screen_settings_license_unlock_summary, and screen_settings_license_unlocked_iap
from strings.xml so they are no longer shipped or translated; ensure no
references remain by searching for these exact resource names before committing.
---
Nitpick comments:
In
`@presentation/src/testPlaystoreiap/java/org/cryptomator/presentation/service/PurchaseManagerTest.kt`:
- Line 20: The field acknowledgedTokens is only mutated via its methods
(clear/add) and never re-assigned, so change its declaration from var to val to
make it immutable; update the declaration of acknowledgedTokens in
PurchaseManagerTest (referencing the acknowledgedTokens symbol) to use val to
match acknowledgePurchase and ensure no other code rebinds it.
🪄 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: 7d63b504-fbbf-4c67-9200-55305f074c72
📒 Files selected for processing (11)
domain/build.gradlepresentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/SettingsFragment.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.ktpresentation/src/main/res/values/strings.xmlpresentation/src/main/res/xml/preferences.xmlpresentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseManager.ktpresentation/src/testPlaystoreiap/java/org/cryptomator/presentation/service/PurchaseManagerTest.ktutil/src/main/java/org/cryptomator/util/FlavorConfig.kt
✅ Files skipped from review due to trivial changes (3)
- presentation/src/main/res/xml/preferences.xml
- util/src/main/java/org/cryptomator/util/FlavorConfig.kt
- presentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseManager.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- domain/build.gradle
- presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.kt
- presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt
Comment on lines
+110
to
+112
| <string name="screen_settings_license_unlock_prompt">Unlock write access</string> | ||
| <string name="screen_settings_license_unlock_summary">Tap to enter your license key.</string> | ||
| <string name="screen_settings_license_unlocked_iap">Write access is unlocked</string> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the three strings at lines 110-112 are referenced anywhere in code or XML.
for key in screen_settings_license_unlock_prompt screen_settings_license_unlock_summary screen_settings_license_unlocked_iap; do
echo "=== $key ==="
rg -n --type kotlin --type java --type xml "R\.string\.$key|@string/$key|\"$key\"" || echo " (no references found)"
doneRepository: cryptomator/android
Length of output: 612
Remove unused license strings at lines 110-112.
These three strings have no references anywhere in the codebase and appear to be leftovers from an earlier refactor iteration. Shipping orphaned strings increases translation maintenance burden unnecessarily. Remove:
screen_settings_license_unlock_prompt(line 110)screen_settings_license_unlock_summary(line 111)screen_settings_license_unlocked_iap(line 112)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@presentation/src/main/res/values/strings.xml` around lines 110 - 112, Remove
the three unused string resources from the XML: delete the <string> entries
named screen_settings_license_unlock_prompt,
screen_settings_license_unlock_summary, and screen_settings_license_unlocked_iap
from strings.xml so they are no longer shipped or translated; ensure no
references remain by searching for these exact resource names before committing.
SailReal
requested changes
Apr 17, 2026
Comment on lines
+218
to
+219
| setResult(RESULT_OK) | ||
| finish() |
Member
There was a problem hiding this comment.
Would recommend to adjust this to
Suggested change
| setResult(RESULT_OK) | |
| finish() | |
| welcomePresenter.finishWithResult(null) |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt (1)
169-176: Naming nit: singular form would match the sibling API.Each call adds/removes a single
Consumer<String>, but the methods are namedaddLicenseChangedListener**s**/removeLicenseChangedListener**s**. The analogous lock-timeout API on line 55 uses singularaddLockTimeoutChangedListener. Consider renaming for consistency.♻️ Proposed rename
- fun addLicenseChangedListeners(listener: Consumer<String>) { + fun addLicenseChangedListener(listener: Consumer<String>) { licenseChangedListeners[listener] = null listener.accept(licenseToken()) } - fun removeLicenseChangedListeners(listener: Consumer<String>) { + fun removeLicenseChangedListener(listener: Consumer<String>) { licenseChangedListeners.remove(listener) }Call sites in
LicenseStateOrchestrator.onResume()/onPause()would need the matching update.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt` around lines 169 - 176, Rename the plural methods addLicenseChangedListeners and removeLicenseChangedListeners to the singular forms addLicenseChangedListener and removeLicenseChangedListener to match the sibling API; update the function declarations in SharedPreferencesHandler (keeping behavior unchanged: licenseChangedListeners[listener] = null and licenseChangedListeners.remove(listener)) and then update all call sites such as LicenseStateOrchestrator.onResume() and LicenseStateOrchestrator.onPause() to use the new method names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt`:
- Around line 169-176: Rename the plural methods addLicenseChangedListeners and
removeLicenseChangedListeners to the singular forms addLicenseChangedListener
and removeLicenseChangedListener to match the sibling API; update the function
declarations in SharedPreferencesHandler (keeping behavior unchanged:
licenseChangedListeners[listener] = null and
licenseChangedListeners.remove(listener)) and then update all call sites such as
LicenseStateOrchestrator.onResume() and LicenseStateOrchestrator.onPause() to
use the new method names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b47d3e9-1f0c-492b-bf45-521ea480bf7d
📒 Files selected for processing (3)
presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.ktpresentation/src/test/java/org/cryptomator/presentation/licensing/LicenseEnforcerTest.ktutil/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt
✅ Files skipped from review due to trivial changes (1)
- presentation/src/test/java/org/cryptomator/presentation/licensing/LicenseEnforcerTest.kt
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
presentation/src/main/res/values/strings.xml (1)
96-100: Use positional format specifiers for translator clarity.
screen_license_check_trial_expiration(%s) andscreen_license_check_trial_active_info(%s) should use%1$sfor consistency with the rest of the file and to make translation unambiguous — the same reasonnotification_auto_upload_messagewas converted to%1$d/%2$din this same change (line 693). Android Studio's resource lint flags non-positional%swhen the string is formatted.Proposed diff
- <string name="screen_license_check_trial_expiration">Expiration Date: %s</string> + <string name="screen_license_check_trial_expiration">Expiration Date: %1$s</string> <string name="screen_license_check_trial_status_active">Active</string> <string name="screen_license_check_trial_status_expired">Expired</string> - <string name="screen_license_check_trial_active_info">Trial expires: %s</string> + <string name="screen_license_check_trial_active_info">Trial expires: %1$s</string>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/res/values/strings.xml` around lines 96 - 100, Update the two string resources so their format specifiers are positional: replace the non-positional "%s" with "%1$s" in screen_license_check_trial_expiration and screen_license_check_trial_active_info so translations remain unambiguous and lint warnings are resolved.presentation/src/testPlaystoreiap/java/org/cryptomator/presentation/service/PurchaseManagerTest.kt (1)
17-254: LGTM.Good coverage across PENDING/PURCHASED/UNSPECIFIED_STATE/unknown states, mixed lists, acknowledgement,
clearIfNotFoundsemantics, and the negative assertion thatPurchaseManagerdoes not arm purchase-revoked state (that's the coordinator's responsibility).One small gap worth considering: no positive assertion that
acknowledgePurchaseis not invoked whenpurchase.isAcknowledged == true. Easy to add alongside the existingacknowledges unacknowledged purchasetest.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/testPlaystoreiap/java/org/cryptomator/presentation/service/PurchaseManagerTest.kt` around lines 17 - 254, Add a test that asserts acknowledgePurchase is NOT invoked when a purchase already has isAcknowledged == true: create a purchase via mockPurchase(ProductInfo.PRODUCT_FULL_VERSION, Purchase.PurchaseState.PURCHASED, "token-1", isAcknowledged = true), call purchaseManager.handleInAppPurchases(listOf(purchase), acknowledgePurchase = acknowledgePurchase) and assert acknowledgedTokens remains empty (or verify no acknowledgment happened); place it alongside the existing `handleInAppPurchases acknowledges unacknowledged purchase` test to cover the negative case.presentation/src/main/java/org/cryptomator/presentation/service/RestoreOutcome.kt (1)
3-7: Nit: object/class names should be PascalCase.Kotlin style guide recommends PascalCase for
objectandclassnames (Restored,NothingToRestore,Failed) rather than SCREAMING_SNAKE_CASE, which reads like an enum constant. Purely a style call; no functional impact.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/service/RestoreOutcome.kt` around lines 3 - 7, The sealed interface RestoreOutcome uses SCREAMING_SNAKE_CASE for its implementations (RESTORED, NOTHING_TO_RESTORE, FAILED); rename them to PascalCase (Restored, NothingToRestore, Failed) and update any references/usages accordingly; adjust the declarations in RestoreOutcome (data object Restored : RestoreOutcome, data object NothingToRestore : RestoreOutcome, data class Failed(val cause: Throwable? = null) : RestoreOutcome) and run a project-wide refactor to fix imports/usages to the new symbols.util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt (1)
178-192: Nit: inconsistent indentation inonSharedPreferenceChanged.The function signature and
whenblock are indented one level shallower than the rest of the class, and the innerLOCK_TIMEOUT ->branch uses a different indent depth than theLICENSE_TOKEN, …branch. Reformat for consistency.Proposed reformatting
-override fun onSharedPreferenceChanged(sharedPreferences: SharedPreferences, key: String?) { - when (key) { - LOCK_TIMEOUT -> { - val lockTimeout = lockTimeout - lockTimeoutChangedListeners.keys.forEach { listener -> - listener.accept(lockTimeout) - } - } - LICENSE_TOKEN, TRIAL_EXPIRATION_DATE, HAS_RUNNING_SUBSCRIPTION -> { - licenseChangedListeners.keys.forEach { listener -> - listener.accept(licenseToken()) - } - } - } -} + override fun onSharedPreferenceChanged(sharedPreferences: SharedPreferences, key: String?) { + when (key) { + LOCK_TIMEOUT -> { + val lockTimeout = lockTimeout + lockTimeoutChangedListeners.keys.forEach { listener -> + listener.accept(lockTimeout) + } + } + LICENSE_TOKEN, TRIAL_EXPIRATION_DATE, HAS_RUNNING_SUBSCRIPTION -> { + licenseChangedListeners.keys.forEach { listener -> + listener.accept(licenseToken()) + } + } + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt` around lines 178 - 192, Reformat the indentation in onSharedPreferenceChanged so the function signature, the when { ... } block, and all case branches use consistent indentation with the rest of the class; specifically align the onSharedPreferenceChanged declaration and the when keyword, make the LOCK_TIMEOUT -> branch body (including the local val lockTimeout and lockTimeoutChangedListeners.keys.forEach { listener -> listener.accept(lockTimeout) }) use the same indent level as the LICENSE_TOKEN, TRIAL_EXPIRATION_DATE, HAS_RUNNING_SUBSCRIPTION -> branch, and ensure the licenseChangedListeners.keys.forEach { listener -> listener.accept(licenseToken()) } block has matching brace alignment; keep existing logic (references: onSharedPreferenceChanged, LOCK_TIMEOUT, lockTimeout, lockTimeoutChangedListeners, LICENSE_TOKEN, TRIAL_EXPIRATION_DATE, HAS_RUNNING_SUBSCRIPTION, licenseChangedListeners, licenseToken()) unchanged.presentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseRefreshCoordinator.kt (1)
44-69: Minor:onSettledreads aggregated state without holdinglock.
hadWriteAccessBeforeis captured before issuing queries, butonSettledreadsinappChange/subsChange/failureoutsidesynchronized(lock). Play Billing callbacks are documented to run on the main thread, so this is fine in practice, but since you're already defensively using a lock around the writes, the read side should match for consistency (and to survive any future change of dispatch thread).🧹 Suggested refactor
fun onSettled() { - val localInapp = inappChange - val localSubs = subsChange - val localFailure = failure + val localInapp: PurchaseFieldChange? + val localSubs: PurchaseFieldChange? + val localFailure: Throwable? + synchronized(lock) { + localInapp = inappChange + localSubs = subsChange + localFailure = failure + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseRefreshCoordinator.kt` around lines 44 - 69, The onSettled function reads shared variables (inappChange, subsChange, failure and later calls licenseEnforcer.hasWriteAccess() to compute hadWriteAccessAfter) without holding the existing lock; wrap the reading of these aggregated states and the computation of hadWriteAccessAfter in synchronized(lock) to mirror the write-side protection: e.g. inside onSettled acquire synchronized(lock) to copy localInapp/localSubs/localFailure and hadWriteAccessAfter (or compute hadWriteAccessAfter while holding the lock), then release the lock and continue the rest of the logic (including sharedPreferencesHandler.setPurchaseRevokedState and complete) using those local copies so all reads are protected by the same lock used for writes.presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.kt (1)
55-62: Minor:consumeLastRestoreOutcome()read-then-clear is non-atomic.If both
LicenseCheckActivityandWelcomeActivity(or any two consumers) ever run theironResumeconcurrently, both could observe the samelastRestoreOutcomebefore either nulls it, resulting in a double dialog. In practice Android Activity lifecycle callbacks all run on the main thread so this is fine today, but a small@Synchronizedor atomic getAndSet would make the invariant explicit and future-proof.🧹 Suggested refactor
- `@Volatile` - var lastRestoreOutcome: RestoreOutcome? = null - - fun consumeLastRestoreOutcome(): RestoreOutcome? { - val outcome = lastRestoreOutcome - lastRestoreOutcome = null - return outcome - } + private val lastRestoreOutcomeRef = java.util.concurrent.atomic.AtomicReference<RestoreOutcome?>(null) + + var lastRestoreOutcome: RestoreOutcome? + get() = lastRestoreOutcomeRef.get() + set(value) { lastRestoreOutcomeRef.set(value) } + + fun consumeLastRestoreOutcome(): RestoreOutcome? = lastRestoreOutcomeRef.getAndSet(null)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.kt` around lines 55 - 62, The read-then-clear of lastRestoreOutcome in consumeLastRestoreOutcome is non-atomic and can race if multiple consumers call it concurrently; update consumeLastRestoreOutcome to perform an atomic get-and-clear (e.g., synchronize the method or use an AtomicReference for lastRestoreOutcome and call getAndSet(null)) so the fetch-and-null operation is atomic and only one caller observes the value (refer to lastRestoreOutcome and consumeLastRestoreOutcome to locate 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
`@presentation/src/playstoreiap/java/org/cryptomator/presentation/service/IapBillingService.kt`:
- Around line 54-70: Pending product-details callbacks in
pendingProductDetailsCallbacks are never drained when
onBillingSetupFinished(billingResult) returns non-OK, leaving callers hung;
update onBillingSetupFinished (and the symmetric failure path around lines
handling billing disconnection) to drain pendingProductDetailsCallbacks on error
by copying and clearing the list then invoking each callback with an empty list
(or an error signal) so UI callers like
LicenseContentViewBinder.loadAndBindPrices receive a terminal response; ensure
you reference and clear pendingProductDetailsCallbacks before calling callbacks
to avoid reentrancy and duplicate invocations.
- Around line 40-76: initBillingClient currently overwrites billingClient,
sharedPreferencesHandler, purchaseManager and purchaseRefreshCoordinator on
every call causing leaked BillingClient instances and stale listeners; modify
initBillingClient to be idempotent by either short-circuiting when an existing
billingClient is initialized and ready (check billingClient != null &&
billingClient.isReady()) or gracefully tearing down the old client before
replacing it (call billingClient.endConnection() and clear/transfer any
in-flight state such as pendingProductDetailsCallbacks) so you do not leak
connections or re-register listeners for
PurchaseManager/PurchaseRefreshCoordinator.
In
`@presentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseRefreshCoordinator.kt`:
- Around line 82-107: The BillingClient async callbacks can throw and prevent
onQueryComplete() from running; wrap the bodies of both
billingClient.queryPurchasesAsync lambdas in a try/catch/finally so any
exception from purchaseManager.handleInAppPurchases,
purchaseManager.handleSubscriptionPurchases or the acknowledge callback is
caught and logged (or captured into failure) and onQueryComplete() is called
from finally; ensure you set failure when an exception occurs and then call
onQueryComplete() in the finally block so queriesCompleted always progresses to
totalQueries.
---
Nitpick comments:
In `@presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.kt`:
- Around line 55-62: The read-then-clear of lastRestoreOutcome in
consumeLastRestoreOutcome is non-atomic and can race if multiple consumers call
it concurrently; update consumeLastRestoreOutcome to perform an atomic
get-and-clear (e.g., synchronize the method or use an AtomicReference for
lastRestoreOutcome and call getAndSet(null)) so the fetch-and-null operation is
atomic and only one caller observes the value (refer to lastRestoreOutcome and
consumeLastRestoreOutcome to locate the change).
In
`@presentation/src/main/java/org/cryptomator/presentation/service/RestoreOutcome.kt`:
- Around line 3-7: The sealed interface RestoreOutcome uses SCREAMING_SNAKE_CASE
for its implementations (RESTORED, NOTHING_TO_RESTORE, FAILED); rename them to
PascalCase (Restored, NothingToRestore, Failed) and update any references/usages
accordingly; adjust the declarations in RestoreOutcome (data object Restored :
RestoreOutcome, data object NothingToRestore : RestoreOutcome, data class
Failed(val cause: Throwable? = null) : RestoreOutcome) and run a project-wide
refactor to fix imports/usages to the new symbols.
In `@presentation/src/main/res/values/strings.xml`:
- Around line 96-100: Update the two string resources so their format specifiers
are positional: replace the non-positional "%s" with "%1$s" in
screen_license_check_trial_expiration and screen_license_check_trial_active_info
so translations remain unambiguous and lint warnings are resolved.
In
`@presentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseRefreshCoordinator.kt`:
- Around line 44-69: The onSettled function reads shared variables (inappChange,
subsChange, failure and later calls licenseEnforcer.hasWriteAccess() to compute
hadWriteAccessAfter) without holding the existing lock; wrap the reading of
these aggregated states and the computation of hadWriteAccessAfter in
synchronized(lock) to mirror the write-side protection: e.g. inside onSettled
acquire synchronized(lock) to copy localInapp/localSubs/localFailure and
hadWriteAccessAfter (or compute hadWriteAccessAfter while holding the lock),
then release the lock and continue the rest of the logic (including
sharedPreferencesHandler.setPurchaseRevokedState and complete) using those local
copies so all reads are protected by the same lock used for writes.
In
`@presentation/src/testPlaystoreiap/java/org/cryptomator/presentation/service/PurchaseManagerTest.kt`:
- Around line 17-254: Add a test that asserts acknowledgePurchase is NOT invoked
when a purchase already has isAcknowledged == true: create a purchase via
mockPurchase(ProductInfo.PRODUCT_FULL_VERSION, Purchase.PurchaseState.PURCHASED,
"token-1", isAcknowledged = true), call
purchaseManager.handleInAppPurchases(listOf(purchase), acknowledgePurchase =
acknowledgePurchase) and assert acknowledgedTokens remains empty (or verify no
acknowledgment happened); place it alongside the existing `handleInAppPurchases
acknowledges unacknowledged purchase` test to cover the negative case.
In `@util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt`:
- Around line 178-192: Reformat the indentation in onSharedPreferenceChanged so
the function signature, the when { ... } block, and all case branches use
consistent indentation with the rest of the class; specifically align the
onSharedPreferenceChanged declaration and the when keyword, make the
LOCK_TIMEOUT -> branch body (including the local val lockTimeout and
lockTimeoutChangedListeners.keys.forEach { listener ->
listener.accept(lockTimeout) }) use the same indent level as the LICENSE_TOKEN,
TRIAL_EXPIRATION_DATE, HAS_RUNNING_SUBSCRIPTION -> branch, and ensure the
licenseChangedListeners.keys.forEach { listener ->
listener.accept(licenseToken()) } block has matching brace alignment; keep
existing logic (references: onSharedPreferenceChanged, LOCK_TIMEOUT,
lockTimeout, lockTimeoutChangedListeners, LICENSE_TOKEN, TRIAL_EXPIRATION_DATE,
HAS_RUNNING_SUBSCRIPTION, licenseChangedListeners, licenseToken()) unchanged.
🪄 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: 54f43bd2-18db-4ed7-a70e-8641617d4f1a
📒 Files selected for processing (24)
presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.ktpresentation/src/main/java/org/cryptomator/presentation/service/PurchaseRevokedReason.ktpresentation/src/main/java/org/cryptomator/presentation/service/PurchaseRevokedToastObserver.ktpresentation/src/main/java/org/cryptomator/presentation/service/RestoreOutcome.ktpresentation/src/main/java/org/cryptomator/presentation/service/RestoreOutcomeHandler.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/dialog/NoFullVersionDialog.ktpresentation/src/main/java/org/cryptomator/presentation/ui/dialog/RestoreFailedDialog.ktpresentation/src/main/java/org/cryptomator/presentation/ui/dialog/RestoreSuccessfulDialog.ktpresentation/src/main/java/org/cryptomator/presentation/ui/layout/LicenseContentViewBinder.ktpresentation/src/main/res/layout/dialog_no_full_version.xmlpresentation/src/main/res/layout/dialog_restore_failed.xmlpresentation/src/main/res/layout/dialog_restore_successful.xmlpresentation/src/main/res/values/strings.xmlpresentation/src/nonplaystoreiap/java/org/cryptomator/presentation/service/IapBillingService.ktpresentation/src/playstoreiap/java/org/cryptomator/presentation/service/IapBillingService.ktpresentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseFieldChange.ktpresentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseManager.ktpresentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseRefreshCoordinator.ktpresentation/src/test/java/org/cryptomator/presentation/service/PurchaseRevokedToastObserverTest.ktpresentation/src/testPlaystoreiap/java/org/cryptomator/presentation/service/PurchaseManagerTest.ktpresentation/src/testPlaystoreiap/java/org/cryptomator/presentation/service/PurchaseRefreshCoordinatorTest.ktutil/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt
✅ Files skipped from review due to trivial changes (4)
- presentation/src/main/res/layout/dialog_restore_successful.xml
- presentation/src/main/res/layout/dialog_no_full_version.xml
- presentation/src/main/res/layout/dialog_restore_failed.xml
- presentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseFieldChange.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- presentation/src/nonplaystoreiap/java/org/cryptomator/presentation/service/IapBillingService.kt
- presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt
Comment on lines
+40
to
+76
| private fun initBillingClient(context: Context) { | ||
| this.sharedPreferencesHandler = SharedPreferencesHandler(context) | ||
| this.purchaseManager = PurchaseManager(sharedPreferencesHandler) | ||
| this.purchaseRefreshCoordinator = PurchaseRefreshCoordinator(sharedPreferencesHandler) | ||
| val pendingPurchasesParams = PendingPurchasesParams.newBuilder() | ||
| .enableOneTimeProducts() | ||
| .enablePrepaidPlans() | ||
| .build() | ||
| billingClient = BillingClient.newBuilder(context) | ||
| .setListener(this) | ||
| .enablePendingPurchases(pendingPurchasesParams) | ||
| .enableAutoServiceReconnection() | ||
| .build() | ||
| billingClient.startConnection(object : BillingClientStateListener { | ||
| override fun onBillingSetupFinished(billingResult: BillingResult) { | ||
| if (billingResult.responseCode == BillingClient.BillingResponseCode.OK) { | ||
| Timber.tag("IapBillingService").d("Billing setup successful") | ||
| queryExistingPurchases() | ||
| synchronized(pendingProductDetailsCallbacks) { | ||
| if (pendingProductDetailsCallbacks.isNotEmpty()) { | ||
| val callbacks = ArrayList(pendingProductDetailsCallbacks) | ||
| pendingProductDetailsCallbacks.clear() | ||
| queryProductDetails { products -> | ||
| callbacks.forEach { it(products) } | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| Timber.tag("IapBillingService").e("Billing setup not successful, error: %d", billingResult.responseCode) | ||
| } | ||
| } | ||
|
|
||
| override fun onBillingServiceDisconnected() { | ||
| Timber.tag("IapBillingService").i("Billing service disconnected") | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
initBillingClient is not idempotent — risks leaking BillingClient instances.
Binder.init(context) can be invoked more than once across the Service lifetime (e.g. multiple bindService calls from different activities, or re-binds after a process-lifecycle event). Each call overwrites billingClient, sharedPreferencesHandler, purchaseManager, and purchaseRefreshCoordinator without ending the previous BillingClient, leaking its connection and any in-flight callbacks. It also re-registers pending-purchase listeners on a stale client. Consider short-circuiting when already initialized, or tearing the existing client down first.
🛡️ Suggested guard
private fun initBillingClient(context: Context) {
+ if (::billingClient.isInitialized) {
+ return
+ }
this.sharedPreferencesHandler = SharedPreferencesHandler(context)📝 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
| private fun initBillingClient(context: Context) { | |
| this.sharedPreferencesHandler = SharedPreferencesHandler(context) | |
| this.purchaseManager = PurchaseManager(sharedPreferencesHandler) | |
| this.purchaseRefreshCoordinator = PurchaseRefreshCoordinator(sharedPreferencesHandler) | |
| val pendingPurchasesParams = PendingPurchasesParams.newBuilder() | |
| .enableOneTimeProducts() | |
| .enablePrepaidPlans() | |
| .build() | |
| billingClient = BillingClient.newBuilder(context) | |
| .setListener(this) | |
| .enablePendingPurchases(pendingPurchasesParams) | |
| .enableAutoServiceReconnection() | |
| .build() | |
| billingClient.startConnection(object : BillingClientStateListener { | |
| override fun onBillingSetupFinished(billingResult: BillingResult) { | |
| if (billingResult.responseCode == BillingClient.BillingResponseCode.OK) { | |
| Timber.tag("IapBillingService").d("Billing setup successful") | |
| queryExistingPurchases() | |
| synchronized(pendingProductDetailsCallbacks) { | |
| if (pendingProductDetailsCallbacks.isNotEmpty()) { | |
| val callbacks = ArrayList(pendingProductDetailsCallbacks) | |
| pendingProductDetailsCallbacks.clear() | |
| queryProductDetails { products -> | |
| callbacks.forEach { it(products) } | |
| } | |
| } | |
| } | |
| } else { | |
| Timber.tag("IapBillingService").e("Billing setup not successful, error: %d", billingResult.responseCode) | |
| } | |
| } | |
| override fun onBillingServiceDisconnected() { | |
| Timber.tag("IapBillingService").i("Billing service disconnected") | |
| } | |
| }) | |
| } | |
| private fun initBillingClient(context: Context) { | |
| if (::billingClient.isInitialized) { | |
| return | |
| } | |
| this.sharedPreferencesHandler = SharedPreferencesHandler(context) | |
| this.purchaseManager = PurchaseManager(sharedPreferencesHandler) | |
| this.purchaseRefreshCoordinator = PurchaseRefreshCoordinator(sharedPreferencesHandler) | |
| val pendingPurchasesParams = PendingPurchasesParams.newBuilder() | |
| .enableOneTimeProducts() | |
| .enablePrepaidPlans() | |
| .build() | |
| billingClient = BillingClient.newBuilder(context) | |
| .setListener(this) | |
| .enablePendingPurchases(pendingPurchasesParams) | |
| .enableAutoServiceReconnection() | |
| .build() | |
| billingClient.startConnection(object : BillingClientStateListener { | |
| override fun onBillingSetupFinished(billingResult: BillingResult) { | |
| if (billingResult.responseCode == BillingClient.BillingResponseCode.OK) { | |
| Timber.tag("IapBillingService").d("Billing setup successful") | |
| queryExistingPurchases() | |
| synchronized(pendingProductDetailsCallbacks) { | |
| if (pendingProductDetailsCallbacks.isNotEmpty()) { | |
| val callbacks = ArrayList(pendingProductDetailsCallbacks) | |
| pendingProductDetailsCallbacks.clear() | |
| queryProductDetails { products -> | |
| callbacks.forEach { it(products) } | |
| } | |
| } | |
| } | |
| } else { | |
| Timber.tag("IapBillingService").e("Billing setup not successful, error: %d", billingResult.responseCode) | |
| } | |
| } | |
| override fun onBillingServiceDisconnected() { | |
| Timber.tag("IapBillingService").i("Billing service disconnected") | |
| } | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/playstoreiap/java/org/cryptomator/presentation/service/IapBillingService.kt`
around lines 40 - 76, initBillingClient currently overwrites billingClient,
sharedPreferencesHandler, purchaseManager and purchaseRefreshCoordinator on
every call causing leaked BillingClient instances and stale listeners; modify
initBillingClient to be idempotent by either short-circuiting when an existing
billingClient is initialized and ready (check billingClient != null &&
billingClient.isReady()) or gracefully tearing down the old client before
replacing it (call billingClient.endConnection() and clear/transfer any
in-flight state such as pendingProductDetailsCallbacks) so you do not leak
connections or re-register listeners for
PurchaseManager/PurchaseRefreshCoordinator.
Comment on lines
+54
to
+70
| override fun onBillingSetupFinished(billingResult: BillingResult) { | ||
| if (billingResult.responseCode == BillingClient.BillingResponseCode.OK) { | ||
| Timber.tag("IapBillingService").d("Billing setup successful") | ||
| queryExistingPurchases() | ||
| synchronized(pendingProductDetailsCallbacks) { | ||
| if (pendingProductDetailsCallbacks.isNotEmpty()) { | ||
| val callbacks = ArrayList(pendingProductDetailsCallbacks) | ||
| pendingProductDetailsCallbacks.clear() | ||
| queryProductDetails { products -> | ||
| callbacks.forEach { it(products) } | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| Timber.tag("IapBillingService").e("Billing setup not successful, error: %d", billingResult.responseCode) | ||
| } | ||
| } |
There was a problem hiding this comment.
Pending product-details callbacks are never drained on billing-setup failure.
If onBillingSetupFinished returns a non-OK response, pendingProductDetailsCallbacks are left queued indefinitely, so any UI waiting for prices (e.g. LicenseContentViewBinder.loadAndBindPrices) silently hangs and the subscription/lifetime buttons stay disabled with no feedback. Auto-reconnection mitigates transient failures, but terminal failures (billing unavailable, feature not supported) never recover. Consider either invoking queued callbacks with an empty list on non-OK, or propagating an error signal so the UI can surface "unavailable".
Also applies to: 118-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/playstoreiap/java/org/cryptomator/presentation/service/IapBillingService.kt`
around lines 54 - 70, Pending product-details callbacks in
pendingProductDetailsCallbacks are never drained when
onBillingSetupFinished(billingResult) returns non-OK, leaving callers hung;
update onBillingSetupFinished (and the symmetric failure path around lines
handling billing disconnection) to drain pendingProductDetailsCallbacks on error
by copying and clearing the list then invoking each callback with an empty list
(or an error signal) so UI callers like
LicenseContentViewBinder.loadAndBindPrices receive a terminal response; ensure
you reference and clear pendingProductDetailsCallbacks before calling callbacks
to avoid reentrancy and duplicate invocations.
Comment on lines
+82
to
+107
| val inappParams = QueryPurchasesParams.newBuilder() | ||
| .setProductType(BillingClient.ProductType.INAPP) | ||
| .build() | ||
| billingClient.queryPurchasesAsync(inappParams) { billingResult: BillingResult, purchases: List<Purchase> -> | ||
| synchronized(lock) { | ||
| if (billingResult.responseCode != BillingClient.BillingResponseCode.OK) { | ||
| failure = Throwable("Query failed with code: ${billingResult.responseCode}") | ||
| } else { | ||
| inappChange = purchaseManager.handleInAppPurchases(purchases, clearIfNotFound = true, acknowledgePurchase = acknowledge) | ||
| } | ||
| } | ||
| onQueryComplete() | ||
| } | ||
| val subsParams = QueryPurchasesParams.newBuilder() | ||
| .setProductType(BillingClient.ProductType.SUBS) | ||
| .build() | ||
| billingClient.queryPurchasesAsync(subsParams) { billingResult: BillingResult, purchases: List<Purchase> -> | ||
| synchronized(lock) { | ||
| if (billingResult.responseCode != BillingClient.BillingResponseCode.OK) { | ||
| failure = Throwable("Query failed with code: ${billingResult.responseCode}") | ||
| } else { | ||
| subsChange = purchaseManager.handleSubscriptionPurchases(purchases, clearIfNotFound = true, acknowledgePurchase = acknowledge) | ||
| } | ||
| } | ||
| onQueryComplete() | ||
| } |
There was a problem hiding this comment.
onComplete can fail to fire if an async callback throws.
The outer try/catch at lines 31/108 only guards setup and the initial queryPurchasesAsync invocations — it cannot catch exceptions thrown later on the Billing callback thread. If purchaseManager.handleInAppPurchases(...) / handleSubscriptionPurchases(...) or the provided acknowledge callback throws inside either response lambda, onQueryComplete() is skipped, queriesCompleted never reaches totalQueries, onSettled() never runs, and onComplete is never invoked — the restore hangs indefinitely from the caller's perspective (e.g. a spinner in the UI).
Wrap each callback body defensively so failures still drive onQueryComplete():
🛡️ Suggested fix
billingClient.queryPurchasesAsync(inappParams) { billingResult: BillingResult, purchases: List<Purchase> ->
- synchronized(lock) {
- if (billingResult.responseCode != BillingClient.BillingResponseCode.OK) {
- failure = Throwable("Query failed with code: ${billingResult.responseCode}")
- } else {
- inappChange = purchaseManager.handleInAppPurchases(purchases, clearIfNotFound = true, acknowledgePurchase = acknowledge)
- }
- }
- onQueryComplete()
+ try {
+ synchronized(lock) {
+ if (billingResult.responseCode != BillingClient.BillingResponseCode.OK) {
+ failure = Throwable("Query failed with code: ${billingResult.responseCode}")
+ } else {
+ inappChange = purchaseManager.handleInAppPurchases(purchases, clearIfNotFound = true, acknowledgePurchase = acknowledge)
+ }
+ }
+ } catch (t: Throwable) {
+ Timber.tag("PurchaseRefreshCoordinator").e(t, "INAPP callback failed")
+ synchronized(lock) { failure = t }
+ } finally {
+ onQueryComplete()
+ }
}
@@
billingClient.queryPurchasesAsync(subsParams) { billingResult: BillingResult, purchases: List<Purchase> ->
- synchronized(lock) {
- if (billingResult.responseCode != BillingClient.BillingResponseCode.OK) {
- failure = Throwable("Query failed with code: ${billingResult.responseCode}")
- } else {
- subsChange = purchaseManager.handleSubscriptionPurchases(purchases, clearIfNotFound = true, acknowledgePurchase = acknowledge)
- }
- }
- onQueryComplete()
+ try {
+ synchronized(lock) {
+ if (billingResult.responseCode != BillingClient.BillingResponseCode.OK) {
+ failure = Throwable("Query failed with code: ${billingResult.responseCode}")
+ } else {
+ subsChange = purchaseManager.handleSubscriptionPurchases(purchases, clearIfNotFound = true, acknowledgePurchase = acknowledge)
+ }
+ }
+ } catch (t: Throwable) {
+ Timber.tag("PurchaseRefreshCoordinator").e(t, "SUBS callback failed")
+ synchronized(lock) { failure = t }
+ } finally {
+ onQueryComplete()
+ }
}📝 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
| val inappParams = QueryPurchasesParams.newBuilder() | |
| .setProductType(BillingClient.ProductType.INAPP) | |
| .build() | |
| billingClient.queryPurchasesAsync(inappParams) { billingResult: BillingResult, purchases: List<Purchase> -> | |
| synchronized(lock) { | |
| if (billingResult.responseCode != BillingClient.BillingResponseCode.OK) { | |
| failure = Throwable("Query failed with code: ${billingResult.responseCode}") | |
| } else { | |
| inappChange = purchaseManager.handleInAppPurchases(purchases, clearIfNotFound = true, acknowledgePurchase = acknowledge) | |
| } | |
| } | |
| onQueryComplete() | |
| } | |
| val subsParams = QueryPurchasesParams.newBuilder() | |
| .setProductType(BillingClient.ProductType.SUBS) | |
| .build() | |
| billingClient.queryPurchasesAsync(subsParams) { billingResult: BillingResult, purchases: List<Purchase> -> | |
| synchronized(lock) { | |
| if (billingResult.responseCode != BillingClient.BillingResponseCode.OK) { | |
| failure = Throwable("Query failed with code: ${billingResult.responseCode}") | |
| } else { | |
| subsChange = purchaseManager.handleSubscriptionPurchases(purchases, clearIfNotFound = true, acknowledgePurchase = acknowledge) | |
| } | |
| } | |
| onQueryComplete() | |
| } | |
| val inappParams = QueryPurchasesParams.newBuilder() | |
| .setProductType(BillingClient.ProductType.INAPP) | |
| .build() | |
| billingClient.queryPurchasesAsync(inappParams) { billingResult: BillingResult, purchases: List<Purchase> -> | |
| try { | |
| synchronized(lock) { | |
| if (billingResult.responseCode != BillingClient.BillingResponseCode.OK) { | |
| failure = Throwable("Query failed with code: ${billingResult.responseCode}") | |
| } else { | |
| inappChange = purchaseManager.handleInAppPurchases(purchases, clearIfNotFound = true, acknowledgePurchase = acknowledge) | |
| } | |
| } | |
| } catch (t: Throwable) { | |
| Timber.tag("PurchaseRefreshCoordinator").e(t, "INAPP callback failed") | |
| synchronized(lock) { failure = t } | |
| } finally { | |
| onQueryComplete() | |
| } | |
| } | |
| val subsParams = QueryPurchasesParams.newBuilder() | |
| .setProductType(BillingClient.ProductType.SUBS) | |
| .build() | |
| billingClient.queryPurchasesAsync(subsParams) { billingResult: BillingResult, purchases: List<Purchase> -> | |
| try { | |
| synchronized(lock) { | |
| if (billingResult.responseCode != BillingClient.BillingResponseCode.OK) { | |
| failure = Throwable("Query failed with code: ${billingResult.responseCode}") | |
| } else { | |
| subsChange = purchaseManager.handleSubscriptionPurchases(purchases, clearIfNotFound = true, acknowledgePurchase = acknowledge) | |
| } | |
| } | |
| } catch (t: Throwable) { | |
| Timber.tag("PurchaseRefreshCoordinator").e(t, "SUBS callback failed") | |
| synchronized(lock) { failure = t } | |
| } finally { | |
| onQueryComplete() | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseRefreshCoordinator.kt`
around lines 82 - 107, The BillingClient async callbacks can throw and prevent
onQueryComplete() from running; wrap the bodies of both
billingClient.queryPurchasesAsync lambdas in a try/catch/finally so any
exception from purchaseManager.handleInAppPurchases,
purchaseManager.handleSubscriptionPurchases or the acknowledge callback is
caught and logged (or captured into failure) and onQueryComplete() is called
from finally; ensure you set failure when an exception occurs and then call
onQueryComplete() in the finally block so queriesCompleted always progresses to
totalQueries.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
presentation/src/main/res/values/strings.xml (1)
730-730: Mark this string alias as non-translatable.This mirrors alias usage elsewhere in the file and keeps the localized placeholder owned by
screen_license_check_trial_active_info.🌐 Proposed resource cleanup
- <string name="screen_settings_license_summary_trial_expires">@string/screen_license_check_trial_active_info</string> + <string name="screen_settings_license_summary_trial_expires" translatable="false">@string/screen_license_check_trial_active_info</string>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/res/values/strings.xml` at line 730, The string resource screen_settings_license_summary_trial_expires is an alias and should be marked non-translatable; update the <string name="screen_settings_license_summary_trial_expires"> element so it includes translatable="false" and still references `@string/screen_license_check_trial_active_info`, mirroring other alias entries so the localized content remains owned by screen_license_check_trial_active_info.presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt (1)
69-84: Clean up the emptyTextWatchercallbacks.Detekt flags Lines 70-71. Expression-bodied no-ops keep the required interface methods without empty blocks.
♻️ Proposed cleanup
binding.licenseContent.etLicense.addTextChangedListener(object : TextWatcher { - override fun beforeTextChanged(s: CharSequence?, start: Int, count: Int, after: Int) {} - override fun onTextChanged(s: CharSequence?, start: Int, before: Int, count: Int) {} + override fun beforeTextChanged(s: CharSequence?, start: Int, count: Int, after: Int) = Unit + override fun onTextChanged(s: CharSequence?, start: Int, before: Int, count: Int) = Unit override fun afterTextChanged(s: Editable?) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt` around lines 69 - 84, Replace the empty override blocks in the anonymous TextWatcher on binding.licenseContent.etLicense by using expression-bodied no-op implementations for beforeTextChanged and onTextChanged to satisfy Detekt; keep the existing afterTextChanged implementation intact (which handles debounceRunnable, debounceHandler.postDelayed, listener?.onLicenseTextChanged(text), and DEBOUNCE_DELAY_MS) so only the two empty methods are condensed to expression bodies while preserving the debounce logic and references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt`:
- Around line 63-68: The current setupLicenseEntryUi calls
licenseContentViewBinder.bindInitialLicenseEntryWithTrialLayout(), which hides
purchaseOptionsGroup and therefore also hides btnTrial/rowTrial (they live under
that group), so the trial CTA never becomes visible; fix by ensuring the trial
row is explicitly shown after binding or by moving the visibility logic into
LicenseContentViewBinder: update
LicenseContentViewBinder.bindInitialLicenseEntryWithTrialLayout (or add a new
method) to set rowTrial and/or btnTrial visibility to VISIBLE while keeping
other purchase rows hidden, or in setupLicenseEntryUi call
binding.licenseContent.rowTrial.visibility = View.VISIBLE (and keep
binding.licenseContent.btnPurchase/GROUP hidden) so the trial button is actually
displayed and functional.
In `@presentation/src/main/res/values/strings.xml`:
- Line 557: The string resource dialog_enter_license_content is worded
inconsistently; replace the mixed passive/imperative phrasing with a single,
clear sentence—e.g., use an imperative or a declarative form such as "Purchase a
license at https://cryptomator.org/android/ or enter your existing Android
License Key"—by updating the value of string name="dialog_enter_license_content"
accordingly.
---
Nitpick comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt`:
- Around line 69-84: Replace the empty override blocks in the anonymous
TextWatcher on binding.licenseContent.etLicense by using expression-bodied no-op
implementations for beforeTextChanged and onTextChanged to satisfy Detekt; keep
the existing afterTextChanged implementation intact (which handles
debounceRunnable, debounceHandler.postDelayed,
listener?.onLicenseTextChanged(text), and DEBOUNCE_DELAY_MS) so only the two
empty methods are condensed to expression bodies while preserving the debounce
logic and references.
In `@presentation/src/main/res/values/strings.xml`:
- Line 730: The string resource screen_settings_license_summary_trial_expires is
an alias and should be marked non-translatable; update the <string
name="screen_settings_license_summary_trial_expires"> element so it includes
translatable="false" and still references
`@string/screen_license_check_trial_active_info`, mirroring other alias entries so
the localized content remains owned by screen_license_check_trial_active_info.
🪄 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: 6c17207e-e7fb-4cdb-8a18-ebde42efef82
📒 Files selected for processing (9)
buildsystem/dependencies.gradlepresentation/build.gradlepresentation/src/androidTestApkstore/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivityTrialScopeGuardTest.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.ktpresentation/src/main/java/org/cryptomator/presentation/ui/layout/LicenseContentViewBinder.ktpresentation/src/main/res/layout/view_license_check_content.xmlpresentation/src/main/res/values/strings.xmlpresentation/src/test/java/org/cryptomator/presentation/ui/layout/LicenseContentViewBinderTest.kt
✅ Files skipped from review due to trivial changes (2)
- presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt
- presentation/src/main/res/layout/view_license_check_content.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- presentation/src/main/java/org/cryptomator/presentation/ui/layout/LicenseContentViewBinder.kt
Comment on lines
+63
to
+68
| private fun setupLicenseEntryUi() { | ||
| licenseContentViewBinder.bindInitialLicenseEntryWithTrialLayout() | ||
| binding.licenseContent.btnTrial.text = getString(R.string.screen_welcome_trial_button) | ||
| binding.licenseContent.btnTrial.setOnClickListener { listener?.onStartTrial() } | ||
| binding.licenseContent.tvLicenseLink.setOnClickListener { listener?.onOpenLicenseLink() } | ||
| binding.licenseContent.btnPurchase.visibility = View.GONE |
There was a problem hiding this comment.
Make the non-freemium trial CTA visible.
bindInitialLicenseEntryWithTrialLayout() hides purchaseOptionsGroup, but btnTrial/rowTrial live under that group in view_license_check_content.xml. As written, Lines 65-66 configure a trial button that remains hidden, so apkstore/fdroid/lite users cannot start the surfaced welcome trial from this screen.
Move the “license entry + trial row only” visibility into LicenseContentViewBinder, or explicitly show the trial row while keeping non-trial purchase rows hidden.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt`
around lines 63 - 68, The current setupLicenseEntryUi calls
licenseContentViewBinder.bindInitialLicenseEntryWithTrialLayout(), which hides
purchaseOptionsGroup and therefore also hides btnTrial/rowTrial (they live under
that group), so the trial CTA never becomes visible; fix by ensuring the trial
row is explicitly shown after binding or by moving the visibility logic into
LicenseContentViewBinder: update
LicenseContentViewBinder.bindInitialLicenseEntryWithTrialLayout (or add a new
method) to set rowTrial and/or btnTrial visibility to VISIBLE while keeping
other purchase rows hidden, or in setupLicenseEntryUi call
binding.licenseContent.rowTrial.visibility = View.VISIBLE (and keep
binding.licenseContent.btnPurchase/GROUP hidden) so the trial button is actually
displayed and functional.
|
|
||
| <string name="dialog_enter_license_title">Provide a valid license</string> | ||
| <string name="dialog_enter_license_content">We detected that you installed Cryptomator without using Google Play Store. Provide a valid license, which can be purchased on <a href="https://cryptomator.org/android/">https://cryptomator.org/android/</a></string> | ||
| <string name="dialog_enter_license_content">A license can be purchased on <a href="https://cryptomator.org/android/">https://cryptomator.org/android/</a> or enter your existing Android License Key</string> |
There was a problem hiding this comment.
Tighten the license-entry sentence.
Current wording mixes passive voice with an imperative: “A license can be purchased … or enter …”.
✏️ Proposed copy fix
- <string name="dialog_enter_license_content">A license can be purchased on <a href="https://cryptomator.org/android/">https://cryptomator.org/android/</a> or enter your existing Android License Key</string>
+ <string name="dialog_enter_license_content">Purchase a license at <a href="https://cryptomator.org/android/">https://cryptomator.org/android/</a> or enter your existing Android license key.</string>📝 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
| <string name="dialog_enter_license_content">A license can be purchased on <a href="https://cryptomator.org/android/">https://cryptomator.org/android/</a> or enter your existing Android License Key</string> | |
| <string name="dialog_enter_license_content">Purchase a license at <a href="https://cryptomator.org/android/">https://cryptomator.org/android/</a> or enter your existing Android license key.</string> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@presentation/src/main/res/values/strings.xml` at line 557, The string
resource dialog_enter_license_content is worded inconsistently; replace the
mixed passive/imperative phrasing with a single, clear sentence—e.g., use an
imperative or a declarative form such as "Purchase a license at
https://cryptomator.org/android/ or enter your existing Android License Key"—by
updating the value of string name="dialog_enter_license_content" accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@presentation/build.gradle`:
- Around line 219-224: The addToFlavors invocation that configures googleFlavors
with dependencies.googleApiServicesDrive contains a redundant/misnamed exclude
entry ("exclude module: 'googlehttpclient'"); remove that line so only the
correct excludes remain (e.g., 'guava-jdk5', 'httpclient', and the correctly
named group/module "com.google.http-client":"google-http-client") in the
addToFlavors block to avoid a silent no-op and clarify intent.
🪄 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: e9695757-c15c-4233-99ab-3c8c35af5d51
📒 Files selected for processing (2)
buildsystem/dependencies.gradlepresentation/build.gradle
✅ Files skipped from review due to trivial changes (1)
- buildsystem/dependencies.gradle
Comment on lines
+219
to
224
| addToFlavors(googleFlavors, dependencies.googleApiServicesDrive) { | ||
| exclude module: 'guava-jdk5' | ||
| exclude module: 'httpclient' | ||
| exclude module: "google-http-client" | ||
| exclude module: "jetified-google-http-client" | ||
| exclude module: 'googlehttpclient' | ||
| exclude group: "com.google.http-client", module: "google-http-client" | ||
| exclude group: "com.google.http-client", module: "jetified-google-http-client" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm no transitive dep actually uses the bare 'googlehttpclient' artifact name,
# and that 'google-http-client' is the canonical module name referenced elsewhere.
rg -nP --type=gradle -C1 "googlehttpclient|google-http-client"Repository: cryptomator/android
Length of output: 2898
Remove the redundant and misnamed exclude: 'googlehttpclient' is not a real module coordinate.
Line 222 contains exclude module: 'googlehttpclient' (without hyphens), but the actual module name is google-http-client with hyphens, which is already correctly excluded on line 223. The bare googlehttpclient exclude is a silent no-op and should be removed to clarify intent.
🧹 Proposed fix
addToFlavors(googleFlavors, dependencies.googleApiServicesDrive) {
exclude module: 'guava-jdk5'
exclude module: 'httpclient'
- exclude module: 'googlehttpclient'
exclude group: "com.google.http-client", module: "google-http-client"
}📝 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
| addToFlavors(googleFlavors, dependencies.googleApiServicesDrive) { | |
| exclude module: 'guava-jdk5' | |
| exclude module: 'httpclient' | |
| exclude module: "google-http-client" | |
| exclude module: "jetified-google-http-client" | |
| exclude module: 'googlehttpclient' | |
| exclude group: "com.google.http-client", module: "google-http-client" | |
| exclude group: "com.google.http-client", module: "jetified-google-http-client" | |
| } | |
| addToFlavors(googleFlavors, dependencies.googleApiServicesDrive) { | |
| exclude module: 'guava-jdk5' | |
| exclude module: 'httpclient' | |
| exclude group: "com.google.http-client", module: "google-http-client" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@presentation/build.gradle` around lines 219 - 224, The addToFlavors
invocation that configures googleFlavors with
dependencies.googleApiServicesDrive contains a redundant/misnamed exclude entry
("exclude module: 'googlehttpclient'"); remove that line so only the correct
excludes remain (e.g., 'guava-jdk5', 'httpclient', and the correctly named
group/module "com.google.http-client":"google-http-client") in the addToFlavors
block to avoid a silent no-op and clarify intent.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt (2)
6-7: UsedoAfterTextChangedto avoid emptyTextWatchermethods.Lines 72–73 trigger the
empty-blocks.EmptyFunctionBlockdetekt rule. Sinceandroidx.core:core-ktxis already a project dependency, replace the TextWatcher anonymous class with a singledoAfterTextChangedcallback to remove the empty implementations and satisfy detekt.♻️ Proposed cleanup
-import android.text.Editable -import android.text.TextWatcher +import androidx.core.widget.doAfterTextChanged @@ - binding.licenseContent.etLicense.addTextChangedListener(object : TextWatcher { - override fun beforeTextChanged(s: CharSequence?, start: Int, count: Int, after: Int) {} - override fun onTextChanged(s: CharSequence?, start: Int, before: Int, count: Int) {} - override fun afterTextChanged(s: Editable?) { - debounceRunnable?.let { debounceHandler.removeCallbacks(it) } - debounceRunnable = null - val text = s?.toString() - if (!text.isNullOrBlank()) { - val runnable = Runnable { listener?.onLicenseTextChanged(text) } - debounceRunnable = runnable - debounceHandler.postDelayed(runnable, DEBOUNCE_DELAY_MS) - } else { - listener?.onLicenseTextChanged(null) - } - } - }) + binding.licenseContent.etLicense.doAfterTextChanged { s -> + debounceRunnable?.let { debounceHandler.removeCallbacks(it) } + debounceRunnable = null + val text = s?.toString() + if (!text.isNullOrBlank()) { + val runnable = Runnable { listener?.onLicenseTextChanged(text) } + debounceRunnable = runnable + debounceHandler.postDelayed(runnable, DEBOUNCE_DELAY_MS) + } else { + listener?.onLicenseTextChanged(null) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt` around lines 6 - 7, Replace the anonymous TextWatcher in WelcomeLicenseFragment (the instance added to the license EditText) with the androidx.core.widget.doAfterTextChanged extension: remove the android.text.Editable and android.text.TextWatcher imports and the object : TextWatcher { ... } block, then call licenseEditText.doAfterTextChanged { /* existing on-change logic */ } to keep only the needed callback; import androidx.core.widget.doAfterTextChanged if necessary.
19-23: Remove the unusedonSkipLicense()callback from the listener contract.
onSkipLicense()is declared in theListenerinterface and overridden inWelcomeActivity, but this fragment never invokes it. The layout contains no skip button or skip-related UI element to trigger this callback. Either wire the callback to a UI element or remove the unused contract member.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt` around lines 19 - 23, The Listener interface declares an unused onSkipLicense() method; remove that method from the Listener interface in WelcomeLicenseFragment (delete the line with fun onSkipLicense()) and then remove the corresponding override/implementation in WelcomeActivity (and any other classes implementing this Listener) so the contract and implementations stay consistent; ensure no leftover references or imports remain and run a quick build to catch remaining usages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt`:
- Around line 6-7: Replace the anonymous TextWatcher in WelcomeLicenseFragment
(the instance added to the license EditText) with the
androidx.core.widget.doAfterTextChanged extension: remove the
android.text.Editable and android.text.TextWatcher imports and the object :
TextWatcher { ... } block, then call licenseEditText.doAfterTextChanged { /*
existing on-change logic */ } to keep only the needed callback; import
androidx.core.widget.doAfterTextChanged if necessary.
- Around line 19-23: The Listener interface declares an unused onSkipLicense()
method; remove that method from the Listener interface in WelcomeLicenseFragment
(delete the line with fun onSkipLicense()) and then remove the corresponding
override/implementation in WelcomeActivity (and any other classes implementing
this Listener) so the contract and implementations stay consistent; ensure no
leftover references or imports remain and run a quick build to catch remaining
usages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aa1f3e40-4798-4515-9845-e71548c8cdad
📒 Files selected for processing (1)
presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt
coderabbitai bot
added a commit
that referenced
this pull request
Apr 19, 2026
Docstrings generation was requested by @tobihagemann. * #630 (comment) The following files were modified: * `data/src/main/java/org/cryptomator/data/db/DatabaseUpgrades.java` * `data/src/main/java/org/cryptomator/data/db/Upgrade13To14.kt` * `data/src/main/java/org/cryptomator/data/db/entities/UpdateCheckEntity.java` * `data/src/main/java/org/cryptomator/data/repository/HubRepositoryImpl.java` * `data/src/main/java/org/cryptomator/data/repository/UpdateCheckRepositoryImpl.java` * `domain/src/main/java/org/cryptomator/domain/Vault.java` * `domain/src/main/java/org/cryptomator/domain/repository/HubRepository.kt` * `domain/src/main/java/org/cryptomator/domain/repository/UpdateCheckRepository.java` * `domain/src/main/java/org/cryptomator/domain/usecases/DoLicenseCheck.java` * `domain/src/main/java/org/cryptomator/domain/usecases/vault/UnlockHubVault.java` * `presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.kt` * `presentation/src/main/java/org/cryptomator/presentation/di/component/ActivityComponent.java` * `presentation/src/main/java/org/cryptomator/presentation/intent/LicenseCheckIntent.java` * `presentation/src/main/java/org/cryptomator/presentation/intent/TextEditorIntent.java` * `presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt` * `presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseStateOrchestrator.kt` * `presentation/src/main/java/org/cryptomator/presentation/presenter/BaseLicensePresenter.kt` * `presentation/src/main/java/org/cryptomator/presentation/presenter/BrowseFilesPresenter.kt` * `presentation/src/main/java/org/cryptomator/presentation/presenter/ChooseCloudServicePresenter.kt` * `presentation/src/main/java/org/cryptomator/presentation/presenter/CloudSettingsPresenter.kt` * `presentation/src/main/java/org/cryptomator/presentation/presenter/SettingsPresenter.kt` * `presentation/src/main/java/org/cryptomator/presentation/presenter/SharedFilesPresenter.kt` * `presentation/src/main/java/org/cryptomator/presentation/presenter/VaultListPresenter.kt` * `presentation/src/main/java/org/cryptomator/presentation/presenter/WelcomePresenter.kt` * `presentation/src/main/java/org/cryptomator/presentation/service/ProductInfo.kt` * `presentation/src/main/java/org/cryptomator/presentation/service/PurchaseRevokedReason.kt` * `presentation/src/main/java/org/cryptomator/presentation/service/PurchaseRevokedToastObserver.kt` * `presentation/src/main/java/org/cryptomator/presentation/service/RestoreOutcomeHandler.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/activity/BrowseFilesActivity.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/activity/SharedFilesActivity.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/activity/TextEditorActivity.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/activity/VaultListActivity.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/activity/view/SharedFilesView.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/activity/view/UpdateLicenseView.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/activity/view/WelcomeView.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/dialog/NoFullVersionDialog.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/dialog/RestoreFailedDialog.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/dialog/RestoreSuccessfulDialog.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/dialog/TrialExpiredDialog.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/fragment/SettingsFragment.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/fragment/SharedFilesFragment.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/fragment/TextEditorFragment.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeIntroFragment.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeNotificationsFragment.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeScreenLockFragment.kt` * `presentation/src/main/java/org/cryptomator/presentation/ui/layout/LicenseContentViewBinder.kt` * `presentation/src/nonplaystoreiap/java/org/cryptomator/presentation/service/IapBillingService.kt` * `presentation/src/playstoreiap/java/org/cryptomator/presentation/service/IapBillingService.kt` * `presentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseManager.kt` * `presentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseRefreshCoordinator.kt` * `util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt`
This was referenced Apr 19, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (1)
presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt (1)
71-86: Clean up the detektEmptyFunctionBlockwarnings.The no-op
TextWatchercallbacks on lines 72-73 are valid Android boilerplate that detekt flags. Since AndroidX Core KTX is already available in the presentation module, prefer refactoring todoAfterTextChangedto eliminate the warnings.♻️ Refactor using doAfterTextChanged
+import androidx.core.widget.doAfterTextChanged import android.content.Context import android.os.Handler import android.os.Looper -import android.text.Editable -import android.text.TextWatcher import android.view.View- binding.licenseContent.etLicense.addTextChangedListener(object : TextWatcher { - override fun beforeTextChanged(s: CharSequence?, start: Int, count: Int, after: Int) {} - override fun onTextChanged(s: CharSequence?, start: Int, before: Int, count: Int) {} - override fun afterTextChanged(s: Editable?) { - debounceRunnable?.let { debounceHandler.removeCallbacks(it) } - debounceRunnable = null - val text = s?.toString() - if (!text.isNullOrBlank()) { - val runnable = Runnable { listener?.onLicenseTextChanged(text) } - debounceRunnable = runnable - debounceHandler.postDelayed(runnable, DEBOUNCE_DELAY_MS) - } else { - listener?.onLicenseTextChanged(null) - } - } - }) + binding.licenseContent.etLicense.doAfterTextChanged { editable -> + debounceRunnable?.let { debounceHandler.removeCallbacks(it) } + debounceRunnable = null + val text = editable?.toString() + if (!text.isNullOrBlank()) { + val runnable = Runnable { listener?.onLicenseTextChanged(text) } + debounceRunnable = runnable + debounceHandler.postDelayed(runnable, DEBOUNCE_DELAY_MS) + } else { + listener?.onLicenseTextChanged(null) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt` around lines 71 - 86, Replace the anonymous TextWatcher on binding.licenseContent.etLicense with the AndroidX doAfterTextChanged extension to remove empty callback methods: remove the object : TextWatcher { ... } block and use etLicense.doAfterTextChanged { s -> ... } keeping the existing debounce logic (debounceRunnable, debounceHandler, DEBOUNCE_DELAY_MS) and the listener?.onLicenseTextChanged(...) calls unchanged so behavior is identical while eliminating the EmptyFunctionBlock detekt warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt`:
- Around line 71-86: Replace the anonymous TextWatcher on
binding.licenseContent.etLicense with the AndroidX doAfterTextChanged extension
to remove empty callback methods: remove the object : TextWatcher { ... } block
and use etLicense.doAfterTextChanged { s -> ... } keeping the existing debounce
logic (debounceRunnable, debounceHandler, DEBOUNCE_DELAY_MS) and the
listener?.onLicenseTextChanged(...) calls unchanged so behavior is identical
while eliminating the EmptyFunctionBlock detekt warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fcb43829-1e38-40fa-96b4-e15bc06bd4fc
📒 Files selected for processing (1)
presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt
There was a problem hiding this comment.
🧹 Nitpick comments (1)
presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt (1)
265-267: Guard external browser launch againstActivityNotFoundException.On devices without a browser/handler for
ACTION_VIEWhttps URIs (stripped AOSP, managed profiles, kiosk setups), this call will crash the onboarding flow. Consider wrapping intry/catchand surfacing a toast or falling back gracefully.🛡️ Proposed fix
override fun onOpenLicenseLink() { - startActivity(Intent(Intent.ACTION_VIEW, Uri.parse("https://cryptomator.org/android/"))) + try { + startActivity(Intent(Intent.ACTION_VIEW, Uri.parse("https://cryptomator.org/android/"))) + } catch (e: android.content.ActivityNotFoundException) { + Toast.makeText(this, R.string.error_no_browser_found, Toast.LENGTH_SHORT).show() + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt` around lines 265 - 267, The onOpenLicenseLink method currently calls startActivity with an ACTION_VIEW https Uri which can throw ActivityNotFoundException on devices without a browser; wrap the startActivity call in a try/catch for ActivityNotFoundException inside onOpenLicenseLink, and in the catch show a user-friendly fallback (e.g., Toast.makeText(...) or a dialog) or silently handle it so the onboarding flow doesn't crash; keep using Intent.ACTION_VIEW and Uri.parse("https://cryptomator.org/android/") but ensure the exception handling prevents the crash and provides graceful feedback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt`:
- Around line 265-267: The onOpenLicenseLink method currently calls
startActivity with an ACTION_VIEW https Uri which can throw
ActivityNotFoundException on devices without a browser; wrap the startActivity
call in a try/catch for ActivityNotFoundException inside onOpenLicenseLink, and
in the catch show a user-friendly fallback (e.g., Toast.makeText(...) or a
dialog) or silently handle it so the onboarding flow doesn't crash; keep using
Intent.ACTION_VIEW and Uri.parse("https://cryptomator.org/android/") but ensure
the exception handling prevents the crash and provides graceful feedback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 820d9fd3-7e0f-4a1a-a8b1-c904082d0645
📒 Files selected for processing (2)
presentation/src/main/java/org/cryptomator/presentation/presenter/VaultListPresenter.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- presentation/src/main/java/org/cryptomator/presentation/presenter/VaultListPresenter.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
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
Introduces a freemium distribution model via a new
playstoreiapbuild flavor. Free users get read-only vault access with a 30-day full-featured trial on first install. Write operations (create vault, upload, create folder/text file, rename, move, delete) are gated behindLicenseEnforcerand redirect to a purchase screen when the trial expires.Key changes:
playstoreiapflavor across all modules, with a realIapBillingServicebacked by Google Play Billing Library 8.2.1. Other flavors get a no-op stub.LicenseEnforcerevaluates write access from three sources: paid license token, active subscription, or active trial. Once trial expiry is observed, a stickytrialExpiredflag latches inSharedPreferencesto block clock-rollback reactivation. Hub vaults use their own subscription status.PurchaseManagerhandles purchase verification, token persistence, and acknowledgement with TOCTOU-safe state transitions.LicenseStateOrchestratordrives reactive UI updates viaSharedPreferenceschange listeners, used by bothWelcomeActivityandLicenseCheckActivity.LicenseContentViewBinderextracts shared purchase/trial UI into a reusable binder for the onboarding welcome flow and the standalone IAP screen.apkstore/fdroid/litealongside license-key entry.LicenseCheckActivity(upsell) keeps its scope guard and never shows the trial row.WelcomeActivitywith a ViewPager2-based onboarding flow (intro, license, notifications, and screen lock when the device isn't already secured) that replaces the old dialog-based first-launch experience for IAP builds.BrowseFilesPresenter,SharedFilesPresenter,VaultListPresenter, andTextEditorActivity, redirecting blocked write actions to the IAP screen with a locked-action header.Vaultmodel extended withhubSubscriptionState.LicenseEnforcerTestcovering trial lifecycle, paid license, subscription, Hub vaults, and flavor-based bypass. Additional tests forPurchaseManager,ProductInfo,DoLicenseCheck,UnlockHubVault, and vault list freemium behavior.LicenseCheckIntent,TextEditorIntent) for type-safe intent extras, replacing raw companion constants.wrap_contentfix for trial-expired dialog scroll.State Machine
stateDiagram-v2 [*] --> ReadOnly: fresh install (non-premium flavors) [*] --> FullAccess: premium flavor ReadOnly --> Trial: startTrial() Trial --> FullAccess: purchase / subscribe Trial --> ReadOnly: trial expires ReadOnly --> FullAccess: purchase / subscribe FullAccess --> ReadOnly: subscription lapses (no lifetime license)Test Plan
:domain:testDebugUnitTest,:presentation:testPlaystoreiapDebugUnitTestassemblePlaystoreDebug,assemblePlaystoreiapDebug,assembleApkstoreDebug,assembleFdroidDebug,assembleLiteDebugplaystoreiap: onboarding flow shows license step, trial starts, write ops workapkstore: welcome License page shows trial button alongside license entry; tapping starts trial and auto-advances