Implement loops within application form flows#355
Conversation
There was a problem hiding this comment.
Pull request overview
Adds loop support to the multi-page application form flow DSL, allowing a task to iterate a set of pages over a has_many association on the flow record. Loops generate nested routes under the parent resource, and the controller layer now resolves either the parent flow record or the active child record per page. A mount_flow_routes helper and a flow_record_id helper centralize routing/controller wiring.
Changes:
- New
Strata::Flows::Loopmodel +loopDSL inApplicationFormFlow, withall_pages, namespaced edit/update pathnames on loopQuestionPages, and loop-awareTaskEvaluatortraversal. - New
mount_flow_routesmapper extension that generates top-level member routes for outer pages and nested member routes under each loop's association. - Controller updates target either
flow_recordor@flow_task.loop_record;on_flow_update_invalidnow receivestarget_record; dummy app, schema, factory, locales, views, docs, and a controller spec are updated to demonstrate the pattern.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| app/models/strata/flows/application_form_flow.rb | Adds loop DSL, all_pages, loop-aware action lookup, duplicate-action detection, and mermaid filtering of Loop nodes. |
| app/models/strata/flows/loop.rb | New Loop model with association resolution, optional symbol/proc scope, and started/completed semantics. |
| app/models/strata/flows/question_page.rb | Adds loop attribute, in_loop?, and namespaced + nested-route variants of edit/update pathnames and paths. |
| app/models/strata/flows/task.rb | Generalizes started?/completed?/path to handle Loop entries; adds first-path/first-incomplete helpers (see comment on needed? parity). |
| app/models/strata/flows/task_evaluator.rb | Carries loop_record/loop_page_idx; rewrites prev/next to step within loops, between child records, and out of the loop. |
| app/models/strata/flows/application_form_controller.rb | Adds flow_record_id, threads child id into action lookup, targets loop child for update/validation/save, passes record to invalid hook. |
| lib/strata/flows/routing_extensions.rb | New mount_flow_routes mapper helper generating outer member routes and nested loop member routes. |
| lib/strata/engine.rb | Mixes RoutingExtensions into ActionDispatch::Routing::Mapper. |
| docs/multi-page-form-flows.md | Documents loop syntax, mount_flow_routes, and flow_record_id (see comments on Ruby-syntax examples and bullet capitalization). |
| spec/models/strata/flows/loop_spec.rb | New unit specs for Loop init, scopes, completion, started semantics. |
| spec/models/strata/flows/task_spec.rb | Adds task-containing-loop coverage. |
| spec/models/strata/flows/task_evaluator_spec.rb | Adds traversal coverage across loops, scopes, predicates, and update_path. |
| spec/models/strata/flows/question_page_spec.rb | Adds in-loop pathname and nested route helper coverage. |
| spec/models/strata/flows/application_form_flow_spec.rb | Adds DSL, all_pages, routes, action lookup, and collision-detection coverage. |
| spec/lib/strata/flows/routing_extensions_spec.rb | New routes spec for top-level + nested loop routes via the dummy app. |
| spec/dummy/spec/controllers/sample_application_forms/sample_application_forms_controller_prior_employer_loop_spec.rb | End-to-end controller spec for the loop flow. |
| spec/dummy/db/migrate/20260520120000_create_sample_employment_details.rb | Adds child table for the loop demo. |
| spec/dummy/db/schema.rb | Schema update for new child table. |
| spec/dummy/config/routes.rb | Switches to mount_flow_routes SampleFlow. |
| spec/dummy/config/locales/sample_applications/en.yml | Adds translations for the new task and loop pages. |
| spec/dummy/app/flows/sample_flow.rb | Adds prior_employment task with the loop. |
| spec/dummy/app/models/sample_application_form.rb | Adds has_many :sample_employment_details. |
| spec/dummy/app/models/sample_employment_detail.rb | New child model with per-page validations. |
| spec/dummy/app/controllers/sample_application_forms_controller.rb | Uses flow_record_id for parent lookup. |
| spec/dummy/app/views/sample_application_forms/edit_prior_employer_business_name.html.erb | New loop edit view bound to @flow_task.loop_record. |
| spec/dummy/app/views/sample_application_forms/edit_prior_employer_role.html.erb | New loop edit view bound to @flow_task.loop_record. |
| spec/dummy/spec/factories/sample_application_forms_factory.rb | Adds factory for sample_employment_detail. |
| loop :employment_details, scope: :is_current, do ... | ||
| loop :employment_details, scope: (employment) -> { employment.is_current }, do ... |
|
|
||
| - Looping pages | ||
| - Document upload form fields | ||
| - form fields on associated records |
| # (where params[:id] is the loop child and params["<flow_class>_id"] is | ||
| # the flow record). | ||
| def flow_record_id | ||
| parent_key = "#{controller_name.singularize}_id" | ||
| params[parent_key] || params[:id] |
| def first_incomplete_path(record) | ||
| @pages.each do |item| | ||
| if item.is_a?(Loop) | ||
| item.records_for(record).each do |child| | ||
| incomplete = item.pages.find { |p| !p.completed?(child) } | ||
| return incomplete.edit_path(record, child) if incomplete | ||
| end | ||
| elsif !item.completed?(record) | ||
| return item.edit_path(record) | ||
| end | ||
| end | ||
| nil | ||
| end | ||
|
|
||
| def enter_forward(item, record) | ||
| if item.is_a?(Loop) | ||
| first_child = item.records_for(record).first | ||
| return nil unless first_child | ||
| first_page = item.pages.find { |p| p.needed?(first_child) } | ||
| first_page&.edit_path(record, first_child) | ||
| else | ||
| @pages.find { |page| !page.completed?(record) }.edit_path(record) | ||
| item.needed?(record) ? item.edit_path(record) : nil | ||
| end |
| return nil, nil | ||
| end | ||
|
|
||
| def to_mermaid |
There was a problem hiding this comment.
This method is pretty large and complex. Would you mind cleaning this up a little bit and making it easier to read? Extracting logic to other private methods, etc.
| end | ||
|
|
||
| def find_page_and_task_by_action(flow_record, action) | ||
| def find_page_and_task_by_action(flow_record, action, id_param = nil) |
There was a problem hiding this comment.
This method is pretty dense and complex (ends up having a loop inside of a loop inside of a loop). Would you mind cleaning this method a little to make it more readable?
| # or the first reachable page of a Loop. | ||
| def enter(item, direction) | ||
| if item.is_a?(Loop) | ||
| enter_loop(item, ordered(item.records_for(@record), direction), direction) |
There was a problem hiding this comment.
Mind extracting these method calls to variables? Makes it easier to debug if there are errors raised.
| t.string "leave_type" | ||
| t.datetime "created_at", null: false | ||
| t.datetime "updated_at", null: false | ||
| t.boolean "reviewed", default: false |
There was a problem hiding this comment.
You have this removed in your PR, however when I run migrations locally it adds this column back in. Did you intend to remove this?
| records = loop_node.records_for(@record) | ||
| current_idx = records.find_index(@loop_record) |
There was a problem hiding this comment.
Heads-up on performance once this meets a real ActiveRecord relation: records_for issues the association query, then find_index enumerates the relation in Ruby and ==-compares each loaded row against @loop_record. That's an O(n) row load + object instantiation on every page transition inside a loop — fine for the sample app's two-row fixtures, but it'll show up if a downstream app ever loops over a relation with hundreds of children (e.g. a long employment history, dependents, line items on a claim).
A cheaper variant that avoids loading the rows:
ids = records.pluck(:id)
current_idx = ids.index(@loop_record.id)
return nil unless current_idx
next_id = direction == :forward ? ids[current_idx + 1] : ids[current_idx - 1] if current_idx > 0 || direction == :forward
next_child = next_id && records.find(next_id)…or, since the loop position is really just "which child are we on," you could sidestep the lookup entirely by carrying the child's index (not the record) on the evaluator and resolving the record lazily.
Not blocking — flagging it because the symptom (a noticeably slow "Next" click deep in a long loop) is the kind of thing that's easy to miss in dev and annoying to debug later.
|
Thanks for the feedback @MichaelCrawfordNava! Heads up that I'm going to continue focusing on the paidleave app updates next week, but I will revisit this PR to refine the solution for the SDK shortly after. So you may not see activity in the next few days but it's on my list to come back to. |

Changes
mount_flow_routeshelperflow_record_idhelperBreaking Changes
on_flow_record_invalidmethod must accept a single parameter:target_recordContext
This PR allows forms to insert loops that iterate over a has_many collection. For now, this only supports looping over existing records and assumes that the records are created outside of the loop.
Looping pages create nested routes, e.g.
Testing