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

Check favorite sessions in Schedule View #41

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
875b195
add star icon in the right of session row
ewa1989 Mar 17, 2024
33888d9
change icon according to the session is favorited
ewa1989 Mar 17, 2024
08a2b0d
create new action, view favoriteIconTapped
ewa1989 Mar 17, 2024
c32c5bf
change to toggle favorite condition of tapped session
ewa1989 Mar 17, 2024
cf49338
change to save and load conferences in UserDefaullts
ewa1989 Mar 17, 2024
5a32df4
Merge commit '1b5005da964abcaf5a5865b68347ab32d6e72aa2' into add-chec…
ewa1989 Mar 18, 2024
846098f
change save data type as `[String: [Session]]` for performance
ewa1989 Mar 20, 2024
23983b5
extract favorites load/save methods to FileClient, separating with fe…
ewa1989 Mar 20, 2024
a7a611b
Merge branch 'main' into add-check-favorite-session-feature
ewa1989 Mar 20, 2024
0e8b403
change file save/load area to Document that is more appropriate becau…
ewa1989 Mar 20, 2024
daa7fa7
fix failed test due to adding load Favorites when onAppear
ewa1989 Mar 20, 2024
1a5419a
separate saving favorites and updating state for testability
ewa1989 Mar 20, 2024
35b4d14
add tests for adding/removing favorites
ewa1989 Mar 20, 2024
f2622b8
extract json encoder and decorder as JSONHandler
ewa1989 Mar 20, 2024
b648df2
Revert "extract json encoder and decorder as JSONHandler"
ewa1989 Mar 21, 2024
a929586
separate FileClient
ewa1989 Mar 21, 2024
4e279e4
move mocks that are intended for specific behavior near related tests
ewa1989 Mar 21, 2024
1db51b5
Merge branch 'main' into add-check-favorite-session-feature
ewa1989 Mar 21, 2024
d3a49ab
remove unused mock
ewa1989 Mar 31, 2024
04cefc0
fix to separate load favorites from fetch data, and to assert separat…
ewa1989 Mar 31, 2024
d3b9901
to keep models having no logics, move logics in Favorites to Reducer …
ewa1989 Mar 31, 2024
cfb2fa3
rename saveDataToFile and loadDataFromFile to explain exactly how the…
ewa1989 Mar 31, 2024
77b7a09
change variable name of Conference, day to conference, as other place…
ewa1989 Apr 1, 2024
0e3b9f8
fix to combine load response into fetch response for simplicity.
ewa1989 Apr 11, 2024
6cca9bd
move a part of logic to Reducer for collecting logics relative with s…
ewa1989 Apr 11, 2024
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 @@ -49,9 +49,3 @@ extension DataClient: DependencyKey {
return data
}
}

let jsonDecoder = {
Copy link
Member

Choose a reason for hiding this comment

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

Still we need to use.

$0.dateDecodingStrategy = .iso8601
$0.keyDecodingStrategy = .convertFromSnakeCase
return $0
}(JSONDecoder())
44 changes: 44 additions & 0 deletions MyLibrary/Sources/DataClient/FileClient.swift
Copy link
Member

Choose a reason for hiding this comment

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

I ask you create in another module. Define in Package.swift.

Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import Dependencies
import DependenciesMacros
import SharedModels
import Foundation

@DependencyClient
public struct FileClient {
public var loadFavorites: @Sendable () throws -> Favorites
public var saveFavorites: @Sendable (Favorites) throws -> Void
}

extension FileClient: DependencyKey {
static public var liveValue: FileClient = .init(
loadFavorites: {
guard let saveData = loadDataFromFile(named: "Favorites") else {
return .init(eachConferenceFavorites: [])
}
let response = try jsonDecoder.decode(Favorites.self, from: saveData)
return response
},
saveFavorites: { favorites in
guard let data = try? jsonEncoder.encode(favorites) else {
return
}
saveDataToFile(data, named: "Favorites")
}
)

static func saveDataToFile(_ data : Data, named fileName: String) {
guard let documentPath = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first else {
return
}
let fileURL = documentPath.appendingPathComponent(fileName + ".json")
try? data.write(to: fileURL)
}

static func loadDataFromFile(named fileName: String) -> Data? {
guard let documentPath = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first else {
return nil
}
let fileURL = documentPath.appendingPathComponent(fileName + ".json")
return try? Data(contentsOf: fileURL)
}
}
14 changes: 14 additions & 0 deletions MyLibrary/Sources/DataClient/JSONHandler.swift
Copy link
Member

Choose a reason for hiding this comment

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

Not have to create in another file. Just put in under the client code even if duplicated.

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import Foundation

let jsonEncoder = {
$0.dateEncodingStrategy = .iso8601
$0.keyEncodingStrategy = .convertToSnakeCase
return $0
}(JSONEncoder())


let jsonDecoder = {
$0.dateDecodingStrategy = .iso8601
$0.keyDecodingStrategy = .convertFromSnakeCase
return $0
}(JSONDecoder())
61 changes: 56 additions & 5 deletions MyLibrary/Sources/ScheduleFeature/Schedule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public struct Schedule {
var day1: Conference?
var day2: Conference?
var workshop: Conference?

var favorites: Favorites = .init(eachConferenceFavorites: [])
@Presents var destination: Destination.State?

public init() {}
Expand All @@ -42,11 +44,13 @@ public struct Schedule {
case path(StackAction<Path.State, Path.Action>)
case destination(PresentationAction<Destination.Action>)
case view(View)
case fetchResponse(Result<SchedulesResponse, Error>)
case fetchResponse(Result<(schedules: SchedulesResponse, favorites: Favorites), Error>)
case savedFavorites(Session, Conference)

public enum View {
case onAppear
case disclosureTapped(Session)
case favoriteIconTapped(Session)
}
}

Expand All @@ -59,6 +63,7 @@ public struct Schedule {
public enum Destination {}

@Dependency(DataClient.self) var dataClient
@Dependency(FileClient.self) var fileClient
@Dependency(\.openURL) var openURL

public init() {}
Expand All @@ -74,7 +79,8 @@ public struct Schedule {
let day1 = try dataClient.fetchDay1()
let day2 = try dataClient.fetchDay2()
let workshop = try dataClient.fetchWorkshop()
return .init(day1: day1, day2: day2, workshop: workshop)
let favorites = try fileClient.loadFavorites()
return (.init(day1: day1, day2: day2, workshop: workshop), favorites)
}))
case let .view(.disclosureTapped(session)):
guard let description = session.description, let speakers = session.speakers else {
Expand All @@ -91,10 +97,29 @@ public struct Schedule {
)
)
return .none
case let .view(.favoriteIconTapped(session)):
let day = switch state.selectedDay {
case .day1:
state.day1!
case .day2:
state.day2!
case .day3:
state.workshop!
}
var favorites = state.favorites
favorites.updateFavoriteState(of: session, in: day)
return .run { [favorites = favorites] send in
try? fileClient.saveFavorites(favorites)
await send(.savedFavorites(session, day))
}
case let .savedFavorites(session, day):
state.favorites.updateFavoriteState(of: session, in: day)
return .none
case let .fetchResponse(.success(response)):
state.day1 = response.day1
state.day2 = response.day2
state.workshop = response.workshop
state.day1 = response.schedules.day1
state.day2 = response.schedules.day2
state.workshop = response.schedules.workshop
state.favorites = response.favorites
return .none
case let .fetchResponse(.failure(error as DecodingError)):
assertionFailure(error.localizedDescription)
Expand Down Expand Up @@ -259,6 +284,32 @@ public struct ScheduleView: View {
}
}
.frame(maxWidth: .infinity, alignment: .leading)

favoriteIcon(for: session)
.onTapGesture {
send(.favoriteIconTapped(session))
}
}
}

@ViewBuilder
func favoriteIcon(for session: Session) -> some View {
Copy link
Member

Choose a reason for hiding this comment

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

These logic should put in Reducer so that we can write test easily.

Copy link
Author

Choose a reason for hiding this comment

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

I'm sorry that I can't image which part of logic to put in Reducer.
Does it mean create new Schedule.Action case and handle it in Reduce?
Or create computed property getting Conference in Schedule.State?

Now I implemented Favorites to use Conference.title as key for getting value of favorited [Session].

Copy link
Author

Choose a reason for hiding this comment

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

I moved these logic to Schedule.swift in d3b9901.

Copy link
Member

Choose a reason for hiding this comment

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

Please remove this method if you have moved.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for my lack of explanation.
I moved a part of logic as far as possible, so now State.favorites: [String: [Session]] is used in view whether each session is favorited or not, only.

let conference =
switch store.selectedDay {
case .day1:
store.day1!
case .day2:
store.day2!
case .day3:
store.workshop!
}

if store.favorites.isFavorited(session, in: conference) {
Image(systemName: "star.fill")
.foregroundColor(.yellow)
} else {
Image(systemName: "star")
.foregroundColor(.gray)
}
}

Expand Down
30 changes: 30 additions & 0 deletions MyLibrary/Sources/SharedModels/Favorites.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
public struct Favorites: Equatable, Codable {
private var eachConferenceFavorites: [String: [Session]]

public init(eachConferenceFavorites: [(conference: Conference, favoriteSessions: [Session])]) {
Copy link
Member

Choose a reason for hiding this comment

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

Since initialized in second line, first parameter should be removed.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for my code readability, but first parameter is necessary.

To create Favorites, give pairs which has conference and its favorites sessions.
Then convert given pairs to dictionary of [conference title: its favorites sessions], and set the dictionary to property.

I intentionally use conference as input to avoid using non-existent conference title as key.
I'll continue to think readable name.

Copy link
Member

Choose a reason for hiding this comment

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

It might be better to use reduce(into:)

self.favorites = favorites.reduce(into: [:]) { result, element in
  result[element.conference.title] = element.favoriteSessions
}

self.eachConferenceFavorites = [:]
for conferenceFavorites in eachConferenceFavorites {
self.eachConferenceFavorites[conferenceFavorites.conference.title] = conferenceFavorites.favoriteSessions
}
}

public mutating func updateFavoriteState(of session: Session, in conference: Conference) {
guard var favorites = eachConferenceFavorites[conference.title] else {
eachConferenceFavorites[conference.title] = [session]
return
}
if favorites.contains(session) {
eachConferenceFavorites[conference.title] = favorites.filter { $0 != session }
} else {
favorites.append(session)
eachConferenceFavorites[conference.title] = favorites
}
}

public func isFavorited(_ session: Session, in conference: Conference) -> Bool {
guard let favorites = eachConferenceFavorites[conference.title] else {
return false
}
return favorites.contains(session)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Move into reducer. Favorite should be keep as pure model.

27 changes: 27 additions & 0 deletions MyLibrary/Tests/ScheduleFeatureTests/Mocks.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import Foundation
import SharedModels

@testable import ScheduleFeature

extension Conference {
static let mock1 = Self(
id: 1,
Expand Down Expand Up @@ -95,3 +97,28 @@ extension Speaker {
]
)
}

extension Favorites {
Copy link
Member

Choose a reason for hiding this comment

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

You can add mock under the your test code.

Copy link
Author

Choose a reason for hiding this comment

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

I moved these specific mocks near related tests in 4e279e4.

static let mock1 = favoritedSession1InConference1
static let favoritedSession1InConference1 = Self(eachConferenceFavorites: [
(.mock1, [.mock1])
])
}

extension ScheduleFeature.Schedule.State {
static let selectingDay1ScheduleWithNoFavorites = {
var initialState = Schedule.State()
initialState.selectedDay = .day1
initialState.day1 = .mock1
return initialState
}()

static let selectingDay1ScheduleWithOneFavorite = {
var initialState = Schedule.State()
initialState.selectedDay = .day1
initialState.day1 = .mock1
let firstSession = initialState.day1!.schedules.first!.sessions.first!
initialState.favorites = .init(eachConferenceFavorites: [(initialState.day1!, [firstSession])])
return initialState
}()
}
38 changes: 38 additions & 0 deletions MyLibrary/Tests/ScheduleFeatureTests/ScheduleTests.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import ComposableArchitecture
import DataClient
import SharedModels
import XCTest

@testable import ScheduleFeature
Expand All @@ -13,12 +14,14 @@ final class ScheduleTests: XCTestCase {
$0[DataClient.self].fetchDay1 = { @Sendable in .mock1 }
$0[DataClient.self].fetchDay2 = { @Sendable in .mock2 }
$0[DataClient.self].fetchWorkshop = { @Sendable in .mock3 }
$0[FileClient.self].loadFavorites = { @Sendable in .mock1 }
Copy link
Member

Choose a reason for hiding this comment

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

Should be separated your test.

Copy link
Author

@ewa1989 ewa1989 Mar 21, 2024

Choose a reason for hiding this comment

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

I'm sorry that I can't image way to separate.
Could you give me more idea?

Current implementation, loading favorites using FileClient and fetching conferences using DataClient at the same time (onAppear).
So it's required to implement testValue of loadFavorites to test onAppear behavior.

I could only image to separate loadResponse(Result<Favorites, Error>) from fetchResponse(Result<SchedulesResponse, Error>), but this idea also requires testValue of loadFavorites, so I think it doesn't satisfy your review.

Copy link
Member

Choose a reason for hiding this comment

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

NO. This effects to current test case. Should be created another test case.

Copy link
Author

Choose a reason for hiding this comment

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

Since fetching data using DataClient and loading favorites using FileClient are must to be executed at the same time, onAppear, I renamed testFetchData to testOnAppear in 04cefc0.

}
await store.send(.view(.onAppear))
await store.receive(\.fetchResponse.success) {
$0.day1 = .mock1
$0.day2 = .mock2
$0.workshop = .mock3
$0.favorites = .mock1
}
}

Expand All @@ -31,8 +34,43 @@ final class ScheduleTests: XCTestCase {
$0[DataClient.self].fetchDay1 = { @Sendable in throw FetchError() }
$0[DataClient.self].fetchDay2 = { @Sendable in .mock2 }
$0[DataClient.self].fetchWorkshop = { @Sendable in .mock3 }
$0[FileClient.self].loadFavorites = { @Sendable in .mock1 }
}
await store.send(.view(.onAppear))
await store.receive(\.fetchResponse.failure)
}

@MainActor
func testAddingFavorites() async {
let initialState: ScheduleFeature.Schedule.State = .selectingDay1ScheduleWithNoFavorites
let firstSession = initialState.day1!.schedules.first!.sessions.first!
let firstSessionFavorited: Favorites = .init(eachConferenceFavorites: [(initialState.day1!, [firstSession])])
let store = TestStore(initialState: initialState) {
Schedule()
} withDependencies: {
$0[FileClient.self].saveFavorites = { @Sendable in XCTAssertEqual($0, firstSessionFavorited) }
}

await store.send(.view(.favoriteIconTapped(firstSession)))
await store.receive(\.savedFavorites) {
$0.favorites = firstSessionFavorited
}
}

@MainActor
func testRemovingFavorites() async {
let initialState: ScheduleFeature.Schedule.State = .selectingDay1ScheduleWithOneFavorite
let firstSession = initialState.day1!.schedules.first!.sessions.first!
let noFavorites: Favorites = .init(eachConferenceFavorites: [(initialState.day1!, [])])
let store = TestStore(initialState: initialState) {
Schedule()
} withDependencies: {
$0[FileClient.self].saveFavorites = { @Sendable in XCTAssertEqual($0, noFavorites) }
}

await store.send(.view(.favoriteIconTapped(firstSession)))
await store.receive(\.savedFavorites) {
$0.favorites = noFavorites
}
}
}