Middleware that acts as NavigatorObserver

[kentcb]

Hello! Here's another tasty design question with respect to redux and flutter...

I need to support undo in my app, so that users can wantonly and confidently enact destructive changes, and also so that state management is generally easier. In my case, I have a hierarchy of items, and items within those items. Users can add/edit/delete (and undo those deletions). So something as simple as hitting an ADD button, then hitting the navigation's back button needs to intelligently manage state.

My implementation is based on a stack of immutable (built) DTOs that are within my state. Then there are three actions to manage this state (BeginTransaction, CommitTransaction, RollbackTransaction). This part was really easy to get up and running. The reducers just do what you'd expect, and include assertions to ensure that the state maintains its integrity (always at least one item on the stack).

The hard part was figuring out where to dispatch these actions and finding a way to somewhat guarantee that transactions will remain balanced. Initially, I just sprinkled them through my views. It worked, but was terrible and I'm pretty sure there were some edge cases that I'd missed. I abandoned that and attempted to manifest transactions solely within middleware. Sounds an attractive proposition, but it was difficult/impossible to achieve cleanly.

Here's a look at my middleware that handles this:

class Transactions extends NavigatorObserver {
  Transactions(globals.CompositeNavigatorObserver compositeNavigatorObserver) {
    compositeNavigatorObserver.addObserver(this);
  }

  List<Middleware<AppState>> getMiddlewareBindings() => [
        new TypedMiddleware<AppState, AddEntry>(_beginTransactionReducer),
        new TypedMiddleware<AppState, RemoveEntry>(_beginTransactionReducer),
        new TypedMiddleware<AppState, AddAllowance>(_beginTransactionReducer),
        new TypedMiddleware<AppState, RemoveAllowance>(_beginTransactionReducer),
      ];

  void _beginTransactionReducer(Store<AppState> store, dynamic action, NextDispatcher next) {
    _beginTransaction(store);
    next(action);
  }

  void _beginTransaction(Store<AppState> store) {
    store.dispatch(new BeginTimesheetTransaction());
  }

  void _commitTransaction(Store<AppState> store) {
    store.dispatch(new CommitTimesheetTransaction());
  }

  void _rollbackTransaction(Store<AppState> store) {
    store.dispatch(new RollbackTimesheetTransaction());
  }

  @override
  void didPush(Route<dynamic> route, Route<dynamic> previousRoute) {
    var store = globals.store;
    var name = route.settings.name;

    if (name == RouteNames.editEntryJobDetails || name == RouteNames.editEntryAllowance) {
      _beginTransaction(store);
    }
  }

  @override
  void didPop(Route<dynamic> route, Route<dynamic> previousRoute) {
    var store = globals.store;
    var name = route.settings.name;

    if (route is MaterialPageRoute<dynamic>) {
      if (name == RouteNames.addEntryJobDetails || name == RouteNames.editEntryJobDetails || name == RouteNames.addEntryAllowance || name == RouteNames.editEntryAllowance) {
        route.completed.then((result) {
          if (result == true) {
            _commitTransaction(store);
          } else {
            _rollbackTransaction(store);
          }
        });
      }
    }
  }
}

And here are the main problems I have with this approach:

• I had to create a CompositeNavigatorObserver class, which just manages any number of NavigatorObserver instances, forwarding all events onto them. I pass this CompositeNavigatorObserver into my MaterialApp. This allows the middleware to monitor and react to navigation events. I could not think of a cleaner way.
• Ugh, look how I sometimes use the global Store instance, and other times it's passed in so I don't have to. I also used Scaffold.of at one point, but that was no better (and was messier).
• Not the end of the world, but now I have to give my routes names because otherwise I have no sane means of triggering transactional logic based on navigation events. Moreover, those names need to distinguish between add/edit scenarios, because the transactional approach is different (with add, the transaction begins before the item is added, with edit there is no specific action so I need to trigger it upon navigation - hmm, maybe I should have a edit actions... 🤔 ).
• In addition, my pages need to pop true when editing completes successfully. This was also a bit messy (in a separate piece of middleware).
• Really not sure how I'd test this effectively, especially given the dependency on global state. I could pass in a delegate that is used to resolve the state, I suppose.

OK, sorry for the rando post again. Really just interested in hearing any thoughts on this direction or how it could be improved.

Thanks!

[brianegan]

Kent! Stop finding all the tough edge cases!!! :P Jokes, of course, really interesting problem, thanks for writing in :)

I'll be honest, even though you've provided a really good description, I feel like I don't have enough of a handle on your problem to give you solid advice because it's a tricky one with Edge cases I probably don't understand. But let me try...

The hard part was figuring out where to dispatch these actions and finding a way to somewhat guarantee that transactions will remain balanced. Initially, I just sprinkled them through my views. It worked, but was terrible and I'm pretty sure there were some edge cases that I'd missed.

Since the above does look complex, and I could also see edge cases creeping in, could you test your views to ensure they're firing the right actions at the right times?

Or could you perhaps elaborate on why it was terrible? I'd be interested in hearing the problems it created.

Since it seems starting / stopping these actions is related to showing / hiding Widgets, that still feels like the cleanest way given my limited knowledge.

[kentcb]

Haha! I'm very grateful that I have you to bounce ideas and questions off, Brian. Feeling a bit lonely as the only flutter/redux/Dart dev at my company...

Perhaps this diagram I whipped up provides a better visualization for my scenario:

So you can see that items consist of sub-items. Users can add or edit items, and add or edit sub-items within those items. At any point in the process, they could bail out by hitting the back button, and what that does is contingent upon where they are in the process. Moreover, because the data has multiple levels to it (items and sub-items), I can simply take a single snapshot. Otherwise, a user might start editing an item, start editing a sub-item within that item, then bail out on editing that sub-item. Doing so should roll back only the sub-item changes, not the entire item.

Anyway, the point is that whilst I have all this working, it's a bit icky to me. Also, not evident from the above diagram (it was getting too busy!) is the fact that items (and sub-items) can also be deleted via a swipe gesture. When this happens, a transaction is started, the (sub)item is deleted, and a snackbar is shown with an UNDO action. If the user hits UNDO, the transaction is rolled back. If the snackbar is dismissed for any other reason, the transaction is committed. Alas, I had to drive this part of the process from the UI rather than middleware, since I simply couldn't find a sane way to do otherwise.

Not sure if I've made things clearer here or worse 🙃 No real expectations for an answer or anything. I guess I just wondered whether I might be overlooking some simpler way of achieving this and having the code for it all in one spot as middleware.

[brianegan]

Heya @kentcb -- even with the detailed breakdown, still not quite sure I could come up with a good solution without sitting down with ya for a bit and hacking away at it! I can't think of anything obvious, as it does seem like there's some tricky logic in there and it would be tough to give an obvious solution without working through all the corner cases :)

[kentcb]

Yep, totally understand @brianegan. Thanks for having a look anyway. I'm more or less happy with how my code is functioning - I'm just not certain it's the best design. I've chalked it up as potential tech debt and moved on.