Skip to content

Instantly share code, notes, and snippets.

@danielabar

danielabar/tech-walkthrough.md

Last active June 27, 2024 13:13
Show Gist options
  • Save danielabar/2f9a96face575973e6cf3b1eb2f963f6 to your computer and use it in GitHub Desktop.
Save danielabar/2f9a96face575973e6cf3b1eb2f963f6 to your computer and use it in GitHub Desktop.
Download ZIP
Technical walkthrough of adding the Distribution Confirmation feature to the Human Essentials project, as part of Ruby for Good
Raw
tech-walkthrough.md

Table of Contents generated with DocToc

Tech Walkthrough

Walkthrough of Distribution Confirmation, a feature I recently added to the Human Essentials project, run by Ruby for Good.

Ruby for Good is a volunteer-run organization that brings together developers to create software solutions for social good. https://rubyforgood.org/about-us

Human Essentials is one of their projects. It's designed to assist Essentials Banks (such as National Diaper Bank: https://nationaldiaperbanknetwork.org/) in managing and distributing supplies like diapers and hygiene products to charitable groups (partners). It offers features such as inventory management, donation tracking, and tracking the distribution of these supplies.

It's a fullstack Rails application, here is the source on GitHub: https://github.com/rubyforgood/human-essentials, I started volunteering on this project about 2 months ago.

Why?

  • Code is public so it's easier to talk about it when we can see it
  • Mix of client and server side logic
  • Seemingly simple on the surface, with discovered complexities
  • Took multiple iterations to get it right
  • Surprisingly, no "Rails way" of doing this, had to "forge new knowledge"

Demo

Demo working feature:

  • http://localhost:3000/dashboard
  • Nav -> Distributions
  • New Distribution -> Note classic Rails CRUD, eg: http://localhost:3000/distributions/new
  • A Distribution model belongs to a Partner and Storage Location
  • Distribution also has many Line Items - which have a product and quantity
  • Fill out form with valid values and click Save
  • Modal appears showing partner, storage location, and line items
  • Also note some activity in the Rails console POST to /validate endpoint, we'll talk about this later when we get into the code
  • Notice the url is still on the /new path
  • Click No -> modal closes, update quantity
  • click Save -> Yes, model is updated
  • Another demo: Intentionally make the form invalid (forget to fill out Partner), in that case, no modal, it renders with validation errors
  • Also note the modal does NOT appear for edit view, intentional, requirements were to only have it for the new view

Requirements

rubyforgood/human-essentials#3090

Notice earlier attempts in the PRs list.

Challenge

Since requirement mentioned "confirmation page", the original thinking around this ticket was for it to be entirely server rendered, including a new confirmation view, flowing like this:

flowchart TD
    A[New Distribution View]
    B[Confirmation View]
    C[Show View]

    A -->|Save| B
    B -->|Yes I'm sure| create["Distributions Controller create action"]
    create --> C
    B -->|No I need to make changes| A
Loading

How to maintain the distribution details between the New and Confirmation views?

Earlier Attempt Using Session (Abandoned)

When starting on a stale ticket, it's good to look at the history and see what's been previously attempted, to avoid repeating mistakes of the past.

First attempt PR (session) rubyforgood/human-essentials#3703

Concern about using session/cookie due to large storage requirements: rubyforgood/human-essentials#3703 (comment)

This app uses Rails Session Cookie Store: https://api.rubyonrails.org/classes/ActionDispatch/Session/CookieStore.html

# config/initializers/session_store.rb
Rails.application.config.session_store :cookie_store, key: '_diaper_session'

Sessions typically contain at most a user ID and flash message; both fit within the 4096 bytes cookie size limit.

Use Database for Persistence

Suggestion to instead use database for persistence of temporary pending state

flowchart TD
    A[New Distribution View]
    B[Confirmation View]
    C[Show View]

    A -->|Save| createPending["Distributions Controller create pending action"]
    createPending --> B
    B -->|Yes I'm sure| create["Distributions Controller update scheduled action"]
    create --> C
    B -->|No I need to make changes +| A
Loading

+ Actually: If going back to make changes from Confirmation View, can't really go back to "New" view because distribution has already been created. It has to go back to an Edit view, but the app needs to be "aware" that this isn't a fully created distribution yet, because it's still in pending status.

My First Attempt - Experimental, Request (Abandoned)

  • I was concerned about the additional complexity of introducing a new pending state and saving the distribution before the user really meant to.
  • Distributions are a fundamental model and used throughout the app, so a lot of scopes/queries would need to change to exclude it.
  • Also a new job would be needed to clean up "abandoned" distributions.
  • So I spent some time trying to think about a way to do this without having to save a temporary distribution I briefly considered using request to maintain the state in between views, via a GET rather than POST but realized it would suffer from the same issue as session wrt large object size: https://github.com/danielabar/human-essentials/commit/38e5cadd9855f05a2f907bed18349f4a51fef15e
  • limited by url length rather than cookie size, but similar issue
  • and wasn't clear how to restore the request data into the form

My Second Attempt - Persistence with Pending State (Abandoned)

Another attempt PR (persistence in pending state): rubyforgood/human-essentials#4341

  • Purely server side solution because there's a new distribution/confirmation route and controller actions
  • Notice from screenshot the url: /distributions/:id/confirm (this means distribution has been saved in the database at this point, but in a pending status specifically created for the confirmation view)

Worked but complicated:

  • This app has a combination of business logic in controller actions, and calling out to services that perform further business actions
  • Existing DistributionCreateService coupled model saving and inventory updating, which would not be wanted when creating a pending distribution.
  • If user wanted to make changes, needed different logic in editing a pending distribution (again, because of coupling with inventory updates)
  • Arguably there could be some refactoring of business logic, especially wrt inventory updating, but
    • Too much scope creep for what should have been a simple UI change
    • Too much risk to touch inventory logic for UI change
    • Another initiative to change inventory management to event sourcing so it wouldn't have made sense to touch this here.
  • Also required updating ALL scopes/queries of distributions to exclude pending

My Third Attempt - Client Side Eureka!

Then I got another idea of how it could be potentially simpler:

Buy in from core maintainers:

Final Version

This is the final version PR that got merged, with a mix of client side and some server side logic: rubyforgood/human-essentials#4367

Product Review

This is the version of the client side solution that was originally submitted for review, as we'll see, some additional work was required:

Server Side Validation Before Dialog Display

rubyforgood/human-essentials#4367 (comment)

Analysis re: validation: rubyforgood/human-essentials#4367 (comment)

Commit to implement validation as "pre check": https://github.com/rubyforgood/human-essentials/commit/1eedd009f54f17a90dec4d9626a105bc68f82725

Changes:

  • Distribution form specifies validation path as input to Stimulus controller using value
  • Stimulus controller submits form to JSON validation endpoint
  • Validation-only endpoint added to distributions controller server side

Collapse Same Line Items and Quantities

rubyforgood/human-essentials#4367 (comment)

Resolved, was still client side at this point: https://github.com/danielabar/human-essentials/commit/f7bd90d92b40334d5f7da7ebc26d381c5b387e51

At this point, it was functioning well from a product perspective.

Next up, tech review...

Tech Review

CSRF

Had skipped csrf check on validation only endpoint: rubyforgood/human-essentials#4367 (comment) because was getting invalid token errors, even though the token was being included in the request.

After much investigation, turns out there are two csrf tokens: one in the form which is specific to that form (create distribution), and one in the meta tag that is good for general purpose requests (such as custom validate only endpoint).

Debugged through the Rails source using bundle open actionpack: ActionPack to figure this out: rubyforgood/human-essentials#4367 (comment)

Solution to csrf token and also system test maintenance re: when system tests run, there is no meta token

Move JS "Templating" Server Side

Further simplification of JS "templating" to show line items - move logic to server side that can also combine the line items, and return validate.html.erb which can be set in the modal as innerHTML:

Feedback

Insights

  • Just because requirements are worded a certain way, doesn't necessarily mean do literally that, it's useful to to think about the problem more broadly (eg: rather than confirmation "page", this could also be implemented as a client side modal, which still resolves the issue of user needs to confirm before submit).
  • Focus on lowest risk solution, especially for a volunteer project where no one's on call if something goes wrong.
  • Re-usability by using Stimulus value (i.e. input) for the preCheckPath rather than hard-coded validation url. Will see how much of this can be re-used on my next task for partner request confirmation: rubyforgood/human-essentials#3052.
  • Written communication matters, it's not just about the code, it's also important to be able to communicate with product, other engineers.
  • No solution is perfect, there will always be problems - approach each one with curiosity and an opportunity to learn something new!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment