Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ protocol PointOfSaleAggregateModelProtocol {
private var cardReaderDisconnection: AnyCancellable?

private let soundPlayer: PointOfSaleSoundPlayerProtocol
private let isLocalCatalogEligible: Bool

private var cancellables: Set<AnyCancellable> = []

Expand All @@ -76,6 +77,18 @@ protocol PointOfSaleAggregateModelProtocol {
_viewStateCoordinator
}

// Track stale sync warning (only relevant when using local catalog)
var isSyncStale: Bool = false
var isStaleSyncWarningDismissed: Bool = false

var showStaleSyncWarning: Bool {
// Only show warning if using local catalog
guard isLocalCatalogEligible else {
return false
}
return isSyncStale && !isStaleSyncWarningDismissed
}

init(entryPointController: POSEntryPointController,
itemsController: PointOfSaleItemsControllerProtocol,
purchasableItemsSearchController: PointOfSaleSearchingItemsControllerProtocol,
Expand All @@ -92,7 +105,8 @@ protocol PointOfSaleAggregateModelProtocol {
soundPlayer: PointOfSaleSoundPlayerProtocol = PointOfSaleSoundPlayer(),
paymentState: PointOfSalePaymentState = .idle,
siteID: Int64,
catalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol? = nil) {
catalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol? = nil,
isLocalCatalogEligible: Bool = false) {
self.entryPointController = entryPointController
self.purchasableItemsController = itemsController
self.purchasableItemsSearchController = purchasableItemsSearchController
Expand All @@ -110,6 +124,7 @@ protocol PointOfSaleAggregateModelProtocol {
self.soundPlayer = soundPlayer
self.siteID = siteID
self.catalogSyncCoordinator = catalogSyncCoordinator
self.isLocalCatalogEligible = isLocalCatalogEligible

publishCardReaderConnectionStatus()
publishPaymentMessages()
Expand Down Expand Up @@ -632,6 +647,28 @@ private extension PointOfSaleAggregateModel {
}
}

// MARK: - Stale Sync Warning
extension PointOfSaleAggregateModel {
var staleSyncThresholdDays: Int {
Constants.staleSyncThresholdDays
}

func dismissStaleSyncWarning() {
isStaleSyncWarningDismissed = true
}

func checkStaleSyncStatus() async {
guard let catalogSyncCoordinator else { return }
isSyncStale = await catalogSyncCoordinator.isSyncStale(for: siteID, maxDays: Constants.staleSyncThresholdDays)
}
}

// MARK: - Constants
private enum Constants {
/// Number of days before showing a stale catalog sync warning
static let staleSyncThresholdDays: Int = 7
}

#if DEBUG
extension PointOfSaleAggregateModel {
func setPreviewState(paymentState: PointOfSalePaymentState, inlineMessage: PointOfSaleCardPresentPaymentMessageType?) {
Expand Down
65 changes: 55 additions & 10 deletions Modules/Sources/PointOfSale/Presentation/ItemListView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -161,20 +161,49 @@ struct ItemListView: View {

@ViewBuilder
private func listView(itemListType: ItemListType) -> some View {
ItemList(
itemsController: itemsController(itemListType),
node: .root,
itemActionHandler: actionHandler(itemListType),
willLoadMore: {
analyticsTracker.trackNextPageWillLoad()
VStack(spacing: 0) {
// Stale sync warning banner
if posModel.showStaleSyncWarning {
staleSyncWarningBanner
.padding(.horizontal, POSPadding.medium)
.padding(.vertical, POSPadding.small)
.transition(.move(edge: .top).combined(with: .opacity))
}
)
.refreshable {
analyticsTracker.trackRefresh()
await itemsController(itemListType).refreshItems(base: .root)

ItemList(
itemsController: itemsController(itemListType),
node: .root,
itemActionHandler: actionHandler(itemListType),
willLoadMore: {
analyticsTracker.trackNextPageWillLoad()
}
)
.refreshable {
analyticsTracker.trackRefresh()
await itemsController(itemListType).refreshItems(base: .root)
}
}
.task {
// Check stale sync status when view appears
await posModel.checkStaleSyncStatus()
}
}

@ViewBuilder
private var staleSyncWarningBanner: some View {
POSNoticeView(
title: Localization.staleSyncWarningTitle,
icon: Image(systemName: "info.circle"),
onDismiss: {
withAnimation {
posModel.dismissStaleSyncWarning()
}
}, content: {
Text(Localization.staleSyncWarningDescription(days: posModel.staleSyncThresholdDays))
.font(POSFontStyle.posBodyMediumRegular())
})
}

private func actionHandler(_ itemListType: ItemListType) -> POSItemActionHandler {
POSItemActionHandlerFactory.itemActionHandler(
itemListType: itemListType,
Expand Down Expand Up @@ -400,6 +429,22 @@ private extension ItemListView {
value: "Coupons",
comment: "Title of the button at the top of Point of Sale to switch to Coupons list."
)

static let staleSyncWarningTitle = NSLocalizedString(
"pos.itemlistview.staleSyncWarning.title",
value: "Refresh catalog",
comment: "Warning title shown when the product catalog hasn't synced in several days"
)

static let staleSyncWarningDescriptionFormat = NSLocalizedString(
"pos.itemlistview.staleSyncWarning.description",
value: "The catalog hasn't been synced in the last %1$ld days. Please ensure you're connected to the internet and sync again in POS Settings.",
comment: "Message shown when the product catalog hasn't synced in the specified number of days. %1$ld is the number of days. Reads like: The catalog hasn't been synced in the last 7 days."
)

static func staleSyncWarningDescription(days: Int) -> String {
String.localizedStringWithFormat(staleSyncWarningDescriptionFormat, days)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public struct PointOfSaleEntryPointView: View {
private let services: POSDependencyProviding
private let siteID: Int64
private let catalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol?
private let isLocalCatalogEligible: Bool

/// periphery: ignore - public in preparation of move to POS module
public init(siteID: Int64,
Expand Down Expand Up @@ -132,6 +133,7 @@ public struct PointOfSaleEntryPointView: View {
self.services = services
self.siteID = siteID
self.catalogSyncCoordinator = catalogSyncCoordinator
self.isLocalCatalogEligible = isLocalCatalogEligible
}

public var body: some View {
Expand Down Expand Up @@ -162,7 +164,8 @@ public struct PointOfSaleEntryPointView: View {
popularPurchasableItemsController: popularPurchasableItemsController,
barcodeScanService: barcodeScanService,
siteID: siteID,
catalogSyncCoordinator: catalogSyncCoordinator)
catalogSyncCoordinator: catalogSyncCoordinator,
isLocalCatalogEligible: isLocalCatalogEligible)
}
.environment(\.posAnalytics, services.analytics)
.environment(\.posCurrencyProvider, services.currency)
Expand Down
10 changes: 8 additions & 2 deletions Modules/Sources/PointOfSale/Utils/PreviewHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ struct POSPreviewHelpers {
barcodeScanService: PointOfSaleBarcodeScanServiceProtocol = PointOfSalePreviewBarcodeScanService(),
analytics: POSAnalyticsProviding = EmptyPOSAnalytics(),
siteID: Int64 = 1,
catalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol? = nil
catalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol? = nil,
isLocalCatalogEligible: Bool = false
) -> PointOfSaleAggregateModel {
return PointOfSaleAggregateModel(
entryPointController: POSEntryPointController(eligibilityChecker: PointOfSalePreviewTabEligibilityChecker()),
Expand All @@ -234,7 +235,8 @@ struct POSPreviewHelpers {
popularPurchasableItemsController: popularItemsController,
barcodeScanService: barcodeScanService,
siteID: siteID,
catalogSyncCoordinator: catalogSyncCoordinator
catalogSyncCoordinator: catalogSyncCoordinator,
isLocalCatalogEligible: isLocalCatalogEligible
)
}

Expand Down Expand Up @@ -635,6 +637,10 @@ final class POSPreviewCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol
func loadLastFullSyncState(for siteID: Int64) async -> POSCatalogSyncState {
return fullSyncStateModel.state[siteID] ?? .syncCompleted(siteID: siteID)
}

func isSyncStale(for siteID: Int64, maxDays: Int) async -> Bool {
return false
}
}

#endif
22 changes: 22 additions & 0 deletions Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ public protocol POSCatalogSyncCoordinatorProtocol {
/// Returns the last known full sync state for a site
/// If no state is cached, determines state from lastSyncDate
func loadLastFullSyncState(for siteID: Int64) async -> POSCatalogSyncState

/// Checks if the last sync is older than the specified number of days
/// - Parameters:
/// - siteID: The site ID to check
/// - maxDays: Maximum number of days before a sync is considered stale
/// - Returns: True if the last sync is older than the specified days or if there has been no sync
func isSyncStale(for siteID: Int64, maxDays: Int) async -> Bool
}

public extension POSCatalogSyncCoordinatorProtocol {
Expand Down Expand Up @@ -305,6 +312,21 @@ public actor POSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol {
fullSyncStateModel.state[siteID] = state
return state
}

public func isSyncStale(for siteID: Int64, maxDays: Int) async -> Bool {
// Check only the last full sync date, incremental syncs don't refresh well enough to consider non-stale.
guard let lastFullSync = await lastFullSyncDate(for: siteID) else {
// If we've never done a full sync, we're stale.
return true
}

guard let thresholdDate = Calendar.current.date(byAdding: .day, value: -maxDays, to: Date()) else {
// This shouldn't fail, and if it does, we can assume the catalog is fine
return false
}

return lastFullSync < thresholdDate
}
}

// MARK: - Syncing State
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,10 @@ final class MockPOSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProtocol {
func loadLastFullSyncState(for siteID: Int64) async -> POSCatalogSyncState {
return fullSyncStateModel.state[siteID] ?? .syncNeverDone(siteID: siteID)
}

var isSyncStaleResult: Bool = false

func isSyncStale(for siteID: Int64, maxDays: Int) async -> Bool {
return isSyncStaleResult
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -860,4 +860,78 @@ extension POSCatalogSyncCoordinatorTests {
// Then - sync should proceed (exactly at 30-day boundary is still eligible)
#expect(mockIncrementalSyncService.startIncrementalSyncCallCount == 1)
}

// MARK: - isSyncStale Tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: test_ prefix isn't necessary with Swift Testing in this file, it's confusing when writing with both XCTest and Swift Testing 😅


@Test func test_isSyncStale_returns_true_when_no_full_sync_performed() async throws {
// Given - no full sync date set

// When
let isStale = await sut.isSyncStale(for: sampleSiteID, maxDays: 7)

// Then
#expect(isStale == true)
}

@Test func test_isSyncStale_returns_false_when_full_sync_is_recent() async throws {
// Given - last full sync was 3 days ago
let threeDaysAgo = Calendar.current.date(byAdding: .day, value: -3, to: Date())!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can use try #require instead of explicitly unwrapping (similarly for other ! usage in this file)

Suggested change
let threeDaysAgo = Calendar.current.date(byAdding: .day, value: -3, to: Date())!
let threeDaysAgo = try #require(Calendar.current.date(byAdding: .day, value: -3, to: Date()))

try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: threeDaysAgo)

// When
let isStale = await sut.isSyncStale(for: sampleSiteID, maxDays: 7)

// Then
#expect(isStale == false)
}

@Test func test_isSyncStale_returns_true_when_full_sync_is_old() async throws {
// Given - last full sync was 10 days ago
let tenDaysAgo = Calendar.current.date(byAdding: .day, value: -10, to: Date())!
try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: tenDaysAgo)

// When
let isStale = await sut.isSyncStale(for: sampleSiteID, maxDays: 7)

// Then
#expect(isStale == true)
}

@Test func test_isSyncStale_ignores_incremental_sync_date() async throws {
// Given - incremental sync was recent, but full sync was old
let yesterday = Calendar.current.date(byAdding: .day, value: -1, to: Date())!
let tenDaysAgo = Calendar.current.date(byAdding: .day, value: -10, to: Date())!
try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: tenDaysAgo, lastIncrementalSyncDate: yesterday)

// When
let isStale = await sut.isSyncStale(for: sampleSiteID, maxDays: 7)

// Then - should only check full sync date
#expect(isStale == true)
}

@Test func test_isSyncStale_boundary_within_threshold() async throws {
// Given - last full sync was 6 days and 23 hours ago (just under 7 days)
let justUnderSevenDays = Calendar.current.date(byAdding: .day, value: -6, to: Date())!
.addingTimeInterval(-23 * 60 * 60) // minus 23 hours
try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: justUnderSevenDays)

// When
let isStale = await sut.isSyncStale(for: sampleSiteID, maxDays: 7)

// Then - just under threshold should not be stale
#expect(isStale == false)
}

@Test func test_isSyncStale_boundary_past_threshold() async throws {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this test case feels similar to test_isSyncStale_returns_true_when_full_sync_is_old, maybe the last full sync could be like 1 second past the threshold or exactly at the threshold?

// Given - last full sync was 8 days ago (clearly past 7 days)
let eightDaysAgo = Calendar.current.date(byAdding: .day, value: -8, to: Date())!
try createSiteInDatabase(siteID: sampleSiteID, lastFullSyncDate: eightDaysAgo)

// When
let isStale = await sut.isSyncStale(for: sampleSiteID, maxDays: 7)

// Then - past threshold should be stale
#expect(isStale == true)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -297,4 +297,8 @@ private final class MockPOSCatalogSyncCoordinator: POSCatalogSyncCoordinatorProt
func loadLastFullSyncState(for siteID: Int64) async -> POSCatalogSyncState {
return fullSyncStateModel.state[siteID] ?? .syncNeverDone(siteID: siteID)
}

func isSyncStale(for siteID: Int64, maxDays: Int) async -> Bool {
return false
}
}