Compass App: Add "Activities", "Itinerary Config" and MVVM Commands

[miquelbeltran]

Part of the WIP for the Compass App example. Merge to compass-app.

This PR introduces:

• A new feature for Activities (UI unfinished).
• A repository for the current Itinerary Configuration.
• A Command utils class to be used in View Models.

Activities

• PR adds the compass_app/app/assets/activities.json (large file!)
• Created ActivityRepository with local and remote implementation.
• Added getActivitiesByDestination to ApiClient
• Added Activity data model
• Created ActivitiesScreen and ActivitiesViewModel.
• WIP: Decided to finish the UI later due to the size the PR was taking.
• WIP: Server implementation for Activities will be completed in another PR.

Itinerary Configuration

• Created the ItineraryConfigRepository with an "in-memory" implementation. (local database or shared preferences could potentially be implemented too)
• Refactored the way screens share data, instead of passing data using the navigator query parameters, the screens store the state (the itinerary configuration) in this repository, and load it when the screen is opened.
• This allows to navigate between screens, back and forth, and keep the selection of data the user made.

Commands

• To handle button taps and other running actions.
• Encapsulates an action, exposes the running state (to show progress indicators), and ensures that the action cannot execute if it is already running (to avoid multiple taps on buttons).
• Two implementations included, one without arguments Command0, and one that supports a single argument Command1.
• Commands also provide an onComplete callback, in case the UI needs to do something when the action finished running (e.g. navigate).
• Tests are included.

TODO in further PRs

• Finish the Activities UI and continue implementing the app flow.
• Introduce an error handling solution.
• Move the data jsons into a common folder (maybe a package?) so they can be shared between app and server and don't duplicate files.

Screencast

As it can be observed, the state of the screen is recovered from the stored "itinerary config".

Note: Activites screen appears empty, the list is just printed on terminal at the moment.

Screencast.from.2024-07-23.10-58-40.webm

Pre-launch Checklist

• I read the Flutter Style Guide recently, and have followed its advice.
• I signed the CLA.
• I read the Contributors Guide.
• I updated/added relevant documentation (doc comments with ///).
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-devrel channel on Discord.

[ericwindmill]

I’ve been thinking about topics discussed in previous PRs. I want to pose the following questions, but I’m not suggesting that you make changes to this PR.

blocs/Components/use-cases - I’m coming around to the idea of the additional “business logic” layer in cases where ViewModels need data from multiple repositories, for preventing ViewModels from getting too complex, for making testing easier, etc. Or is it better to just have an optional additional layer of repositories? (i.e. for features that are complex, there’s a repository that basically combines data for the other repositories?)

Either way, the problem is that the app doesn’t need an additional layer right now, and I'm not sure it will. So do we force blocs to provide a good example for larger teams, or do we do what’s right for this app?

(If we do add another layer, I think we should call them "components" or logic components or something. Blocs means something else to most flutter developers, and use-cases closely mirrors Clean architecture, which comes with a lot of baggage. Teams internal to Google call them some variation of component or bloc.)

Shared models - This is something @reidbaker had comments about previously. Android docs recommend having shared models for APIs, and then having an additional model for the UI. In the flutter client, you might have ‘class Activity’, which is used by the ViewModels, and in the shared model library you might have class ActivityApiModel. This recommendation makes sense to me in cases where the app is complex, but has the same problem – this app doesn’t need two different models, because they’d be the same. Again, should we force this because our target audience is bigger teams, or do we just wait and see if that need arises.

We could solve these problems by writing about these different options in the docs, but I do think for the first iteration of the docs, it’ll be better to have a single recommendation.

commands - These are new to this PR. This is interesting because it kind of has the same problem as the two previous topics. I think they’re a good idea and should stay in this app, but this particular app could be perfectly functional without them. This makes me think we should be forcing the above recommendations, because it’s unlikely that this sample will ever get complex enough that all of our architectural recommendations make sense here.

Again - not recommending anything change in this PR, just opening up the conversation.

[reidbaker]

blocs/Components/use-cases - I’m coming around to the idea of the additional “business logic” layer in cases where ViewModels need data from multiple repositories, for preventing ViewModels from getting too complex, for making testing easier, etc. Or is it better to just have an optional additional layer of repositories? (i.e. for features that are complex, there’s a repository that basically combines data for the other repositories?)

I am about to head to a meeting and will respond more fully later but to answer the repository of repositories case NO. Repositories are a single source of truth, by introducing multiple you break that architectural rule. The temptation to build a repository with other repositories tends to come from the desire to have multiple sources of data. That is one reason to break out the service logic into its own class that does not hold state. The second reason i have seen that developers want to combine logic is that the manipulations are complex on the source data. If the manipulations are pure data that can be handled with a helper class, if they also include visuals then that is a good indication that the view is too complex and should be broken out into a reusable "component" (I forget what word we use for the combination of business logic and widget).

[reidbaker]

I’ve been thinking about topics discussed in previous PRs. I want to pose the following questions, but I’m not suggesting that you make changes to this PR.

blocs/Components/use-cases - I’m coming around to the idea of the additional “business logic” layer in cases where ViewModels need data from multiple repositories, for preventing ViewModels from getting too complex, for making testing easier, etc. Or is it better to just have an optional additional layer of repositories? (i.e. for features that are complex, there’s a repository that basically combines data for the other repositories?)

Either way, the problem is that the app doesn’t need an additional layer right now, and I'm not sure it will. So do we force blocs to provide a good example for larger teams, or do we do what’s right for this app?

I think we force a feature that needs the additional complexity. I think it is easier to explain away that for "reasons" it is important to have a feature that has awkward requirements than it is to ask people to imagine that a feature is more complex in a way that justifies the architecture.

(If we do add another layer, I think we should call them "components" or logic components or something. Blocs means something else to most flutter developers, and use-cases closely mirrors Clean architecture, which comes with a lot of baggage. Teams internal to Google call them some variation of component or bloc.)

Using internal vocab a Component is an independent unit of code that can be dropped onto any page. What makes it independent is that it knows its own dependencies (injected) and can build and rebuild on changes. The code that makes up a component is View (widgets that consume a view model), bloc (business logic that creates a view model) and View Model that is a plain old dart object that contains the parameters needed to inflate the view.

FWIW I if you dont like components then use-cases seems to make the most sense from the names I have seen used. Thinking about this now I like the idea of calling them SuperWidgets since they are widgets just much more complex.

Shared models - This is something @reidbaker had comments about previously. Android docs recommend having shared models for APIs, and then having an additional model for the UI. In the flutter client, you might have ‘class Activity’, which is used by the ViewModels, and in the shared model library you might have class ActivityApiModel. This recommendation makes sense to me in cases where the app is complex, but has the same problem – this app doesn’t need two different models, because they’d be the same. Again, should we force this because our target audience is bigger teams, or do we just wait and see if that need arises.

I think we force a feature that requires the difference because our audience is bigger teams and we need to address where that translation code lives. Something simple here would be to have a json or protobugg endpoint and then convert to a dart object. That has a bonus of being a pattern devs will likely need to interact with.

We could solve these problems by writing about these different options in the docs, but I do think for the first iteration of the docs, it’ll be better to have a single recommendation.

commands - These are new to this PR. This is interesting because it kind of has the same problem as the two previous topics. I think they’re a good idea and should stay in this app, but this particular app could be perfectly functional without them. This makes me think we should be forcing the above recommendations, because it’s unlikely that this sample will ever get complex enough that all of our architectural recommendations make sense here.

Again - not recommending anything change in this PR, just opening up the conversation.

[miquelbeltran]

I think I have to expand a bit on the use of Commands (sorry for formatting as I'm on my phone).

This is something I have seen in some VM examples including Flutter Engineering book and I liked the idea, as I quickly found myself repeating the same tasks (exposing a running state for each action). I've also seen dotnet maui mvvm uses a similar type of object to handle interaction actions.

One task I want to tackle soon is error handling, and I think Commands will be very useful, in order to expose a failed state per action to do imperative actions like showing a SnackBar.

I still have to work on that, so the current Command implementation may not be optimal.

The flutter docs don't really cover this, so it is an opportunity for us to find what could be best suited for our target audience.

[miquelbeltran]

I think we force a feature that needs the additional complexity. I think it is easier to explain away that for "reasons" it is important to have a feature that has awkward requirements than it is to ask people to imagine that a feature is more complex in a way that justifies the architecture.

I think we force a feature that requires the difference because our audience is bigger teams and we need to address where that translation code lives.

I will keep that in mind, I think the "itinerary results" screen will be a good candidate to show that.

Once we have compiled all the itinerary configuration, we need to generate a final itinerary, compiling the selected destination, list of activities, schedule and number of guests. This logic could be located in a component/use case rather than directly in a ViewModel, and will require querying different repositories to obtain the necessary data.

[miquelbeltran]

I continued work on another branch while we review this one. I have simplified a bit the Command class, without callbacks, and allowing Widgets to listen to changes, e.g:

widget.viewModel.updateItineraryConfig.addListener(() {
  if (widget.viewModel.updateItineraryConfig.completed) {
    // consuming the "completed" status, clear the result
    widget.viewModel.updateItineraryConfig.clearResult();
    context.go('/results');
  }
  if (widget.viewModel.updateItineraryConfig.error) {
    // show error
    // retry calling to widget.viewModel.updateItineraryConfig.execute() again
  }
});

This is definitely something that could be done on the ViewModel directly, so it is not strictly necessary to have this Command class for it, and we could remove it if we decide so. On the other hand, it does encapsulate the pattern for executing an action, checking the running state and reacting to a possible outcome, something we have to repeat over and over.

[ericwindmill]

I am about to head to a meeting and will respond more fully later but to answer the repository of repositories case NO. Repositories are a single source of truth, by introducing multiple you break that architectural rule. The temptation to build a repository with other repositories tends to come from the desire to have multiple sources of data. That is one reason to break out the service logic into its own class that does not hold state. The second reason i have seen that developers want to combine logic is that the manipulations are complex on the source data. If the manipulations are pure data that can be handled with a helper class, if they also include visuals then that is a good indication that the view is too complex and should be broken out into a reusable "component" (I forget what word we use for the combination of business logic and widget).

Your explanation makes sense to me, but FWIW I took that idea straight from the Android docs: https://developer.android.com/topic/architecture/data-layer#multiple-levels

I think we force a feature that needs the additional complexity. I think it is easier to explain away that for "reasons" it is important to have a feature that has awkward requirements than it is to ask people to imagine that a feature is more complex in a way that justifies the architecture.

100%, good call. This makes way more sense.

[miquelbeltran]

As talked with Eric, we merge the PR into compass-app. There will be a new PR in the coming days.