Mobile Status Report [1/4]: the lookups and the system snapshot

[malinajirka]

Description

Part 1 of 4 building the Mobile Status Report (WOOMOB-3704) — the app-level counterpart to the server-side System Status Report, which carries no device or app information and is empty on tickets filed from the login screen.

This part adds no report of its own. It adds the reads the report needs, each of which is useful to review on its own terms:

• FeatureFlagAction.loadRemoteFeatureFlagsInEffect — reports what the app is acting on, which is not the same as what is in the cache: an expired cache is not consulted, so it resolves to nil rather than describing behaviour the app is not exhibiting. The new enum case makes every exhaustive switch over FeatureFlagAction incomplete, which is why the mocks and several unrelated test files are touched.
• AppSettingsAction.getPOSCatalogFileBlockedByHost — the marker recorded when a catalog file sync fails because the host blocked the file, and cleared when it succeeds. true explains a store that keeps falling back to the slower paginated sync.
• POS local catalog eligibility — exposes the cached decision only. The live check refreshes on a cache miss, which can fetch, and a status report must not change what it reports.
• ConnectivityObserver — widened with the interface type and the expensive-path flag.
• APNs device token on PushNotesManager, reported redacted.
• English byte count formatter — the free-space figure must stay greppable in English on a non-English device.
• POS visibility/eligibility logging — the checks that decide POS visibility write their result as a side effect of evaluating, so the report must not recompute them to explain a hidden tab. Logging the reason puts it on the same ticket instead.
• MobileStatusReportSystemSnapshot — the device, app and connectivity facts gathered behind one value, so the provider stays free of direct UIKit and system calls and can be tested against a fixed snapshot.

The snapshot type has no consumer until part 2; that is the nature of a stacked split.

Stack: part 1 (this PR) → part 2 → part 3 → part 4. Each part is under 300 lines of non-test code, and every commit builds on its own.

Test Steps

Test on the last PR in this chain - I plan to merge all the PRs at the same time.

Screenshots

N/A — no user-facing change in this part.

---

• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

_{The release note for the feature lands with part 4, where it becomes reachable.}

[wpmobilebot]

📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS Prototype
	Build Number	pr17641-a2711ff
	Version	25.3
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	a2711ff
	Installation URL	6gjhjebl9mvt8

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

[iamgabrielma]

imho we should remove the nil defaults here, otherwise makes it easier to become a silent failure gap.

[malinajirka]

Fixed in 28b7e89 — defaults removed; all three conformers now implement the properties explicitly.

[iamgabrielma]

This one it's also nil after PushNotificationRegistrationState.swift:136 clears it on unregistration, worth checking if we should need to change something else around this logic.

[malinajirka]

Good catch — it is also cleared on unregistration (logout, and when APNs registration fails). Doc fixed in 43bab1d. No logic change needed: either way the app holds no token, so the report reading "missing" stands.

[iamgabrielma]

I think we have some duplication here, see pre-existing SupportFormMetadataProvider.swift:161-172 inside getDeviceFreeSpace().

Additional claude comments:

1. they diverge: the loop condition. A has the && sizeAbbreviationsIndex < sizeAbbreviations.count - 1 bound; B doesn't. B would run sizeAbbreviationsIndex past YB and crash on index-out-of-range for inputs above ~1 YB — unreachable in practice for free disk space, so this is a latent inconsistency rather than a live bug. The real cost is that the two will drift on any future fix, which is exactly what the commit set out to prevent.

There's also a smaller duplication the PR adds one level up: MobileStatusReportSystemSnapshot.freeSpace() (lines 82-88 of the new file) repeats the same URL(fileURLWithPath: "/").resourceValues(forKeys: [.volumeAvailableCapacityKey]) / volumeAvailableCapacity / fallback-to-"unknown" block that getDeviceFreeSpace() already has at SupportFormMetadataProvider.swift:153-156. So after this PR, two places compute device free space and two places format bytes in English.

The fix for both is the same: have getDeviceFreeSpace() return Int64(capacityBytes).englishByteCountRepresentable, and ideally have the snapshot call a single shared free-space helper.

2. No test for englishByteCountRepresentable. It's a hand-rolled numeric formatter — the one thing in this PR that's pure input→output and trivially testable — and it has no Int64+HelpersTests. It also has an off-by-one: while capacity > 1024 renders exactly 1024 bytes as 1024.00 bytes rather than 1.00 KB. Should be >=.

[malinajirka]

Fixed in 1fdbc46 — getDeviceFreeSpace() and the snapshot now share one free-space read (UIDevice.freeDiskSpaceInEnglish) and one formatter, the 1024 off-by-one is fixed (>=), and Int64HelpersTests covers it.

[iamgabrielma]

This comment contradicts the func signature, it can be tested!

Some can't, like Bundle.main, UIDevice.current, UIScreen.main, UIApplication.shared, ... but many others are DI'd and testable.

[malinajirka]

Right — reworded in e79d900 and added MobileStatusReportSystemSnapshotTests for the injected mappings. The new test promptly caught String(describing:) rendering the imported enums as UNAuthorizationStatus(rawValue: 2) — fixed in a2711ff.

[iamgabrielma]

Is this necessary? We're logging it twice with slight differences:

- POSTabEligibilityChecker.swift:212 → DDLogInfo("POS cannot be launched for site \(siteID): …")
- POSTabVisibilityChecker.swift:133 → DDLogInfo("POS tab not visible for site \(site.siteID): …")

[malinajirka]

Intentional — they are different gates with different reason sets: the visibility checker logs why the tab is hidden (device idiom, iOS version, country, feature flags), the eligibility checker why POS cannot launch (WC plugin version, feature switch, currency). A site can pass one and fail the other, and a later part of the report quotes each prefix separately so HEs can grep them independently.

[iamgabrielma]

With this change there are some cases that do not log anything, see:

        if case .notReachable = connectivityObserver.currentStatus {
            return eligibilityService.loadCachedPOSTabVisibility(siteID: site.siteID) ?? false
        }

When that yields false, either nothing cached, or the last known state was hidden then the tab disappears and no log line is written. Suggestion:

if case .notReachable = connectivityObserver.currentStatus {
    guard let cached = eligibilityService.loadCachedPOSTabVisibility(siteID: site.siteID) else {
        logNotVisible("offline with no cached visibility for the site")
        return false
    }
    if !cached {
        logNotVisible("offline, and the last known visibility was hidden")
    }
    return cached
}

[malinajirka]

Fixed in 220d2a7 — adopted your suggestion as-is.

[iamgabrielma]

Here returns nil in two situations that are easy to conflate: nothing cached at all, and something cached but stale, but I think is fine?

[malinajirka]

Yes — intentional: it mirrors exactly what isRemoteFeatureFlagEnabled consults, and the app behaves identically in both nil cases (falls back to defaults). The report copy spells the ambiguity out: "false (nothing fetched this session, or the last fetch aged out)".

[iamgabrielma]

Is there a "better" name for these two? I have no specific suggestions but it's unclear to me if I'd understand what expensive vs constrained really mean. Perhaps isConnectionMetered, isLowDataModeEnabled?

[malinajirka]

Agreed — clarity for the support team wins here. Renamed to isConnectionMetered / isLowDataModeEnabled in 6a69e3a; the docs keep the NWPath.isExpensive/isConstrained cross-reference.

[iamgabrielma]

Is a bit tricky to wrap my head around this, but flagging just in case rings any bell. It says it mirrors exactly, but:

1. isRemoteFeatureFlagEnabled checks three sources in order: override store, then cache, then refetch.
2. remoteFeatureFlagsInEffect only reads the second. So when an override is set, the property returns the cached value while the app is acting on the overridden one, no?

This may cause oddness when testing perhaps, since for example Settings > Debug Panel > Override Feature Flags on the alpha build, toggles something on while server has it off.

If this is an actual issue (i'm not super clear), maybe the fix is just to read the override first, same order as isRemoteFeatureFlagEnabled?

[malinajirka]

Unless I'm missing something, override is available only to developers right? So I didn't worry about it, since I don't assume developers will be reaching out to support for help. Wdyt @iamgabrielma ?

[iamgabrielma]

Yeah, i think it should be fine, just wanted to bring it up because I wasn't clear on it either 👍

[iamgabrielma]

LGTM, dropped an additional comment on https://github.com/woocommerce/woocommerce-ios/pull/17641/changes#r3725638526

[wpmobilebot]

Version 25.4 has now entered code-freeze, so the milestone of this PR has been updated to 25.5.