Simplify data model/data flows

[ikusteu]

I was fixing a minor bug: sorting of customer area calendar slots intraday. The fix was very quick, but when I went to write the tests for the behaviour, it was a nightmare. We have a really complex state setup for some tests (namely data retrieval - redux selectors).

This got me thinking, again, about the complexity of the data model and the data flows.

I don't have a clear solution, but am opening this to discuss possible solutions, as well as when would we have some time to work on this (as it's not really a high-priority feature work).

Some things I'd like integrated:

• usage of iterables/generators like we did in librocco - we could copy/paste the iterable methods (wrapIter and the rest), or we could, maybe create a separate repo and publish those as a package (I would call it @code-myriad/toolbox or something like that)
• simplify the data model (firestore) or replace firestore altogether with supabase (for instance), in order to use a more structured (postgres) database
• add some additional data structures that would make our job easier:
  • I was thinking of adding a sorted array as a wrapper around built in Array prototype:
    • I would have it extend the array (keep array's internal data structure), but expose .add method (or override the .push method) so that each time a new item is added, it's .spliced into the array so that the array is sorted
    • initially, each time a new item is added, we could simply use Array.prorotype.findIndex to find the location where to splice in the new value
    • additionally, if we need better performance (which I don't see happening in eisbuk), we could use binary search (in place of .findIndex, which, I think, iterates linearly, or, if really needed, we could use a binary tree or a B-tree to store the data)
    • additionally, each time the structure is instantiated, it could accept a compare function, so that it can me used with more complex data types (say we want to use sorted array of slots and we want to sort them by date, we would pass in a compare function comparing a.date and b.date
    • finally, we could add a .reverse method, which overrides [Symbol.iterator] to count backwards (using a for loop) and yield the elements in reverse order
    • the only caveat here is that firestore would probably accept this structure (as it's an array under the hood), but when an item is read from firestore, we would need to wrap it with additional methods (which can be solved by creating document mappings for firestore docs, kinda like Python couchdb library does for couchdb docs)
• try and write the selectors in a more composable manner - so that we don't need to mock the entire state (for booked slots, we need to create customer's bookings collection, bookedSlots, currentDate and slotsByDay for a simple test) and so that the selectors could be tested in isolation

As a prerequisite for this, I think we should have extensive e2e tests as both the implementation and tests would probably be changed during this rehaul.
We should probably focus on making the app work as is right now (solve all the bugs, extend tests and make it usable in production) and then do any or all of the work described here as a 2.0 or something as this is more aimed at maintainability and reliability, then the core functionality itself.

[silviot]

I have a lot of sympathy for the underlying ideas and motivations of this proposal.
The three points listed as possible future enhancements are of two wildly different kinds: incremental enhancement (point 1, 3 and 4) and radical architectural changes (point 2).
I think moving away from firestore would be such a big change that, at that point, we'd have an easier time doing a complete rewrite, maybe using Svelte. But I don't see that happening, as enticing as it is.

Regarding point 3, my gut feeling is that the proposed Array wrapper would introduce many caveats and solve few problems. I see risks, but not adavantages in using that in place of sorting arrays on the fly before they hit the UI.

[ikusteu]

@silviot I completely understand (both the sympathy and restrictions). I was going to suggest some data model changes, but all of those come with some trade offs. As for the Supabase migration...yes it would reduce a lot of headache (at this point I'm discontent with Firebase as much as I am with React, or even more). What I was going to suggest is to definitely aim towards better modularity and composability of our codebase: this can be incremental and has directly to do with some points noted above (1, 3, 4), but also, indirectly it would make it easier to make a switch at some point in the future.

My suggestions regarding the incremental modularity (all of these should be doable in complete isolation):

• ditch react-redux-firebase-firestore:
  • I wrote this package, inspired by the popular react-redux-firebase package, which didn't at the time have support for the new firebase (I can't remember the version jump, but treeshakable API, using more functions than classes).
  • The original package, being written as a library for wide usage used a lot of generalised functionality and a redux (singleton) store to communicate everything
  • I suggest we simplify that and take more control of our subscriptions (have them be view/collection specific) and ditch the whole "declarative" react-way of doing things
  • Even though we're using React, we can still use view specific hooks and context to achieve data flows similar to what we'd have had we used Svelte
  • As stated, this doesn't have to be a huge switch - we can use the setup we have so far, but rewrite the data flows page by page (having the rest still use the current data flows)

[ikusteu]

Some suggestions for data model changes:

I've thought of ditching slotsByDay altogether, as I thought it was a query optimisation. Later I've remembered this is an optimisation for the sake of making fewer requests (and lowering the firebase bill). Having thought about that...it might have been a premature optimisation, which is definitely valid, but makes for more moving parts (data triggers) and the potential problems stemming from it. I'm not saying we should definitely lose the slotsByDay, but I'm voicing it out as one of potential steps to reduce the mismatch of documents (as we had in #804 and #807) in case those errors persist.

Additionally, I've thought of merging attendance entries with slots entries, but then we would lose the benefit of slotsByDay (fewer document reads each time admin interacts with the attendance view) and, if we we're to lose slotsByDay altogether, we wouldn't really like the non-admins (who would, in that case, need to receive slots) to receive attendance data (as part of slot entry)...

[ikusteu]

Ok, after much thinking and @silviot's comment on atomicity of firestore updates, I have the following idea: We could refactor the db interactions to not use data triggers at all, but create a db interface where each action does exactly what it's supposed to, nothing more and nothing less. Then we could thoroughly test each interaction and have it be done using firestore.runTransaction, which, AFAIK, should preserve transaction integrity. Additionally, we could wrap each action to send error reports to sentry automatically.

The drawback of this would be that we would need to think about how each action affects the entire state (which, it feels, we now have to do ex post, when sourcing the bug)