From 836308471f8d31ccdcdd3a5bd88bc76cc1c0831b Mon Sep 17 00:00:00 2001 From: erdgeist Date: Mon, 20 Jul 2026 19:45:43 +0200 Subject: Make headline images explicit, add asset credits - related_assets gains a `headline` boolean (DB-enforced: at most one per page), replacing "first image by position" as the headline rule. A rake task backfills the current first image on every live head/draft, so nothing changes visually until an editor changes it. - The image picker sidebar gets a star toggle reflecting the flag; the TinyMCE inline-image picker's badge now reads it too, instead of assuming position 0. - No headline chosen (or none attached) now falls back to the gallery-count caption itself becoming the lightbox trigger, instead of the gallery being unreachable. - Assets gain creator, source_url, and license_key (against a new config/asset_licenses.yml dictionary). asset_credit renders a degrading attribution line, reused as a hidden per-image glightbox caption so credit is one click away for every image, not only the headline's always-visible one. - Fixed: asset thumbnails rendered unconditionally regardless of whether a real variant exists on disk. Asset#has_variant? checks file existence, not content type -- some legacy PDFs have real pre-rewrite thumbnails a content-type check would have hidden. - assets#new/edit rebuilt onto the same node_description/node_content layout as assets#show, picking up the three new fields in the process. --- test/controllers/related_assets_controller_test.rb | 46 +++++++++++++ test/models/asset_license_test.rb | 31 +++++++++ test/models/asset_test.rb | 23 ++++++- test/models/helpers/content_helper_test.rb | 78 +++++++++++++++++++++- test/models/related_asset_test.rb | 44 ++++++++++-- 5 files changed, 215 insertions(+), 7 deletions(-) create mode 100644 test/models/asset_license_test.rb (limited to 'test') diff --git a/test/controllers/related_assets_controller_test.rb b/test/controllers/related_assets_controller_test.rb index 9d20cbb..2384adc 100644 --- a/test/controllers/related_assets_controller_test.rb +++ b/test/controllers/related_assets_controller_test.rb @@ -88,4 +88,50 @@ class RelatedAssetsControllerTest < ActionController::TestCase ordered_asset_ids = node.draft.reload.related_assets.map(&:asset_id) assert_equal [second.id, first.id], ordered_asset_ids end + + test "update sets the headline flag" do + login_as :quentin + node = Node.root.children.create!(:slug => "related_assets_headline_test") + asset = Asset.create!(:name => "headline-photo", :upload_content_type => "image/png") + node.draft.assets << asset + related = node.draft.related_assets.find_by(:asset_id => asset.id) + + patch :update, params: { :node_id => node.id, :id => related.id, :headline => "true" } + + assert_response :success + assert related.reload.headline? + end + + test "update with headline=true clears any previous headline on the same page" do + login_as :quentin + node = Node.root.children.create!(:slug => "related_assets_headline_swap_test") + first = Asset.create!(:name => "first-headline", :upload_content_type => "image/png") + second = Asset.create!(:name => "second-headline", :upload_content_type => "image/png") + node.draft.assets << first + node.draft.assets << second + + first_related = node.draft.related_assets.find_by(:asset_id => first.id) + second_related = node.draft.related_assets.find_by(:asset_id => second.id) + first_related.update!(:headline => true) + + patch :update, params: { :node_id => node.id, :id => second_related.id, :headline => "true" } + + assert_response :success + assert_not first_related.reload.headline? + assert second_related.reload.headline? + end + + test "update with headline=false clears the headline" do + login_as :quentin + node = Node.root.children.create!(:slug => "related_assets_headline_unset_test") + asset = Asset.create!(:name => "unset-headline", :upload_content_type => "image/png") + node.draft.assets << asset + related = node.draft.related_assets.find_by(:asset_id => asset.id) + related.update!(:headline => true) + + patch :update, params: { :node_id => node.id, :id => related.id, :headline => "false" } + + assert_response :success + assert_not related.reload.headline? + end end diff --git a/test/models/asset_license_test.rb b/test/models/asset_license_test.rb new file mode 100644 index 0000000..9f2d04f --- /dev/null +++ b/test/models/asset_license_test.rb @@ -0,0 +1,31 @@ +require 'test_helper' + +class AssetLicenseTest < ActiveSupport::TestCase + test "find returns a populated entry for a known key" do + entry = AssetLicense.find("cc_by_4") + + assert_equal "cc_by_4", entry.key + assert_equal "https://creativecommons.org/licenses/by/4.0/", entry.url + assert entry.requires_attribution + assert_equal "license", entry.style + end + + test "find returns nil for an unknown or blank key" do + assert_nil AssetLicense.find("not_a_real_license") + assert_nil AssetLicense.find(nil) + assert_nil AssetLicense.find("") + end + + test "keys includes every dictionary entry" do + assert_includes AssetLicense.keys, "cc0" + assert_includes AssetLicense.keys, "own_work" + end + + test "a note-style entry has no attribution requirement and no url" do + entry = AssetLicense.find("own_work") + + assert_not entry.requires_attribution + assert_nil entry.url + assert_equal "note", entry.style + end +end diff --git a/test/models/asset_test.rb b/test/models/asset_test.rb index a1041e4..d246abe 100644 --- a/test/models/asset_test.rb +++ b/test/models/asset_test.rb @@ -19,5 +19,26 @@ class AssetTest < ActiveSupport::TestCase assert_equal 0, draft.assets.length assert_equal 0, RelatedAsset.count end - + + test "image? is true for supported image content types" do + assert Asset.new(:upload_content_type => "image/png").image? + assert Asset.new(:upload_content_type => "image/jpeg").image? + end + + test "image? is false for non-image content types" do + assert_not Asset.new(:upload_content_type => "application/pdf").image? + assert_not Asset.new(:upload_content_type => nil).image? + end + + test "license_key must be a known dictionary key" do + asset = Asset.new(:license_key => "not_a_real_license") + I18n.with_locale(:en) do + assert_not asset.valid? + assert_includes asset.errors[:license_key], "is not included in the list" + end + end + + test "license_key may be blank" do + assert Asset.new(:license_key => nil).valid? + end end diff --git a/test/models/helpers/content_helper_test.rb b/test/models/helpers/content_helper_test.rb index a7ed478..d6c7b43 100644 --- a/test/models/helpers/content_helper_test.rb +++ b/test/models/helpers/content_helper_test.rb @@ -2,7 +2,81 @@ require 'test_helper' class ContentHelperTest < ActionView::TestCase test "weekday_abbr delegates through the current I18n locale" do - I18n.locale = :de - assert_equal "Mo", weekday_abbr(Time.parse("2026-07-06")) + I18n.with_locale(:de) do + assert_equal "Mo", weekday_abbr(Time.parse("2026-07-06")) + end + end + + test "asset_credit returns nil for a nil asset" do + assert_nil asset_credit(nil) + end + + test "asset_credit returns nil when creator, source, and license are all blank" do + assert_nil asset_credit(Asset.new(:name => "blank")) + end + + test "asset_credit renders creator, source link, and license link when all are present" do + asset = Asset.new(:name => "demo", :creator => "Jane Doe", + :source_url => "https://example.org/photo", :license_key => "cc_by_4") + + result = I18n.with_locale(:en) { asset_credit(asset) } + + assert_match %r{Photo demo}, result + assert_match "by Jane Doe", result + assert_match %r{Licensed under CC BY 4.0}, result + end + + test "asset_credit falls back to plain text when source_url is blank" do + asset = Asset.new(:name => "demo", :creator => "Jane Doe", :license_key => "cc_by_4") + assert_no_match %r{Photo demo}, I18n.with_locale(:en) { asset_credit(asset) } + end + + test "asset_credit omits the 'by' clause when creator is blank" do + asset = Asset.new(:name => "demo", :source_url => "https://example.org/photo", :license_key => "cc_by_4") + assert_no_match "by ", I18n.with_locale(:en) { asset_credit(asset) } + end + + test "a note-style license renders with no 'Licensed under' prefix and no link" do + asset = Asset.new(:name => "demo", :creator => "CCC", :license_key => "own_work") + result = I18n.with_locale(:en) { asset_credit(asset) } + + assert_match "Own work", result + assert_no_match "Licensed under", result + assert_no_match " "demo", :creator => "Jane Doe") + asset.license_key = "a_retired_key" + + result = I18n.with_locale(:en) { asset_credit(asset) } + + assert_match "Photo demo by Jane Doe", result + assert_no_match "Licensed under", result + end + + test "headline_image renders nothing when no images are attached" do + node = Node.root.children.create!(:slug => "headline_image_empty_test") + @page = node.draft + assert_nil headline_image + end + + test "headline_image renders the flagged headline asset" do + node = Node.root.children.create!(:slug => "headline_image_flagged_test") + asset = Asset.create!(:name => "flagged", :upload_content_type => "image/png") + node.draft.assets << asset + node.draft.related_assets.find_by(:asset_id => asset.id).update!(:headline => true) + @page = node.draft + + assert_match "glightbox", headline_image + end + + test "headline_image falls back to a gallery-open caption when no headline is flagged" do + node = Node.root.children.create!(:slug => "headline_image_no_flag_test") + asset = Asset.create!(:name => "unflagged", :upload_content_type => "image/png") + node.draft.assets << asset + @page = node.draft + + I18n.with_locale(:en) { assert_match t(:open_gallery), headline_image } end end diff --git a/test/models/related_asset_test.rb b/test/models/related_asset_test.rb index a739e6b..710b4cc 100644 --- a/test/models/related_asset_test.rb +++ b/test/models/related_asset_test.rb @@ -1,8 +1,44 @@ require 'test_helper' -class RelatedImageTest < ActiveSupport::TestCase - # Replace this with your real tests. - test "the truth" do - assert true +class RelatedAssetTest < ActiveSupport::TestCase + test "headline can be set on an image asset" do + node = Node.root.children.create!(:slug => "related_asset_headline_image_test") + asset = Asset.create!(:name => "photo", :upload_content_type => "image/png") + node.draft.assets << asset + related = node.draft.related_assets.find_by(:asset_id => asset.id) + + related.headline = true + assert related.valid? + end + + test "headline cannot be set on a non-image asset" do + node = Node.root.children.create!(:slug => "related_asset_headline_pdf_test") + asset = Asset.create!(:name => "programme", :upload_content_type => "application/pdf") + node.draft.assets << asset + related = node.draft.related_assets.find_by(:asset_id => asset.id) + + related.headline = true + assert_not related.valid? + assert_includes related.errors[:headline], "can only be set on image assets" + end + + test "the headline validation does not raise when asset is missing" do + related = RelatedAsset.new(:headline => true) + assert_not related.valid? + end + + test "at most one headline per page is enforced at the database level" do + node = Node.root.children.create!(:slug => "related_asset_headline_uniqueness_test") + first = Asset.create!(:name => "first", :upload_content_type => "image/png") + second = Asset.create!(:name => "second", :upload_content_type => "image/png") + node.draft.assets << first + node.draft.assets << second + + node.draft.related_assets.find_by(:asset_id => first.id).update!(:headline => true) + second_related = node.draft.related_assets.find_by(:asset_id => second.id) + + assert_raises(ActiveRecord::RecordNotUnique) do + RelatedAsset.where(:id => second_related.id).update_all(:headline => true) + end end end -- cgit v1.3