-
-
Notifications
You must be signed in to change notification settings - Fork 37
Show purchase status in settings #426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughReorders settings sections and adds a purchase status section when the user does not have full access. Introduces PurchaseStatusCell and PurchaseStatusCellViewModel to display an icon, title, and subtitle with Combine bindings. SettingsViewModel now conditionally builds sections (inserting purchaseStatusSection based on subscription/unlock state), exposes hasFullAccess, and provides a footer for the about section when full access exists. Localization adds keys for full-version footer, trial expiration date, and free-tier subtitle. SettingsViewController handles selection of PurchaseStatusCellViewModel by triggering the unlock flow. Project file updated to include the two new Swift sources. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Cryptomator/Settings/SettingsViewModel.swift(1 hunks)SharedResources/en.lproj/Localizable.strings(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Cryptomator/Settings/SettingsViewModel.swift (5)
Cryptomator/Common/Cells/ButtonCellViewModel.swift (1)
createDisclosureButton(19-21)Cryptomator/Settings/SettingsCoordinator.swift (3)
showAbout(31-35)showManageSubscriptions(85-97)showUnlockFullVersion(79-83)CryptomatorCommon/Sources/CryptomatorCommonCore/LocalizedString.swift (1)
getValue(12-22)Cryptomator/Purchase/IAPViewController.swift (1)
restorePurchase(277-290)Cryptomator/Purchase/PurchaseViewController.swift (1)
restorePurchase(55-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and test (freemium)
🔇 Additional comments (2)
SharedResources/en.lproj/Localizable.strings (1)
216-216: LGTM! Localization key follows conventions and uses consistent terminology.The new localization string is well-formed and appropriately placed within the Settings section. The key name "fullVersionStatus" clearly indicates its purpose, and "Full Version" matches the terminology used throughout the app.
Cryptomator/Settings/SettingsViewModel.swift (1)
79-84: Status cell implementation is correct.The localization key
"settings.fullVersionStatus"exists in the English localization file with the value "Full Version". The cell implementation is appropriate, usingBindableTableViewCellViewModelwithselectionStyle: .noneandaccessoryType: .checkmarkto correctly represent a non-interactive status indicator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
Cryptomator/Settings/SettingsViewController.swift (1)
103-141: Simplify didSelectRow logic by handlingPurchaseStatusCellViewModelup frontFunctionally this works (purchase status row deselects and triggers
showUnlockFullVersion()), but you now:
- Call
dataSource?.itemIdentifier(for:)twice, and- Potentially deselect the same row twice for
PurchaseStatusCellViewModel.You can keep behavior while tightening the code by handling the purchase‑status case first and reusing the same item identifier, e.g.:
override func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) { - let cellViewModel = dataSource?.itemIdentifier(for: indexPath) as? ButtonCellViewModel<SettingsButtonAction> + let item = dataSource?.itemIdentifier(for: indexPath) + + if item is PurchaseStatusCellViewModel { + tableView.deselectRow(at: indexPath, animated: true) + coordinator?.showUnlockFullVersion() + return + } + + let cellViewModel = item as? ButtonCellViewModel<SettingsButtonAction> @@ - switch cellViewModel?.action { + switch cellViewModel?.action { // existing cases… } - - if dataSource?.itemIdentifier(for: indexPath) is PurchaseStatusCellViewModel { - tableView.deselectRow(at: indexPath, animated: true) - coordinator?.showUnlockFullVersion() - } }Keeps the UX identical while reducing lookups and local complexity in this already large delegate method.
Cryptomator/Settings/PurchaseStatusCellViewModel.swift (1)
19-23: Consider using non-optional Bindable types for title and subtitle.The initializer accepts non-optional
Stringparameters fortitleandsubtitle(line 19), but wraps them inBindable<String?>(lines 16-17). Since these values are always present, usingBindable<String>instead would eliminate unnecessary optionality and make the code more type-safe.Apply this diff to use non-optional Bindable types:
- let title: Bindable<String?> - let subtitle: Bindable<String?> + let title: Bindable<String> + let subtitle: Bindable<String>Cryptomator/Settings/SettingsViewModel.swift (1)
103-118: Cache the DateFormatter to improve performance.The
DateFormatteris created on every access topurchaseStatusCellViewModel(lines 106-108). SinceDateFormattercreation is relatively expensive and this computed property is accessed whenever sections are rebuilt, consider caching the formatter as a static property or lazy var.Add a cached date formatter at the class level:
+ private static let expirationDateFormatter: DateFormatter = { + let formatter = DateFormatter() + formatter.dateStyle = .medium + formatter.timeStyle = .none + return formatter + }() + private var purchaseStatusCellViewModel: PurchaseStatusCellViewModel { let subtitle: String if let trialExpirationDate = cryptomatorSettings.trialExpirationDate, trialExpirationDate > Date() { - let dateFormatter = DateFormatter() - dateFormatter.dateStyle = .medium - dateFormatter.timeStyle = .none - subtitle = String(format: LocalizedString.getValue("settings.trial.expirationDate"), dateFormatter.string(from: trialExpirationDate)) + subtitle = String(format: LocalizedString.getValue("settings.trial.expirationDate"), Self.expirationDateFormatter.string(from: trialExpirationDate)) } else { subtitle = LocalizedString.getValue("settings.freeTier.subtitle") }Cryptomator/Settings/PurchaseStatusCell.swift (1)
30-66: LGTM! Layout implementation is clean and accessible.The Auto Layout setup is well-structured with:
- Proper use of layout margins
- Multi-line label support for dynamic type
- Appropriate spacing and sizing
One optional enhancement: consider setting
isAccessibilityElement = trueand providing a combined accessibility label that includes the icon meaning, title, and subtitle for VoiceOver users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Cryptomator.xcodeproj/project.pbxproj(4 hunks)Cryptomator/Settings/PurchaseStatusCell.swift(1 hunks)Cryptomator/Settings/PurchaseStatusCellViewModel.swift(1 hunks)Cryptomator/Settings/SettingsViewController.swift(1 hunks)Cryptomator/Settings/SettingsViewModel.swift(4 hunks)SharedResources/en.lproj/Localizable.strings(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: iammajid
Repo: cryptomator/ios PR: 426
File: Cryptomator/Settings/SettingsViewModel.swift:73-90
Timestamp: 2025-11-11T11:58:54.204Z
Learning: In SettingsViewModel.swift (Cryptomator iOS app), when displaying subscription/purchase status in the aboutSectionElements: subscriptions take precedence over lifetime licenses. If both hasRunningSubscription and fullVersionUnlocked are true (an edge case that shouldn't happen in practice), only the "Manage Subscriptions" button should be shown, not the "Full Version" status cell. This is intentional design behavior.
📚 Learning: 2025-11-11T11:58:54.204Z
Learnt from: iammajid
Repo: cryptomator/ios PR: 426
File: Cryptomator/Settings/SettingsViewModel.swift:73-90
Timestamp: 2025-11-11T11:58:54.204Z
Learning: In SettingsViewModel.swift (Cryptomator iOS app), when displaying subscription/purchase status in the aboutSectionElements: subscriptions take precedence over lifetime licenses. If both hasRunningSubscription and fullVersionUnlocked are true (an edge case that shouldn't happen in practice), only the "Manage Subscriptions" button should be shown, not the "Full Version" status cell. This is intentional design behavior.
Applied to files:
Cryptomator/Settings/PurchaseStatusCellViewModel.swiftCryptomator/Settings/SettingsViewController.swiftCryptomator/Settings/PurchaseStatusCell.swiftCryptomator/Settings/SettingsViewModel.swift
📚 Learning: 2024-10-10T15:32:49.838Z
Learnt from: tobihagemann
Repo: cryptomator/ios PR: 384
File: FileProviderExtensionUI/FileProviderCoordinator.swift:85-86
Timestamp: 2024-10-10T15:32:49.838Z
Learning: In `ReauthenticationViewController.swift`, the `coordinator` property is declared as `weak`, preventing retain cycles with `FileProviderCoordinator`.
Applied to files:
Cryptomator/Settings/SettingsViewController.swift
🧬 Code graph analysis (3)
Cryptomator/Settings/SettingsViewController.swift (2)
Cryptomator/Purchase/PurchaseViewController.swift (2)
tableView(31-41)tableView(43-53)Cryptomator/Settings/SettingsCoordinator.swift (1)
showUnlockFullVersion(79-83)
Cryptomator/Settings/PurchaseStatusCell.swift (1)
Cryptomator/Common/Combine/Publisher+OptionalAssign.swift (1)
assign(13-17)
Cryptomator/Settings/SettingsViewModel.swift (4)
Cryptomator/Common/TableViewModel.swift (1)
getFooterTitle(24-26)CryptomatorCommon/Sources/CryptomatorCommonCore/LocalizedString.swift (1)
getValue(12-22)Cryptomator/Purchase/PremiumManager.swift (3)
trialExpirationDate(50-52)trialExpirationDate(119-127)trialExpirationDate(168-170)Cryptomator/Common/Cells/ButtonCellViewModel.swift (1)
createDisclosureButton(19-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and test (premium)
🔇 Additional comments (6)
Cryptomator.xcodeproj/project.pbxproj (1)
442-443: Purchase status cell + view model are correctly wired into the main app targetThe new
PurchaseStatusCell.swiftandPurchaseStatusCellViewModel.swiftare properly added to:
PBXFileReference(files exist in the Settings folder),PBXBuildFile(compile sources entries),- the
Settingsgroup hierarchy, and- the
Cryptomatortarget’sSourcesbuild phase.No config or target‑membership issues stand out.
Also applies to: 1063-1064, 2044-2045, 2792-2793
SharedResources/en.lproj/Localizable.strings (1)
216-218: LGTM! Clear and well-structured localization strings.The three new localization keys follow the existing naming convention and provide clear, user-friendly messages for the purchase status feature. The date format specifier on line 217 is correctly used.
Cryptomator/Settings/SettingsViewModel.swift (4)
52-77: LGTM! Dynamic section generation logic is clean and correct.The conditional inclusion of the purchase status section based on
hasFullAccessis well-implemented and aligns with the PR objectives.
79-82: LGTM! Footer title logic is correct.The implementation properly returns the footer text only for the about section when the user has full access.
84-86: LGTM! Clean abstraction for access checking.The
hasFullAccesscomputed property provides a clear, reusable check for premium access across the view model.
88-96: LGTM! Correctly removes the "Restore Purchase" button.The about section elements have been properly refactored to remove the "Restore Purchase" button, addressing the feedback from the previous review. The conditional inclusion of "Manage Subscriptions" is appropriate.
Adds a visual indicator for users with a lifetime license in the Settings screen. Users with an active lifetime purchase now see a "Full Version" label with a checkmark, confirming their premium status. Additionally, the "Restore Purchase" button has been removed for subscription users as they already have active premium access.