Skip to content
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

feat(ErrorBanner): offline banner #478

Merged
merged 3 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions androidApp/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

<uses-permission android:name="android.permission.ACCESS_COARSE_LOCATION" />
<uses-permission android:name="android.permission.ACCESS_FINE_LOCATION" />
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
<uses-permission android:name="android.permission.INTERNET" />

<application
android:name=".MainApplication"
Expand Down
4 changes: 4 additions & 0 deletions iosApp/iosApp.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
6E04E32F2BE95A8D006F8131 /* NearbyViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E04E32E2BE95A8D006F8131 /* NearbyViewModel.swift */; };
6E20278E2BD989630037554F /* DummyTestAppView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E20278D2BD989630037554F /* DummyTestAppView.swift */; };
6E2027902BD989AC0037554F /* ProductionAppView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E20278F2BD989AC0037554F /* ProductionAppView.swift */; };
6E2D6CA12CC2EDD700959605 /* ErrorBannerViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E2D6CA02CC2EDD700959605 /* ErrorBannerViewModelTests.swift */; };
6E35D4CE2B72C74500A2BF95 /* MapboxMaps in Frameworks */ = {isa = PBXBuildFile; productRef = 6E35D4CD2B72C74500A2BF95 /* MapboxMaps */; };
6E35D4D02B72C7B700A2BF95 /* HomeMapView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E35D4CF2B72C7B700A2BF95 /* HomeMapView.swift */; };
6E35D4D32B72CD3900A2BF95 /* HomeMapViewTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E35D4D22B72CD3900A2BF95 /* HomeMapViewTests.swift */; };
Expand Down Expand Up @@ -249,6 +250,7 @@
6E04E32E2BE95A8D006F8131 /* NearbyViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NearbyViewModel.swift; sourceTree = "<group>"; };
6E20278D2BD989630037554F /* DummyTestAppView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DummyTestAppView.swift; sourceTree = "<group>"; };
6E20278F2BD989AC0037554F /* ProductionAppView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductionAppView.swift; sourceTree = "<group>"; };
6E2D6CA02CC2EDD700959605 /* ErrorBannerViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ErrorBannerViewModelTests.swift; sourceTree = "<group>"; };
6E35D4CF2B72C7B700A2BF95 /* HomeMapView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomeMapView.swift; sourceTree = "<group>"; };
6E35D4D22B72CD3900A2BF95 /* HomeMapViewTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomeMapViewTests.swift; sourceTree = "<group>"; };
6E3C8D7D2C11FDA80059C28C /* ActionButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ActionButton.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -594,6 +596,7 @@
isa = PBXGroup;
children = (
6EFEE42A2BEC100100810319 /* NearbyViewModelTests.swift */,
6E2D6CA02CC2EDD700959605 /* ErrorBannerViewModelTests.swift */,
);
path = ViewModels;
sourceTree = "<group>";
Expand Down Expand Up @@ -1292,6 +1295,7 @@
8C5F47662C40842200FB71DA /* TripDetailsStopViewTests.swift in Sources */,
8CDF2C342BE9357E007FC912 /* OptionalNavigationLinkTests.swift in Sources */,
8CA1FB772BF813F500384658 /* TripDetailsStopListSplitViewTests.swift in Sources */,
6E2D6CA12CC2EDD700959605 /* ErrorBannerViewModelTests.swift in Sources */,
8CB823DB2BC5F053002C87E0 /* StopDetailsRoutesViewTests.swift in Sources */,
6E4EACFC2B7A82AC0011AB8B /* MockLocationFetcher.swift in Sources */,
8CE36C932CADEDD300D77F22 /* FetchApiTests.swift in Sources */,
Expand Down
6 changes: 6 additions & 0 deletions iosApp/iosApp/ComponentViews/ErrorBanner.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ struct ErrorBanner: View {
}
.frame(minHeight: minHeight)
}
case .networkError:
ErrorCard { HStack {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the default alignment in this HStack not center? It looks kinda like the image is below the centerline of the text.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added some borders to each view - I think looks it is centered when accounting for the full line height of the text.
image

I swapped in HStack(alignment: .center) and didn't notice a difference

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay yeah, I think it's just a matter of the icon being slightly bottom heavy. Thanks for checking!

Image(systemName: "wifi.slash")
Text("Unable to connect")
Copy link
Contributor

Choose a reason for hiding this comment

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

Because this has no reload button like the other two banners, it also has no Spacer(), so we're getting the awkward squished banner. If you toss a Spacer() at the end here, I think that'll get it to take up the full width.

Spacer()
}}
case nil:
// for some reason, .collect on an EmptyView doesn't work
EmptyView()
Expand Down
8 changes: 4 additions & 4 deletions iosApp/iosApp/ComponentViews/ErrorCard.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@

import SwiftUI

struct ErrorCard: View {
var details: Text
struct ErrorCard<Content: View>: View {
@ViewBuilder let details: Content
var button: (() -> AnyView)?

init(_ details: () -> Text) {
init(_ details: () -> Content) {
self.details = details()
button = nil
}

init(details: Text, button: (() -> AnyView)? = nil) {
init(details: Content, button: (() -> AnyView)? = nil) {
self.details = details
self.button = button
}
Expand Down
3 changes: 3 additions & 0 deletions iosApp/iosApp/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,9 @@
},
"trains" : {
"comment" : "trains"
},
"Unable to connect" : {

},
"Unruly Passenger" : {
"comment" : "Possible alert cause"
Expand Down
15 changes: 12 additions & 3 deletions iosApp/iosApp/ViewModels/ErrorBannerViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,33 @@
let errorRepository: IErrorBannerStateRepository

@Published
private(set) var errorState: ErrorBannerState? = nil

Check notice on line 16 in iosApp/iosApp/ViewModels/ErrorBannerViewModel.swift

View check run for this annotation

Xcode Cloud / MBTA Transit Staging | PR Branch Workflow | iosAppRetries - iOS

iosApp/iosApp/ViewModels/ErrorBannerViewModel.swift#L16

Redundant Optional Initialization Violation: Initializing an optional variable with nil is redundant (redundant_optional_initialization)

@Published
var loadingWhenPredictionsStale: Bool

// option for testing
var skipListeningForStateChanges = false

init(
errorRepository: IErrorBannerStateRepository = RepositoryDI().errorBanner,
initialLoadingWhenPredictionsStale: Bool = false
initialLoadingWhenPredictionsStale: Bool = false,
skipListeningForStateChanges: Bool = false
) {
self.errorRepository = errorRepository
loadingWhenPredictionsStale = initialLoadingWhenPredictionsStale
errorState = self.errorRepository.state.value
self.skipListeningForStateChanges = skipListeningForStateChanges
}

@MainActor
func activate() async {
for await errorState in errorRepository.state {
self.errorState = errorState
errorRepository.subscribeToNetworkStatusChanges()

if !skipListeningForStateChanges {
for await errorState in errorRepository.state {
self.errorState = errorState
}
}
}

Expand Down
7 changes: 6 additions & 1 deletion iosApp/iosAppTests/Utils/FetchApiTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ final class FetchApiTests: XCTestCase {
onRefreshAfterError: { expRefresh.fulfill() }
)
XCTAssertNotNil(errorBannerRepo.state.value)
errorBannerRepo.state.value?.action()

if let action = errorBannerRepo.state.value?.action {
action()
} else {
XCTFail("data error missing action")
}
await fulfillment(of: [expRefresh], timeout: 1)
}

Expand Down
26 changes: 26 additions & 0 deletions iosApp/iosAppTests/ViewModels/ErrorBannerViewModelTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//
// ErrorBannerViewModelTests.swift
// iosAppTests
//
// Created by Kayla Brady on 10/18/24.
// Copyright © 2024 MBTA. All rights reserved.
//

import Foundation
@testable import iosApp
import shared
import XCTest

final class ErrorBannerViewModelTests: XCTestCase {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this!

func testActivateSubscribesToNetworkChanges() async {
let onSubscribeExp = XCTestExpectation(description: "onSubscribe called")
let repo = MockErrorBannerStateRepository(state: nil, onSubscribeToNetworkChanges: { onSubscribeExp.fulfill() })
let errorVM = ErrorBannerViewModel(
errorRepository: repo,
initialLoadingWhenPredictionsStale: false,
skipListeningForStateChanges: true
)
await errorVM.activate()
wait(for: [onSubscribeExp], timeout: 1)

Check notice on line 24 in iosApp/iosAppTests/ViewModels/ErrorBannerViewModelTests.swift

View check run for this annotation

Xcode Cloud / MBTA Transit Staging | PR Branch Workflow | iosAppRetries - iOS

iosApp/iosAppTests/ViewModels/ErrorBannerViewModelTests.swift#L24

Instance method 'wait' is unavailable from asynchronous contexts; Use await fulfillment(of:timeout:enforceOrder:) instead; this is an error in Swift 6
}
}
8 changes: 8 additions & 0 deletions iosApp/iosAppTests/Views/ErrorBannerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@ final class ErrorBannerTests: XCTestCase {
wait(for: [callsAction], timeout: 1)
}

@MainActor func testWhenNetworkError() throws {
let sut = ErrorBanner(.init(
errorRepository: MockErrorBannerStateRepository(state: .NetworkError()),
initialLoadingWhenPredictionsStale: true
))
XCTAssertNotNil(try sut.inspect().find(text: "Unable to connect"))
}

@MainActor func testLoadingWhenPredictionsStale() throws {
let sut = ErrorBanner(.init(
errorRepository: MockErrorBannerStateRepository(state: .StalePredictions(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package com.mbta.tid.mbta_app

import com.mbta.tid.mbta_app.network.INetworkConnectivityMonitor
import com.mbta.tid.mbta_app.network.NetworkConnectivityMonitor
import com.mbta.tid.mbta_app.utils.AndroidSystemPaths
import com.mbta.tid.mbta_app.utils.SystemPaths
import org.koin.dsl.module

fun platformModule() = module {
includes(
module { single { createDataStore(get()) } },
module { single<SystemPaths> { AndroidSystemPaths(get()) } }
module { single<SystemPaths> { AndroidSystemPaths(get()) } },
module { single<INetworkConnectivityMonitor> { NetworkConnectivityMonitor(get()) } }
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.mbta.tid.mbta_app.network

import android.annotation.SuppressLint
import android.content.Context
import android.content.Context.CONNECTIVITY_SERVICE
import android.net.ConnectivityManager
import android.net.Network

class NetworkConnectivityMonitor(context: Context) : INetworkConnectivityMonitor {
private var networkCallback: ConnectivityManager.NetworkCallback? = null
private val connectivityManager =
context.getSystemService(CONNECTIVITY_SERVICE) as ConnectivityManager

@SuppressLint("MissingPermission")
// Permission is included in AndroidManifest.xml
override fun registerListener(onNetworkAvailable: () -> Unit, onNetworkLost: () -> Unit) {
networkCallback =
object : ConnectivityManager.NetworkCallback() {
override fun onAvailable(network: Network) {
onNetworkAvailable()
}

override fun onUnavailable() {
onNetworkLost()
}

override fun onLost(network: Network) {
onNetworkLost()
}
}
networkCallback?.let { connectivityManager.registerDefaultNetworkCallback(it) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ class RepositoryDI : IRepositories, KoinComponent {
class RealRepositories : IRepositories {
// initialize repositories with platform-specific dependencies as null.
// instantiate the real repositories in makeNativeModule

override val alerts = null
override val appCheck = null
override val config = ConfigRepository()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import kotlinx.datetime.Instant

sealed class ErrorBannerState {
/** What to do when the button in the error banner is pressed */
abstract val action: () -> Unit
abstract val action: (() -> Unit)?

data class StalePredictions(val lastUpdated: Instant, override val action: () -> Unit) :
ErrorBannerState() {
fun minutesAgo() = (Clock.System.now() - lastUpdated).inWholeMinutes
}

data class DataError(override val action: () -> Unit) : ErrorBannerState()

data class NetworkError(override val action: (() -> Unit)?) : ErrorBannerState()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package com.mbta.tid.mbta_app.network

/** Observe changes in the device's network connectivity. */
interface INetworkConnectivityMonitor {
fun registerListener(onNetworkAvailable: () -> Unit, onNetworkLost: () -> Unit)
}
Original file line number Diff line number Diff line change
@@ -1,24 +1,47 @@
package com.mbta.tid.mbta_app.repositories

import com.mbta.tid.mbta_app.model.ErrorBannerState
import com.mbta.tid.mbta_app.network.INetworkConnectivityMonitor
import kotlin.time.Duration.Companion.minutes
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.datetime.Clock
import kotlinx.datetime.Instant
import org.koin.core.component.KoinComponent
import org.koin.core.component.inject

sealed class NetworkStatus {
data object Connected : NetworkStatus()

data object Disconnected : NetworkStatus()
}

abstract class IErrorBannerStateRepository(initialState: ErrorBannerState? = null) : KoinComponent {

private val networkConnectivityMonitor: INetworkConnectivityMonitor by inject()

/*
Registers platform-specific observer of network status changes.
*/
open fun subscribeToNetworkStatusChanges() {
this.networkConnectivityMonitor.registerListener(
onNetworkAvailable = { setNetworkStatus(NetworkStatus.Connected) },
onNetworkLost = { setNetworkStatus(NetworkStatus.Disconnected) }
)
}

abstract class IErrorBannerStateRepository
protected constructor(initialState: ErrorBannerState? = null) {
protected val flow = MutableStateFlow(initialState)
val state = flow.asStateFlow()

private var networkStatus: NetworkStatus? = null

private var predictionsStale: ErrorBannerState.StalePredictions? = null
private val dataErrors = mutableMapOf<String, ErrorBannerState.DataError>()

protected open fun updateState() {
flow.value =
when {
networkStatus == NetworkStatus.Disconnected -> ErrorBannerState.NetworkError(null)
dataErrors.isNotEmpty() ->
// encapsulate all the different error actions within one error
ErrorBannerState.DataError { dataErrors.values.forEach { it.action() } }
Expand All @@ -41,6 +64,11 @@ protected constructor(initialState: ErrorBannerState? = null) {
updateState()
}

private fun setNetworkStatus(newStatus: NetworkStatus) {
networkStatus = newStatus
updateState()
}

fun setDataError(key: String, action: () -> Unit) {
dataErrors[key] = ErrorBannerState.DataError(action)
updateState()
Expand All @@ -60,8 +88,15 @@ protected constructor(initialState: ErrorBannerState? = null) {

class ErrorBannerStateRepository : IErrorBannerStateRepository(), KoinComponent

class MockErrorBannerStateRepository(state: ErrorBannerState? = null) :
IErrorBannerStateRepository(state) {
class MockErrorBannerStateRepository(
state: ErrorBannerState? = null,
onSubscribeToNetworkChanges: (() -> Unit)? = null
) : IErrorBannerStateRepository(state) {
private val onSubscribeToNetworkChanges = onSubscribeToNetworkChanges
val mutableFlow
get() = flow

override fun subscribeToNetworkStatusChanges() {
onSubscribeToNetworkChanges?.invoke()
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
package com.mbta.tid.mbta_app.repositories

import com.mbta.tid.mbta_app.model.ErrorBannerState
import com.mbta.tid.mbta_app.network.INetworkConnectivityMonitor
import dev.mokkery.MockMode
import dev.mokkery.matcher.any
import dev.mokkery.mock
import dev.mokkery.verify
import kotlin.test.AfterTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertIs
Expand All @@ -13,8 +19,13 @@ import kotlinx.coroutines.flow.take
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.datetime.Clock
import org.koin.core.context.startKoin
import org.koin.core.context.stopKoin
import org.koin.dsl.module

class ErrorBannerStateRepositoryTest {
@AfterTest fun `stop koin`() = run { stopKoin() }

@Test
fun `initial state is null`() = runBlocking {
val repo = ErrorBannerStateRepository()
Expand Down Expand Up @@ -112,4 +123,18 @@ class ErrorBannerStateRepositoryTest {

assertNull(channel.receive())
}

@Test
fun `subscribe to connectivity changes`() {

val mockNetworkMonitor = mock<INetworkConnectivityMonitor>(MockMode.autofill)

startKoin { modules(module { single<INetworkConnectivityMonitor> { mockNetworkMonitor } }) }

val repo = ErrorBannerStateRepository()

repo.subscribeToNetworkStatusChanges()

verify { mockNetworkMonitor.registerListener(any(), any()) }
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package com.mbta.tid.mbta_app

import com.mbta.tid.mbta_app.network.INetworkConnectivityMonitor
import com.mbta.tid.mbta_app.network.NetworkConnectivityMonitor
import com.mbta.tid.mbta_app.utils.IOSSystemPaths
import com.mbta.tid.mbta_app.utils.SystemPaths
import org.koin.dsl.module

fun platformModule() = module {
includes(
module { single { createDataStore() } },
module { single<SystemPaths> { IOSSystemPaths() } }
module { single<SystemPaths> { IOSSystemPaths() } },
module { single<INetworkConnectivityMonitor> { NetworkConnectivityMonitor() } }
)
}
Loading