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

Simple SyncUp tutorial fixes #3141

Merged
merged 22 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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 @@ -680,7 +680,7 @@ func testFeature() {
}
```

However, if your test suite is apart of an app target, then the entry point of the app will execute
However, if your test suite is a part of an app target, then the entry point of the app will execute
and potentially cause an early access of `@Shared`, thus capturing a different default value than
what is specified above. This quirk of tests in app targets is documented in
<doc:Testing#Testing-gotchas> of the <doc:Testing> article, and a similar quirk exists for Xcode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ struct SyncUp: Equatable, Identifiable, Codable {
var meetings: [Meeting] = []
var theme: Theme = .bubblegum
var title = ""

var durationPerAttendee: Duration {
duration / attendees.count
}
}

struct Attendee: Equatable, Identifiable, Codable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ struct SyncUp: Equatable, Identifiable, Codable {
var meetings: IdentifiedArrayOf<Meeting> = []
var theme: Theme = .bubblegum
var title = ""

var durationPerAttendee: Duration {
duration / attendees.count
}
}

struct Attendee: Equatable, Identifiable, Codable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ struct SyncUp: Equatable, Identifiable, Codable {
var meetings: IdentifiedArrayOf<Meeting> = []
var theme: Theme = .bubblegum
var title = ""

var durationPerAttendee: Duration {
duration / attendees.count
}
}

struct Attendee: Equatable, Identifiable, Codable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,51 +52,7 @@
[identified-array-gh]: http://github.com/pointfreeco/swift-identified-collections

@Code(name: "Models.swift", file: ListsOfSyncUps-01-code-0003.swift)
}

<!--
NB: Removing the Tagged discussion for now because it was too confusing for readers.

There's another improvement that can be made to our domain models, but this one is completely
optional.

Right now all of the IDs of our models are `UUID`. This means it is technically possible to
do something non-sensical like check if a `syncUp.id` is equal to an `attendee.id`. That is
never a valid thing to do, and so it would be ideal if we could detect that at _compile time_,
rather than runtime.

We can do this by leveraging one of our other libraries, [Tagged][tagged-gh]. This library
does not automatically come with the Composable Architecture, and so you will need to add it,
and these next steps are completely optional for the purpose of this tutorial.

[tagged-gh]: http://github.com/pointfreeco/swift-tagged

@Step {
Add the `Tagged` library to your project by navigating to the build settings, and then to
the package dependencies tab, and input the GitHub URL:
[https://github.com/pointfreeco/swift-tagged](https://github.com/pointfreeco/swift-tagged).

@Image(source: syncups-02-package-dependencies-tagged.png)
}

@Step {
Go back to Models.swift, and import the `Tagged` library, and upgrade all usages of a plain
`UUID` to be a tagged `UUID`.

> Note: The tag can even just be `Self`, but you can also have multiple tagged values in a
> model by created dedicated tagged types. See the [Tagged repo][tag-collisions-gh] for more
> information.

[tag-collisions-gh]: https://github.com/pointfreeco/swift-tagged#handling-tag-collisions

@Code(name: "Models.swift", file: ListsOfSyncUps-01-code-0004.swift)
}
> Important: To keep things simple for the rest of this tutorial we will _not_ assume that
you have implemented the `Tagged` type into your domain. If you choose to use `Tagged` then
you will need to do some extra work to construct tagged values that is not covered in
the tutorial. For example, once we get to using dependencies to control UUIDs, you will need
to write `Attendee.ID(uuid())` instead of just `uuid()`.
-->
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@

That is all it takes to integrate two features together using the tools from the
Composable Architecture. Notice that we have not yet mentioned _how_ we want the
`SyncUpFormView` is presented, _e.g._ as a sheet, popover, full-screen cover, or something
`SyncUpFormView` to be presented, _e.g._ as a sheet, popover, full-screen cover, or something
else? The reducer does not care about that detail. It only cares about the domain modeling of
the optional feature. You do not need to decide the _type_ of navigation until you implement
the view layer.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
Go back to the SyncUpsList.swift file, and start by applying the [`@Shared`](<doc:Shared>)
property wrapper to the `syncUps` field.

> Note: Our changes will not compile until step 3.
Copy link
Contributor Author

@dafurman dafurman Jun 5, 2024

Choose a reason for hiding this comment

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

Just a suggestion to throw this in, because often when I go through enough steps in a tutorial without code compiling, I end up wondering if I missed something in a step, which distracts from the tutorial itself.

However, if you don't want this change, you can revert 9d93448 where this is added

stephencelis marked this conversation as resolved.
Show resolved Hide resolved

@Code(name: "SyncUpsList.swift", file: PersistingSyncUps-01-code-0001.swift, previousFile: PersistingSyncUps-01-code-0001-previous.swift)
}

Expand Down Expand Up @@ -51,7 +53,7 @@

With that change the project should be compiling. It is worth noting that
``ComposableArchitecture/PersistenceReaderKey/fileStorage(_:)`` only works with `Codable` data
types, and earlier in the tutorial when add models to Models.swift we made them codable from
types, and earlier in the tutorial when we added models to Models.swift we made them codable from
the beginning.

To confirm that persistence works we need to run the app in the simulator, but we haven't done
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@

> Note: We are pre-emptively applying the `@CasePathable` macro to make it possible to write
tests against these actions in a keypath-like syntax. The `@CasePathable` macro is
automatically applied to `Action` enums inside reducers, but macros cannot recursive apply
automatically applied to `Action` enums inside reducers, but macros cannot recursively apply
themselves and so we must do it manually sometimes.

@Code(name: "SyncUpDetail.swift", file: EditingAndDeletingSyncUp-02-code-0001.swift, previousFile: EditingAndDeletingSyncUp-02-code-0001-previous.swift)
Expand Down Expand Up @@ -184,7 +184,8 @@
}

@Step {
Use the ``ComposableArchitecture/Reducer/ifLet(_:action:then:fileID:line:)-42kki`` operator
Add support for the `.alert` case, and
Copy link
Contributor Author

@dafurman dafurman Jun 5, 2024

Choose a reason for hiding this comment

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

because the step also makes this change in the diff from
EditingAndDeletingSyncUp-02-code-0003.swift


to
EditingAndDeletingSyncUp-02-code-0004.swift

use the ``ComposableArchitecture/Reducer/ifLet(_:action:then:fileID:line:)-42kki`` operator
stephencelis marked this conversation as resolved.
Show resolved Hide resolved
again to integrate the alert's logic into the `SyncUpDetail` reducer.

@Code(name: "SyncUpDetail.swift", file: EditingAndDeletingSyncUp-02-code-0004.swift)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
}

Next we add a case to the `Action` enum for each action the user can perform in the view.
Currently there are 3 buttons, and so we will have an action for each.
Currently there are 2 buttons, and so we will have an action for each.
Copy link
Contributor Author

@dafurman dafurman Jun 5, 2024

Choose a reason for hiding this comment

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

From this, which today just covers the two actions - delete and edit.

Add an action for each of the buttons in the UI, including tapping the "Delete" button,
and the "Edit" button.


@Step {
Add an action for each of the buttons in the UI, including tapping the "Delete" button,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@

@Step {
Assert how the state changes after sending the action. In particular, the `syncUp` data
inside the `edit` case of the destination should change. We can use the special
``ComposableArchitecture/PresentationState/subscript(case:)-7uqte`` defined on
`$destination` to modify the data in a particular case.
inside the `edit` case of the destination should change.
Copy link
Contributor Author

Choose a reason for hiding this comment

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


Run the test suite again to confirm that everything still passes.

Expand All @@ -71,79 +69,5 @@
It is also possible to shorten this test quite a bit by using a non-exhaustive test store, as
we did in <doc:PresentingSyncUpForm>, but we will leave that as an exercise for the reader.
}
}

<!--
This is all old now that we are using @Shared and deleting directly in the reducer. We can
either omit this or update it.

@Section(title: "Testing the delete flow") {
@ContentAndMedia {
Next we will test the flow of the user deleting the sync-up. This involves asserting that the
confirmation alert shows, and emulating the user interacting with the alert.
}

@Steps {
@Step {
Create a new test method for testing the delete user flow. Also create a
``ComposableArchitecture/TestStore`` for the `SyncUpDetail` feature.

@Code(name: "SyncUpDetailTests.swift", file: TestingSyncUpDetail-02-code-0001.swift, previousFile: TestingSyncUpDetail-02-code-0001-previous.swift)
}

@Step {
Emulate the user tapping the "Delete" button and assert that the alert state is populated.

Run the test suite to confirm that it passes.

@Code(name: "SyncUpDetailTests.swift", file: TestingSyncUpDetail-02-code-0002.swift)
}

@Step {
Emulate the user confirming deletion of the sync-up by sending a deeply nested action to the
`.destination`, when it is `.presented`, in the `.alert` case, and then finally the
`.confirmButtonTapped` action. Also assert that the alert will be dismissed by setting the
`destination` state to `nil`.

TODO: Make alert action case-pathable

@Code(name: "SyncUpDetailTests.swift", file: TestingSyncUpDetail-02-code-0003.swift)
}

@Step {
Run the test suite again to see that now it does _not_ pass. We have a test failure.

@Code(name: "SyncUpDetailTests.swift", file: TestingSyncUpDetail-02-code-0004.swift)
}

This failure is happening because we are now making use effects that emit actions, and by
default the ``ComposableArchitecture/TestStore`` forces us to assert on _everything_ happening
in the feature. This is really great because it helps us catch potential problems when new
logic and behavior is added to our features.

The failure is letting us know that the system received an action that we did not
assert against. This is also a great failure to have because it forces us to prove that we
know how effects execute and feed their data back in the system.

Let's fix this test failure by asserting on more of what is happening in the system.

@Step {
Use the ``ComposableArchitecture/TestStore/receive(_:_:timeout:assert:file:line:)-dkei``
method to assert that we expect to receive a delegate action to delete the sync-up.

Run the test suite to see that one of the previous two test failures has now been fixed.

> Note: We are using case key path composition in order to describe the deeply nested action
> that is being received.

@Code(name: "SyncUpDetailTests.swift", file: TestingSyncUpDetail-02-code-0005.swift)
}

Now that this test passes we have definitively proven that
the `.deleteSyncUp` delegate action is sent when the user confirms deletion of the sync-up.
That will communicate to the parent that it should finish deleting the sync-up _and_ it should
dismiss the presented sheet.
}
}
-->
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,31 @@ struct MeetingView: View {
}
}

extension SyncUp {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding SyncUp.mock while we're making the MeetingView, which gets used later on in the tutorial

static let mock = Self(
id: SyncUp.ID(),
attendees: [
Attendee(id: Attendee.ID(), name: "Blob"),
Attendee(id: Attendee.ID(), name: "Blob Jr"),
Attendee(id: Attendee.ID(), name: "Blob Sr"),
],
duration: .seconds(60),
meetings: [
Meeting(
id: Meeting.ID(),
date: Date().addingTimeInterval(-60 * 60 * 24 * 7),
transcript: """
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor \
incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud \
"""
)
],
theme: .orange,
title: "Design"
)
}

stephencelis marked this conversation as resolved.
Show resolved Hide resolved
#Preview {
MeetingView(meeting: SyncUp.mock.meetings[0], syncUp: .mock)
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
@Step {
Integrate the `SyncUpsList` reducer into the `AppReducer` by holding onto its state and
actions, and using the ``ComposableArchitecture/Scope`` reducer to compose the two
reducer together.
reducers together.

@Code(name: "App.swift", file: SyncUpDetailNavigation-01-code-0002.swift)
}
Expand Down Expand Up @@ -243,7 +243,7 @@
}

That is all it takes to perform a navigation from the list feature to the detail feature.
When the navigation link is tapped, SwiftUI will search up the view heirarchy to find the
When the navigation link is tapped, SwiftUI will search up the view hierarchy to find the
`NavigationStack` powering the view, find its `path` binding, and append the new `.detail`
state to the path, thus triggering a drill-down animation.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import XCTest

@testable import SyncUps

final class AppFeatureTests: XCTestCase {@MainActor
final class AppFeatureTests: XCTestCase {
@MainActor
func testDelete() async throws {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import XCTest

@testable import SyncUps

final class AppFeatureTests: XCTestCase {@MainActor
final class AppFeatureTests: XCTestCase {
@MainActor
func testDelete() async throws {
let syncUp = SyncUp.mock
@Shared(.syncUps) var syncUps = [syncUp]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import XCTest

@testable import SyncUps

final class AppFeatureTests: XCTestCase {@MainActor
final class AppFeatureTests: XCTestCase {
@MainActor
func testDelete() async throws {
let syncUp = SyncUp.mock
@Shared(.syncUps) var syncUps = [syncUp]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import XCTest

@testable import SyncUps

final class AppFeatureTests: XCTestCase {@MainActor
final class AppFeatureTests: XCTestCase {
@MainActor
func testDelete() async throws {
let syncUp = SyncUp.mock
@Shared(.syncUps) var syncUps = [syncUp]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import XCTest

@testable import SyncUps

final class AppFeatureTests: XCTestCase {@MainActor
final class AppFeatureTests: XCTestCase {
@MainActor
func testDelete() async throws {
let syncUp = SyncUp.mock
@Shared(.syncUps) var syncUps = [syncUp]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import XCTest

@testable import SyncUps

final class AppFeatureTests: XCTestCase {@MainActor
final class AppFeatureTests: XCTestCase {
@MainActor
func testDelete() async throws {
let syncUp = SyncUp.mock
@Shared(.syncUps) var syncUps = [syncUp]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import XCTest

@testable import SyncUps

final class AppFeatureTests: XCTestCase {@MainActor
final class AppFeatureTests: XCTestCase {
@MainActor
func testDelete() async throws {
let syncUp = SyncUp.mock
@Shared(.syncUps) var syncUps = [syncUp]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import XCTest

@testable import SyncUps

final class AppFeatureTests: XCTestCase {@MainActor
final class AppFeatureTests: XCTestCase {
@MainActor
func testDelete() async throws {
let syncUp = SyncUp.mock
@Shared(.syncUps) var syncUps = [syncUp]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ struct RecordMeeting {
if state.secondsElapsed.isMultiple(of: secondsPerAttendee) {
if state.secondsElapsed == state.syncUp.duration.components.seconds {
state.syncUp.meetings.insert(
Meeting(id: Meeting.ID(), date: Date(), transcript: transcript),
Meeting(id: Meeting.ID(), date: Date(), transcript: state.transcript),
at: 0
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ struct RecordMeeting {
if state.secondsElapsed.isMultiple(of: secondsPerAttendee) {
if state.secondsElapsed == state.syncUp.duration.components.seconds {
state.syncUp.meetings.insert(
Meeting(id: Meeting.ID(), date: Date(), transcript: transcript),
Meeting(id: Meeting.ID(), date: Date(), transcript: state.transcript),
at: 0
)
return .run { _ in await dismiss() }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ struct RecordMeeting {
if state.secondsElapsed.isMultiple(of: secondsPerAttendee) {
if state.secondsElapsed == state.syncUp.duration.components.seconds {
state.syncUp.meetings.insert(
Meeting(id: Meeting.ID(), date: Date(), transcript: transcript),
Meeting(id: Meeting.ID(), date: Date(), transcript: state.transcript),
at: 0
)
return .run { _ in await dismiss() }
Expand Down
Loading