ADR051: Add support for more comparisons to conditions

[lfdebrux]

PR Checklist

• If you are proposing a new decision record document, used the right template for that
  - (ADR, decision-record, engagement, research)
• Set yourself as the Assignee
• Tag anyone you would like to review, or @forms-design or @forms-devs
• Fill in the template below

---

What

Proposes changes to the form document schema to support another kind of comparison to trigger a condition for a form route.

We have a need to support multiple options for a selection question going to the same exit page: see the proposed design in Mural [1] for context. Interestingly, we don't need to support multiple different selection questions going to the same exit page, this is not possible in the proposed design.

We think the easiest way to implement this is to allow one condition to be triggered by multiple different selection options. This also has the helpful effect of making a form document shorter if a selection question has lots of conditions that go to the same question.

When considering how to define a condition that is triggered by multiple different selection options, I considered adding a new answer_values property to the condition, but found that continuing to use answer_value but allowing the value to be an object was more backwards-compatible and also more flexible for future possible uses.

The structure of the object describing the comparison was designed based on looking at what other form builders do in their schemas; particular the XGov Digital Form Builder [2] and MoJ Form Builder [3]. I'm open to other suggestions for the structure though.

One thing to consider is whether this proposal is breaking the "you aren't gonna need it" YAGNI principle; while the flexibility is probably more than what is needed for the immediate problem, I think the fact that it's a pain to do what we're trying to do currently and we need to change the form document schema (and raise an ADR) to achieve it suggests that the current schema for conditions in form documents is not flexible enough.

How to review

1. Semantic: Do you agree with the changes?
2. Syntactic: Spelling, grammar, etc.
3. Bikeshedding: Do you agree with the JSON structure used to describe a comparison?

Who can review

Any developers can review this.

@thomasiles and @SamJamCul input into it, but are also free to leave comments!

[theseanything]

What about when different questions have routing conditions to the "same" exit page?

[lfdebrux]

The current design precludes that; exit pages are associated with one and only one question, and can only be routed to from that question.

[theseanything]

The proposed solution is really about enabling more complex routing logic (multi-value comparisons), rather than the problem it's motivated by: deduplicating exit page content.

These are separate axes. The "in" operator dedupes conditions, but the content still lives on the condition. so the moment you want the same exit page from a different question, you're duplicating heading + markdown again.

The core issue is that exit pages aren't normalised into a separate entity.

[lfdebrux]

I agree that having exit pages as a separate entity would be another way of solving this problem, and would in some ways would be preferable. That was one of the other options we considered. Unfortunately, it's a lot more work!

I explored how to migrate exit page content from the conditions model to its own model safely, it would take at least 6 steps, without changing the form document schema (which we should probably do as well). We could do it with less steps with an hour's downtime, but I don't think it would be much quicker.

It also doesn't help that at this point the design for exit pages has not yet been through usability testing, and the design for routing which motivates the design for exit pages is also only just now going through usability testing, so whatever we do could end up being unnecessary work if the result of the usability testing is that we need to make (further) large changes to the design.

The proposed solution is an attempt to find a way to meet the constraints of the designs in a pragmatic way. As you say, it doesn't help with the case where multiple questions go to the same exit page, but as currently the design doesn't allow that, that is not an issue. That extra wrinkle from the design also complicates the case for splitting out the conditions I think; because it's not clear to me now where they should sit in the model or the form document schema.

I also think the extra flexibility would be helpful generally. However, it's definitely a trade-off. It would be nice to have the time to split out conditions properly. We'd need to have a clear steer that the approach in this ADR is not what we want to do.

[theseanything]

Could you talk more about the different options you explored?

[lfdebrux]

There's a bunch of notes in this Google Doc: https://docs.google.com/document/d/1lm1vOcpevxeBJzBc8IDXXouSwBe2q6cjkNX-GWJGRGM/edit?pli=1&tab=t.yuz2c21e5ys4#heading=h.wjfjy9chxzgu

I can add an outline of what was considered if that's helpful? Or would more detail be better?

[theseanything]

Overloading answer_value makes it polymorphic. Despite the backwards-compatible framing, it's effectively a breaking change, a consumer might get an object rather than an expected string.

Have we considered introducing a separate attribute for these new comparisons? (Which would be additive, rather than breaking.)

[lfdebrux]

Hmm, that's a fair point, I hadn't considered the point about consumer expecting a string. I think though since our code doesn't have any expectation about the type of the data in answer_value, it just does a blind comparison with what the answer from the user data is, this isn't in practice a breaking change?

I did consider adding a new attribute, but I thought that was itself also a breaking change, because currently answer_value === null means "always follow this route", but with an additional attribute that would no longer be the case, it would depend on the value of the other attribute.

So I was coming at it from the point of view of our existing code: it would do the wrong thing if it got a form document where there was an extra attribute for multiple answer values, whereas if the form document had something in answer_value which was not a string it would just ignore it and not route (which is still the wrong thing, but less bad than always following the route I think).

[theseanything]

We now have multiple ways of expressing the same logic i.e. answer_type: "example" is equivalent to answer_type: { operator: in, values: ["example"] } - which is confusing.

[lfdebrux]

Yes, that is messy, I agree. I'm not sure how big a problem it is though, what are your thoughts?

[theseanything]

What specifically do the "values" reference? Is the option selection "name" (e.g. the id) or the actual value? (I assume we want the "name" - as we want to be agnostic to the language being used (e.g. welsh or English)

[lfdebrux]

It's the "value" of the selection option we want, the value is language independent, and is intended to be used for routing. See govuk-forms/forms-admin@c7ce55b, govuk-forms/forms-runner#1800.

[theseanything]

Thanks @lfdebrux for your thinking on this. I don't think it's the right model, and we should be normalising exit pages instead.

The duplication this ADR is trying to avoid is really a modelling problem: an exit page has no independent existence, it's just a heading and markdown copied onto every condition that leads to it. Reworking answer_value into a comparison object dedupes conditions and kinda solves it at the wrong layer.

Richer comparisons like "in" are general routing logic, not the fix for exit-page duplication, so I'd design that as its own ADR rather than bake a speculative comparison engine into this one.

I'm going to advocate that we spend the time to get this fundamental model right.

Personally I think we can do this in way the minimise's blocking getting the branching shipped:

1. Add exit pages table + ExitPage model (+ mobility translations)
2. Add relation of condition to exit page (exit_page_id)
3. Surface exit_pages as a new additive field on the FormDocument, and exit_page_id in conditions.
4. Runner preferentially uses exit_page_id, over the existing exit_page_heading, exit_page_markdown
5. New forms using the new branching UI model exit pages in the new way, existing old forms are still supported.
6. After feature launch we can migrate and deprecate the old exit_page_heading/markdown attributes.

[lfdebrux]

Okay, I've opened #258 instead, which proposes creating a separate exit page model.