Add freemium model with 30-day trial, Google Play IAP, and write-access gating#630
Open
tobihagemann wants to merge 60 commits intodevelopfrom
Open
Add freemium model with 30-day trial, Google Play IAP, and write-access gating#630tobihagemann wants to merge 60 commits intodevelopfrom
tobihagemann wants to merge 60 commits intodevelopfrom
Conversation
…ial info, fix subscription state
…itles, rename section to Premium
…eContentViewBinder
…ove double enforcement
![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
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) |
Member
Author
There was a problem hiding this comment.
Could you clarify the intent here? A literal welcomePresenter.finishWithResult(null) would change behavior. Presenter.finishWithResult(null) sets RESULT_CANCELED (Presenter.kt:81), while the existing code sets RESULT_OK. VaultListPresenter.welcomeFlowCompleted() is a @Callback with dispatchResultOkOnly = true (the default), so switching to RESULT_CANCELED would suppress the welcome-complete callback and skip the follow-up prepareView() on return.
Two ways forward:
- Keep the current
setResult(RESULT_OK); finish()inopenVaultList()(no change). - Add a
WelcomePresenter.completeWelcomeFlow()helper that doesactivity().setResult(RESULT_OK); finish()internally and routeopenVaultList()through it, preserving callback semantics while matching the presenter-style.
Which do you prefer?
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
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
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
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
…radle excludes, prune unused strings
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
presentation/build.gradle (1)
70-85:⚠️ Potential issue | 🟡 MinorThe removal of
applicationIdSuffix ".debug"prevents debug and release variants from being installed side‑by‑side on the same device.No other code in the codebase depends on the
.debugsuffix, and the change is isolated, suggesting the removal is intentional. If this is required for IAP/Play Store testing (which typically requires production applicationIds), clarify the intent in the PR description or a code comment; otherwise, consider reinstating the suffix to preserve the standard development workflow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/build.gradle` around lines 70 - 85, The debug build no longer sets applicationIdSuffix ".debug", which prevents installing debug and release variants side‑by‑side; restore applicationIdSuffix ".debug" inside the debug block (the debug build config in presentation/build.gradle) so the debug variant can be installed alongside release, or if removal was intentional for Play/IAP testing add a clear PR description or a code comment in the debug { ... } block explaining why applicationIdSuffix was removed.
🤖 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`:
- Line 718: The string resource entry named
"screen_settings_license_summary_trial_expires" uses a resource reference value
and should be marked translatable="false"; update the <string
name="screen_settings_license_summary_trial_expires"> declaration to include
translatable="false" so translators won't attempt to localize the literal
"@string/screen_license_check_trial_active_info" reference.
---
Outside diff comments:
In `@presentation/build.gradle`:
- Around line 70-85: The debug build no longer sets applicationIdSuffix
".debug", which prevents installing debug and release variants side‑by‑side;
restore applicationIdSuffix ".debug" inside the debug block (the debug build
config in presentation/build.gradle) so the debug variant can be installed
alongside release, or if removal was intentional for Play/IAP testing add a
clear PR description or a code comment in the debug { ... } block explaining why
applicationIdSuffix was removed.
🪄 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: 4e067037-99bd-4126-aa3a-afdd6e709381
📒 Files selected for processing (4)
data/build.gradlepresentation/build.gradlepresentation/src/main/res/values/strings.xmlpresentation/src/playstoreiap/java/org/cryptomator/presentation/service/IapBillingService.kt
| <string name="screen_settings_license_summary_write_access">Write access unlocked!</string> | ||
| <string name="screen_settings_license_summary_tap_to_unlock">Tap to unlock write access</string> | ||
| <string name="screen_settings_license_summary_write_access_mail">Write access for %1$s</string> | ||
| <string name="screen_settings_license_summary_trial_expires">@string/screen_license_check_trial_active_info</string> |
There was a problem hiding this comment.
Mark string-reference value as translatable="false".
The value is a resource reference (@string/screen_license_check_trial_active_info), not user-facing text. Consistent with every other reference-only entry in this file (e.g., line 101), it should be translatable="false" so translators don't try to localize the literal @string/... token.
✏️ Proposed fix
- <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>📝 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="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 718, The string
resource entry named "screen_settings_license_summary_trial_expires" uses a
resource reference value and should be marked translatable="false"; update the
<string name="screen_settings_license_summary_trial_expires"> declaration to
include translatable="false" so translators won't attempt to localize the
literal "@string/screen_license_check_trial_active_info" reference.
Inline observeTrialExpiry latch into evaluateTrialState, route hasActiveTrial and evaluateUiState through the single computation. Halves prefs reads on write-gate checks.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
presentation/src/test/java/org/cryptomator/presentation/licensing/LicenseEnforcerTest.kt (1)
28-492: LGTM — thorough coverage of the new consolidation.Good coverage of the sticky-latch invariants (one-shot write on first past-due observation, idempotence afterwards, no reset on
startTrial), hub-vault fallback paths, and UI state composition. TheassumeTrue(!FlavorConfig.isPremiumFlavor, …)usage correctly skips negative paths on premium builds.Optional nit: the
86400000Lliteral (1 day in ms) appears ~10 times; extracting aprivate companion val ONE_DAY_MS = TimeUnit.DAYS.toMillis(1)would improve readability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/test/java/org/cryptomator/presentation/licensing/LicenseEnforcerTest.kt` around lines 28 - 492, The tests in LicenseEnforcerTest repeat the 86400000L (one day ms) literal; replace these occurrences by adding a private companion val ONE_DAY_MS = TimeUnit.DAYS.toMillis(1) in the LicenseEnforcerTest class and update all uses (e.g., in tests referencing System.currentTimeMillis() +/- 86400000L and comparisons in startTrial/evaluateUiState/hasActiveTrial tests) to use ONE_DAY_MS for clarity and consistency.presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt (1)
109-124: Nit:FlavorConfig.isPremiumFlavorbranch at Line 114 is effectively unreachable.
hasPaidLicense()already returnstrueunconditionally for premium flavors, sohasWriteAccess()at Line 110 would have returned early. The guard is harmless as defense-in-depth, but you could drop it to reduce noise — or keep it explicitly as a safety net with a short comment noting intent.🤖 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/licensing/LicenseEnforcer.kt` around lines 109 - 124, The check for FlavorConfig.isPremiumFlavor inside ensureWriteAccess is redundant because hasWriteAccess already returns true for premium builds via hasPaidLicense; remove the if (FlavorConfig.isPremiumFlavor) { return false } branch from ensureWriteAccess (or alternatively replace it with a brief comment if you prefer a defensive guard), leaving ensureWriteAccess, hasWriteAccess and hasPaidLicense as the single source of truth for premium behavior.
🤖 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/licensing/LicenseEnforcer.kt`:
- Around line 109-124: The check for FlavorConfig.isPremiumFlavor inside
ensureWriteAccess is redundant because hasWriteAccess already returns true for
premium builds via hasPaidLicense; remove the if (FlavorConfig.isPremiumFlavor)
{ return false } branch from ensureWriteAccess (or alternatively replace it with
a brief comment if you prefer a defensive guard), leaving ensureWriteAccess,
hasWriteAccess and hasPaidLicense as the single source of truth for premium
behavior.
In
`@presentation/src/test/java/org/cryptomator/presentation/licensing/LicenseEnforcerTest.kt`:
- Around line 28-492: The tests in LicenseEnforcerTest repeat the 86400000L (one
day ms) literal; replace these occurrences by adding a private companion val
ONE_DAY_MS = TimeUnit.DAYS.toMillis(1) in the LicenseEnforcerTest class and
update all uses (e.g., in tests referencing System.currentTimeMillis() +/-
86400000L and comparisons in startTrial/evaluateUiState/hasActiveTrial tests) to
use ONE_DAY_MS for clarity and consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e95c9f1-d6af-482d-8e72-660675e760fb
📒 Files selected for processing (2)
presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.ktpresentation/src/test/java/org/cryptomator/presentation/licensing/LicenseEnforcerTest.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