diff --git a/.munkit/specs/2026-05-08-post-pilot-fixes/brief.md b/.munkit/specs/2026-05-08-post-pilot-fixes/brief.md new file mode 100644 index 0000000..bc4fc9c --- /dev/null +++ b/.munkit/specs/2026-05-08-post-pilot-fixes/brief.md @@ -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 `` 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. diff --git a/.munkit/specs/2026-05-08-post-pilot-fixes/notes.md b/.munkit/specs/2026-05-08-post-pilot-fixes/notes.md new file mode 100644 index 0000000..48ff8e3 --- /dev/null +++ b/.munkit/specs/2026-05-08-post-pilot-fixes/notes.md @@ -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 `
` 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 `
`, not `

`. The `prose` Tailwind plugin + styles `

` margins, not generic `

`. If the body markup is + `
...
...
` rather than `

...

...

`, those + blocks may visually collapse depending on prose configuration. +- Action Text's default `actiontext/contents/_content.html.erb` wraps the + body in `
`. 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 + `
`, and the application stylesheet's `trix-content` + rules (`> * + * { margin-top: 1rem }`) provide the spacing. Stacking + `prose` on top introduced no value for `
`-based Trix paragraphs and + risked subtle conflicts. +- **Image rendering**: added scoped CSS for `figure.attachment` inside + `.trix-content` — clamps `` 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. diff --git a/app/assets/stylesheets/application.css b/app/assets/stylesheets/application.css index ef1c54a..f173dfa 100644 --- a/app/assets/stylesheets/application.css +++ b/app/assets/stylesheets/application.css @@ -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 (`
`). + 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; diff --git a/app/helpers/materials_helper.rb b/app/helpers/materials_helper.rb index d28de22..43ee25f 100644 --- a/app/helpers/materials_helper.rb +++ b/app/helpers/materials_helper.rb @@ -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? @@ -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? diff --git a/app/models/concerns/translatable.rb b/app/models/concerns/translatable.rb index 0a78469..16d6354 100644 --- a/app/models/concerns/translatable.rb +++ b/app/models/concerns/translatable.rb @@ -26,6 +26,11 @@ module Translatable extend ActiveSupport::Concern + included do + class_attribute :_translatable_columns, instance_writer: false, default: [] + before_validation :_compact_translatable_blanks + end + class_methods do # Declares one or more translatable attributes backed by `_translations` # JSONB columns. @@ -35,6 +40,7 @@ module Translatable def translates(*attrs) attrs.each do |attr| column = :"#{attr}_translations" + self._translatable_columns = _translatable_columns + [ column ] define_method(attr) do translations = public_send(column) || {} @@ -65,4 +71,22 @@ def translates(*attrs) end end end + + private + + # Drops locale slots whose value is blank (`nil`, `""`, whitespace-only) from + # every `_translations` JSONB column before validation. Keeps the + # column free of noise that a form may post for unfilled locales, and lets + # readers treat "missing" and "blank" identically. + def _compact_translatable_blanks + self.class._translatable_columns.each do |column| + current = public_send(column) + next unless current.is_a?(Hash) + + cleaned = current.reject { |_, v| v.to_s.strip.empty? } + next if cleaned == current + + public_send(:"#{column}=", cleaned) + end + end end diff --git a/app/views/challenges/_card.html.erb b/app/views/challenges/_card.html.erb index 711e087..09634f2 100644 --- a/app/views/challenges/_card.html.erb +++ b/app/views/challenges/_card.html.erb @@ -1,4 +1,4 @@ -<%# locals: (challenge:) %> +<%# locals: (challenge:, mode: :preview) %> <% accent_class = case challenge.category when "material" then "border-l-imasus-dark-green" @@ -14,12 +14,16 @@ when "business" then "bg-imasus-light-pink/40 text-imasus-dark-green" else "bg-imasus-dark-green/10 text-imasus-dark-green" end -%> -<%= turbo_frame_tag dom_id(challenge) do %> + + public_mode = mode == :public + link_target = public_mode ? challenges_path : preview_challenge_path(challenge.to_param) + link_data = public_mode ? {} : { turbo_frame: "preview" } + + card = capture do %>
- <% if logged_in? %> + <% if logged_in? && !public_mode %>
<%= bookmark_toggle( bookmarkable_type: "Challenge", @@ -38,15 +42,15 @@

- <%= link_to preview_challenge_path(challenge.to_param), + <%= link_to link_target, aria: { label: t("challenges.card.open_label", code: challenge.code) }, - data: { turbo_frame: "preview" }, + data: link_data, class: "before:absolute before:inset-0 before:z-0 before:content-[''] focus:outline-none hover:underline" do %> <%= challenge.question %> <% end %>

- <% if curator? %> + <% if curator? && !public_mode %>
<%= link_to t("challenges.actions.edit"), edit_challenge_path(challenge.to_param), @@ -56,3 +60,8 @@ <% end %>
<% end %> +<% if public_mode %> + <%= card %> +<% else %> + <%= turbo_frame_tag dom_id(challenge) do %><%= card %><% end %> +<% end %> diff --git a/app/views/published_projects/show.html.erb b/app/views/published_projects/show.html.erb index c9a0780..102d11e 100644 --- a/app/views/published_projects/show.html.erb +++ b/app/views/published_projects/show.html.erb @@ -17,11 +17,9 @@

<%= @project.title %>

<% if @project.challenge.present? %> -

- - <%= @project.challenge.code %> - -

+
+ <%= render "challenges/card", challenge: @project.challenge, mode: :public %> +
<% end %> <% if @project.publication_updated_at %> @@ -30,7 +28,7 @@

<% end %> -
+
<%= @project.process_summary %>
diff --git a/test/controllers/published_projects_controller_test.rb b/test/controllers/published_projects_controller_test.rb index 657681a..1280275 100644 --- a/test/controllers/published_projects_controller_test.rb +++ b/test/controllers/published_projects_controller_test.rb @@ -86,6 +86,54 @@ def sign_in(user) assert_select "a[href=?]", edit_project_publication_path(@published), count: 0 end + test "show wraps the process summary in trix-content without a redundant prose wrapper" do + get published_project_url(slug: @published.slug) + assert_response :success + + # ActionText emits the body inside
. The published + # page should rely on .trix-content styling rather than wrap it again in + # `prose`, which has no rules for the
-as-paragraph markup that Trix + # produces and only adds noise. + assert_select ".trix-content" + assert_select ".prose .trix-content", count: 0 + end + + test "show renders the challenge as a full card linking to the challenges page" do + challenge = Challenge.create!( + code: "C6", + category: "material", + question_translations: { "en" => "How might we replace plastics?" }, + description_translations: { "en" => "Framing description for C6." } + ) + @published.update!(challenge: challenge) + + get published_project_url(slug: @published.slug) + assert_response :success + + assert_select "article[data-challenge=?]", "C6" do + assert_select "h3 a[href=?]", challenges_path + assert_select "a[data-turbo-frame=preview]", count: 0 + end + end + + test "show hides the challenge bookmark toggle even when logged in" do + challenge = Challenge.create!( + code: "C7", + category: "design", + question_translations: { "en" => "How might we ..." }, + description_translations: { "en" => "..." } + ) + @published.update!(challenge: challenge) + sign_in(@member) + + get published_project_url(slug: @published.slug) + assert_response :success + + assert_select "article[data-challenge=?]", "C7" do + assert_select ".bookmark-toggle", count: 0 + end + end + test "show returns 404 for a disabled published project" do admin = User.create!(name: "Admin", email: "admin-pp@example.com", password: @password, role: :admin) diff --git a/test/helpers/materials_helper_test.rb b/test/helpers/materials_helper_test.rb index 842a2f4..ae8748a 100644 --- a/test/helpers/materials_helper_test.rb +++ b/test/helpers/materials_helper_test.rb @@ -1,6 +1,8 @@ require "test_helper" class MaterialsHelperTest < ActionView::TestCase + include GlossaryHighlightHelper + # --- materials_chip_toggle_url -------------------------------------------- test "adds the slug to an empty facet" do @@ -70,4 +72,55 @@ class MaterialsHelperTest < ActionView::TestCase assert_not materials_chip_active?("origin_type", "plants", selected_by_facet: {}) end + + # --- material_prose fallback ---------------------------------------------- + + test "material_prose falls back to English when the current-locale slot is missing" do + material = Material.new(description_translations: { "en" => "A natural fiber." }) + + rendered = I18n.with_locale(:es) { material_prose(material, :description) } + + assert_not_nil rendered + assert_includes rendered.to_s, "A natural fiber." + end + + test "material_prose falls back to English when the current-locale slot is an empty string" do + material = Material.new( + description_translations: { "en" => "A natural fiber.", "es" => "" } + ) + + rendered = I18n.with_locale(:es) { material_prose(material, :description) } + + assert_not_nil rendered + assert_includes rendered.to_s, "A natural fiber." + end + + test "material_prose falls back to English when the current-locale slot is whitespace only" do + material = Material.new( + description_translations: { "en" => "A natural fiber.", "es" => " \n " } + ) + + rendered = I18n.with_locale(:es) { material_prose(material, :description) } + + assert_not_nil rendered + assert_includes rendered.to_s, "A natural fiber." + end + + test "material_prose returns nil when the attribute is blank in every locale" do + material = Material.new(description_translations: { "en" => "", "es" => "" }) + + assert_nil I18n.with_locale(:es) { material_prose(material, :description) } + end + + # --- material_meta_description fallback ----------------------------------- + + test "material_meta_description falls back to English when the current-locale slot is empty" do + material = Material.new( + description_translations: { "en" => "Soft heavy rib knit.", "es" => "" } + ) + + meta = I18n.with_locale(:es) { material_meta_description(material) } + + assert_equal "Soft heavy rib knit.", meta + end end diff --git a/test/models/material_test.rb b/test/models/material_test.rb index a2f6229..c68f2d1 100644 --- a/test/models/material_test.rb +++ b/test/models/material_test.rb @@ -28,6 +28,29 @@ def valid_attributes(overrides = {}) assert_equal "A fiber", I18n.with_locale(:it) { record.description } end + # --- Translatable: blank locale slots -------------------------------------- + + test "blank locale slots are dropped from translation columns on save" do + record = Material.create!(valid_attributes( + description_translations: { "en" => "A fiber", "es" => "", "it" => " " }, + sensorial_qualities_translations: { "en" => "Soft", "es" => "" }, + what_problem_it_solves_translations: { "en" => "Replaces nylon", "el" => "\n" } + )) + + record.reload + assert_equal({ "en" => "A fiber" }, record.description_translations) + assert_equal({ "en" => "Soft" }, record.sensorial_qualities_translations) + assert_equal({ "en" => "Replaces nylon" }, record.what_problem_it_solves_translations) + end + + test "blank locale slots are dropped on update too" do + record = Material.create!(valid_attributes) + record.update!(description_translations: { "en" => "Updated", "es" => "", "it" => "" }) + + record.reload + assert_equal({ "en" => "Updated" }, record.description_translations) + end + test "translatable narrative fields are all declared" do record = Material.new( interesting_properties_translations: { "en" => "Breathable" },