Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 119 additions & 0 deletions .munkit/specs/2026-05-08-post-pilot-fixes/brief.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
# Post-pilot fixes

## What

Fix two clusters of bugs discovered during real-user pilot testing:

1. **Material detail page collapses sections in non-English locales** when a
facilitator has edited the entry in English. Sections that have no
translation should fall back to English content instead of disappearing.
2. **Participant-published project page** (`/published/:slug`) has three
distinct issues:
- The challenge is rendered as a small "C6"-style code pill instead of the
full challenge card visitors see on the challenges index.
- User-uploaded images do not render correctly in some cases.
- Multi-paragraph text in the process summary sometimes appears as a
single paragraph.

## Why

Pilot facilitators and participants have started using the app end-to-end. The
material translation regression makes Spanish/Italian/Greek visitors see a
near-empty material page, which undermines the database's value as a shared
multilingual reference. The published project page is the public artefact a
team produces from a workshop — image and paragraph rendering issues plus the
weak challenge framing all degrade the perceived quality of that artefact for
external visitors. These are pilot-feedback fixes, not new features, and should
ship as one tightly-scoped PR.

## Acceptance Criteria

### 1. Material translation fallback

- [ ] On `/materials/:id` in any non-English locale (`es`, `it`, `el`), every
translatable section that has a value in **either** the current locale
or English renders, with content displayed in the available language.
- [ ] When a translatable attribute is blank in the current locale (`nil`,
empty string, or whitespace-only), the section falls back to the
English value and renders it. Section heading and content both appear.
- [ ] When a translatable attribute is blank in **all** locales, the section
is hidden entirely (current behaviour preserved).
- [ ] The same fallback rule applies to the meta-description used for the
page's `<meta name="description">` tag.
- [ ] Fix covers all five translated attributes: `description`,
`interesting_properties`, `structure`, `sensorial_qualities`,
`what_problem_it_solves`.
- [ ] Editing a material in one locale does not write empty strings into the
other locale slots in the JSONB column. Slots that the editor leaves
blank stay absent (or are removed) rather than being persisted as `""`.
- [ ] Tests cover: (a) read fallback when the current-locale slot is an empty
string, (b) read fallback when the slot is missing entirely, (c) save
path does not introduce blank-string slots for other locales.

### 2. Published project — challenge card

- [ ] On `/published/:slug`, the challenge is rendered using the same visual
challenge card used on the challenges index (`challenges/_card.html.erb`).
- [ ] On the published page, the card's primary click target navigates to
`/challenges` (the index page), not to the turbo-frame preview drawer.
- [ ] The bookmark toggle is **hidden** when the card is rendered on the
published page.
- [ ] All other behaviour and content of the card on the challenges index
remains unchanged (preview drawer, bookmark toggle, color-coded border,
category pill).
- [ ] The shared partial accepts a clearly-named option to switch between
these two click/bookmark modes; the published page passes the new mode
and the challenges index keeps its current default.

### 3. Published project — user-uploaded images

- [ ] Inline images embedded in the participant's process summary render
correctly on `/published/:slug`: visible, sensible dimensions, not
broken.
- [ ] The hero image continues to render as it does today.
- [ ] If the issue is in ActionText attachment rendering, the fix lives in
the published-project view layer (partial, helper, or attachment
template) without changing how participants author content.
- [ ] A reproduction case is captured in `notes.md` with the minimal
authoring steps that triggered the bug, so the fix can be verified.

### 4. Published project — paragraph rendering

- [ ] When the stored process-summary HTML contains multiple paragraphs,
they render visibly separated on `/published/:slug`.
- [ ] When the stored HTML is a single paragraph, content still renders;
this spec does not retroactively break long single-paragraph blocks
into multiple paragraphs.
- [ ] The fix is the smallest one that resolves the reported symptom: CSS
adjustment if `prose` styling is collapsing margins, or a rendering
adjustment if the issue is in how ActionText output is wrapped.
- [ ] The reproduction case (what the participant typed and what the stored
HTML looks like) is recorded in `notes.md`.

### Cross-cutting

- [ ] All four fixes ship in **one** Munkit spec and **one** PR.
- [ ] No new translatable strings are introduced beyond those needed by
the challenge-card reuse (none expected; the card already uses `t(...)`).
- [ ] Each behaviour change is covered by a Minitest test that fails before
the fix and passes after.

## Out of Scope

- Backfilling existing materials whose JSONB columns already contain
empty-string slots from prior saves. The read-side fix already handles
these; cleanup can happen later if needed.
- Rewriting the participant editor to enforce paragraph breaks or sanitize
uploaded image dimensions at authoring time.
- Adding new images or content surfaces to the published page.
- Changing the challenges index page itself (other than supporting the new
mode option on the shared card partial).
- Adding analytics or dashboards for pilot feedback.

## Notes

- The translation bug's root cause is the read helper using `_in(locale)`
with `||` short-circuiting on empty strings (which are truthy). See
`notes.md` for the diagnosis and proposed fix shape.
- The challenge card on the published page is a public-facing surface for
anonymous visitors, which is why the bookmark toggle is hidden there.
208 changes: 208 additions & 0 deletions .munkit/specs/2026-05-08-post-pilot-fixes/notes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
# Notes: 2026-05-08-post-pilot-fixes

Diagnosis, scratch space, and open questions for the post-pilot bug-fix slice.

---

## 1. Material translation fallback — diagnosis

### Symptom

Reproduced from a real user report on
`https://app.imasus.eu/materials/pyratex-musa-1`:

- In `en`: description, sensorial qualities, what-problem-it-solves and
interesting-properties sections all render with English content.
- In `es`: those four sections (heading + body) are entirely missing from the
page; only the right-hand sidebar (supplier, availability, raw material,
tags) survives.

### Root cause

Two read helpers in `app/helpers/materials_helper.rb` bypass the
`Translatable` concern's fallback chain and use `_in(locale)` directly:

```ruby
def material_prose(material, attribute)
value = material.public_send(:"#{attribute}_in", I18n.locale) ||
material.public_send(:"#{attribute}_in", Material::BASE_LOCALE)
return nil if value.to_s.strip.empty?
...
end

def material_meta_description(material)
value = material.description_in(I18n.locale) ||
material.description_in(Material::BASE_LOCALE)
...
end
```

`_in(locale)` returns the raw stored value with no fallback. The `||` falls
through only on `nil`, but the form save path is persisting **empty strings**
into the other locales' JSONB slots when the facilitator edits in English.
An empty string is truthy, so `||` does not fall through, and
`value.to_s.strip.empty?` then makes the helper return `nil` → the show view
silently hides the section (`<% next unless prose %>` at
`app/views/materials/show.html.erb:114`).

The view-side fix is a one-character change per helper:

```ruby
value = material.public_send(:"#{attribute}_in", I18n.locale).presence ||
material.public_send(:"#{attribute}_in", Material::BASE_LOCALE)
```

### Why fix the save path too

The `Translatable` setter (`translatable.rb:55-59`) writes whatever the form
hands it. If the form posts an empty string for the locale being edited, the
column accumulates `{ "es" => "", "it" => "" }` noise. That noise is the
ultimate cause of the regression — once it's in the column, any read site
that doesn't use `.presence` will trip over it. Two layers of defence:

- **Read side:** treat blank as missing (this spec).
- **Save side:** drop blank values from the incoming hash before merging, so
the JSONB only contains slots the editor actually filled in.

### Open question — backfill

Existing rows already contain empty-string slots from prior saves. With the
read-side fix, this is harmless: those rows render via the English fallback
correctly. We are explicitly **not** backfilling in this spec; it can be a
small follow-up rake task if the noise becomes a maintenance issue.

---

## 2. Published project — challenge card

### Current state

On `app/views/published_projects/show.html.erb:19-25`, the challenge is a
small uppercase-code pill (just `challenge.code`, e.g. "C6"). On the
challenges index, `app/views/challenges/_card.html.erb` renders a much
richer card: code + category pill, the question as a clickable link, color
band on the left, bookmark toggle.

### Plan

Reuse `app/views/challenges/_card.html.erb` as a shared partial. Add a local
that switches between two modes:

- **Default mode (`mode: :preview`)** — current behaviour: question link uses
`data-turbo-frame="preview"`, bookmark toggle visible. Used on the
challenges index.
- **Public mode (`mode: :public`)** — question link goes to
`challenges_path` (or anchored to the challenge if cheap), no
`data-turbo-frame` attribute, bookmark toggle hidden. Used on the
published-project page.

Pass the mode via `render "challenges/card", challenge:, mode: :public` from
the published-project view.

### Open question — link target on public mode

The brief specifies "navigates to `/challenges`". Options:

1. Plain `challenges_path` — simplest, lands at the index top.
2. `challenges_path(anchor: dom_id(challenge))` — index scrolled to that
challenge, requires the index card to carry the matching DOM id.

Default to option 1 unless option 2 is trivial when implementing.

---

## 3 & 4. Published project — images and paragraphs

### Repro plan

Cannot inspect a real participant's project (no admin access to their data).
Reproduction has to happen locally:

1. Boot dev server, sign in as a participant in a workshop.
2. Create a project, run through publication flow.
3. In the process summary Trix editor:
- Type two short paragraphs separated by a blank line, save, publish, view
`/published/:slug` — does the rendered output show two paragraphs?
- Repeat with a single Enter (line break, not paragraph break).
- Insert one or two images via Trix's attach button, save, publish, view.
4. Inspect the stored `action_text_rich_texts.body` for each case.

### Hypotheses

**Images**
- Trix stores attachments as `<figure data-trix-attachment="...">` with the
Active Storage signed id. ActionText resolves them via
`app/views/active_storage/blobs/_blob.html.erb` (or the default
`actiontext/contents/_content` template). On the published page, the body
is rendered with `<%= @project.process_summary %>` inside a `prose` div.
- Likely culprits: missing variant template, `image_variant_tag` not used,
or the layout's CSP / image proxy preventing the Active Storage redirect
URL from rendering.

**Paragraphs**
- Trix wraps blocks in `<div>`, not `<p>`. The `prose` Tailwind plugin
styles `<p>` margins, not generic `<div>`. If the body markup is
`<div>...</div><div>...</div>` rather than `<p>...</p><p>...</p>`, those
blocks may visually collapse depending on prose configuration.
- Action Text's default `actiontext/contents/_content.html.erb` wraps the
body in `<div class="trix-content">`. The combination of `prose` +
`trix-content` may need a small CSS tweak to give block-level whitespace,
or we render with `actiontext/content` rather than the bare rich text.

These are hypotheses to verify during repro, not pre-decided fixes.

### Open question — paragraph fix scope

If repro shows the issue is purely cosmetic (block elements without
margins), a CSS-only fix in the published-page view is the right answer.
If repro shows participants are entering content as one logical paragraph,
no rendering change can split it — flag in `notes.md` and don't try to
"fix" it server-side.

---

## Open questions summary

- Backfill of empty-string locale slots: out of scope, revisit if needed.
- Public-mode card link target: `/challenges` vs anchored — pick the
cheaper option during implementation.
- Paragraph fix: CSS vs content-shape — defer until reproduction is in
hand.

## Implementation findings (resolved during this spec)

- **Save-side compaction** lives in `Translatable` itself, not in `Material`.
All models using the concern now drop blank-string locale slots before
validation via a shared `_compact_translatable_blanks` callback. This
also fixes the same latent issue for `Tag`, `Challenge`, `GlossaryTerm`,
and `Workshop`, even though the immediate report was about `Material`.
- **Public-mode card link target** picked option 1: plain `challenges_path`.
Anchored navigation would have required the index card to render the
matching DOM id at all viewport sizes; not worth the extra complexity for
a single visitor click.
- **Curator edit button** is also hidden in `:public` mode on the challenge
card. The published page is for anonymous visitors; staff who need to edit
the challenge can do it from the challenges index.
- **Paragraph rendering**: removed the redundant `prose prose-stone max-w-none`
wrapper around `process_summary`. ActionText already wraps its output in
`<div class="trix-content">`, and the application stylesheet's `trix-content`
rules (`> * + * { margin-top: 1rem }`) provide the spacing. Stacking
`prose` on top introduced no value for `<div>`-based Trix paragraphs and
risked subtle conflicts.
- **Image rendering**: added scoped CSS for `figure.attachment` inside
`.trix-content` — clamps `<img>` to container width, centers it, restores
the figure margins that Tailwind preflight strips, and styles the caption.
This fixes both "image overflows the column" and "image looks unstyled".
- **No backfill** of existing materials. The read-side fix already handles
legacy blank-string slots. New writes will be compacted by the
`before_validation` callback.

## Follow-up candidates (not in this spec)

- Rake task to scrub blank-string locale slots from existing rows. Optional
cleanup; not load-bearing now that the read- and save-side fixes are in.
- Editor-side guidance / placeholder for participants on how the published
page lays out their content.
- Reproduction stayed at the Rails-runner level; before next pilot cycle,
consider a system test that boots a real browser, attaches a hero image,
embeds an inline image in Trix, publishes, and asserts both render.
26 changes: 26 additions & 0 deletions app/assets/stylesheets/application.css
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,32 @@ trix-editor.trix-content.trix-content--email-compose {
.trix-content strong { font-weight: 700; }
.trix-content em { font-style: italic; }

/* Inline image and file attachments produced by Trix (`<figure class="attachment">`).
Tailwind's preflight resets figure margins; we restore vertical rhythm and
clamp the image to its container so wide uploads do not overflow the
article column on the published page. */
.trix-content figure.attachment {
margin-block: 1.25rem;
}
.trix-content figure.attachment img {
display: block;
max-width: 100%;
height: auto;
margin-inline: auto;
border-radius: 0.375rem;
}
.trix-content figure.attachment .attachment__caption {
display: block;
margin-top: 0.5rem;
font-size: 0.875rem;
color: rgb(31 61 63 / 0.6);
text-align: center;
}
.trix-content figure.attachment .attachment__caption .attachment__size {
margin-left: 0.25rem;
color: rgb(31 61 63 / 0.45);
}

trix-toolbar .trix-button--text {
min-width: 2.6em;
height: 1.6em;
Expand Down
4 changes: 2 additions & 2 deletions app/helpers/materials_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def materials_chip_active?(facet, slug, selected_by_facet:)
# @param attribute [Symbol] one of {Material::TRANSLATED_ATTRIBUTES}
# @return [ActiveSupport::SafeBuffer, nil]
def material_prose(material, attribute)
value = material.public_send(:"#{attribute}_in", I18n.locale) ||
value = material.public_send(:"#{attribute}_in", I18n.locale).presence ||
material.public_send(:"#{attribute}_in", Material::BASE_LOCALE)
return nil if value.to_s.strip.empty?

Expand Down Expand Up @@ -124,7 +124,7 @@ def material_gallery_items(material)
# @param material [Material]
# @return [String, nil]
def material_meta_description(material)
value = material.description_in(I18n.locale) ||
value = material.description_in(I18n.locale).presence ||
material.description_in(Material::BASE_LOCALE)
return nil if value.to_s.strip.empty?

Expand Down
Loading