diff --git a/app/forms/concerns/hyrax/redirects_field_behavior.rb b/app/forms/concerns/hyrax/redirects_field_behavior.rb new file mode 100644 index 0000000000..bac11ef490 --- /dev/null +++ b/app/forms/concerns/hyrax/redirects_field_behavior.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +module Hyrax + # Form-side handling for the `redirects` nested-attribute field. + # + # Submitted form payloads arrive under `redirects_attributes` and are + # turned into plain hashes (the persisted shape — see + # `config/metadata/redirects.yaml`, `type: hash`) by the populator. + # On render, the prepopulator wraps each persisted hash in a + # `Hyrax::Redirect` value object so the view can call `.path` / + # `.canonical` / `.sequence`. + # + # The `deserialize!` override removes the renamed `redirects` key + # before Reform's `from_hash` runs, so the form's `redirects` + # property is written exclusively by the populator. See + # `Hyrax::BasedNearFieldBehavior` for the parallel pattern. + # + # ## Feature gating + # + # `self.included` runs at class load time and uses the structural + # gate `Hyrax.config.redirects_enabled?`. The runtime Flipflop + # check isn't meaningful here because the form class is being + # defined, not handling a request. + # + # All runtime-side methods (`deserialize!`, populator, prepopulator) + # delegate to `Hyrax.config.redirects_active?`, the two-gate + # combinator (`redirects_enabled? && Flipflop.redirects?`). Calling + # `Flipflop.redirects?` directly is unsafe when the config is off + # because the feature isn't registered in that case. + module RedirectsFieldBehavior + def self.included(descendant) + return unless Hyrax.config.redirects_enabled? + descendant.property :redirects_attributes, + virtual: true, + populator: :redirects_attributes_populator, + prepopulator: :redirects_attributes_prepopulator + end + + # Reform's FormBuilderMethods rewrites `redirects_attributes` → + # `redirects` before `from_hash` runs. Strip the renamed key so + # the populator on `redirects_attributes` is the single entry + # point for form-driven writes. + def deserialize!(params) + result = super + if Hyrax.config.redirects_active? && result.respond_to?(:delete) + result.delete('redirects') + result.delete(:redirects) + end + result + end + + private + + # Builds plain hashes (the persisted shape) from the submitted + # `redirects_attributes` payload. Drops rows marked for destruction + # or with a blank path. Normalizes paths to the canonical form + # stored in the uniqueness ledger. + def redirects_attributes_populator(fragment:, **_options) + return unless respond_to?(:redirects) + return unless Hyrax.config.redirects_active? + entries = Array(fragment&.values) + .reject { |row| row['_destroy'].to_s == 'true' || row['path'].to_s.strip.empty? } + .each_with_index.map do |row, i| + { 'path' => Hyrax::RedirectPathNormalizer.call(row['path']), + 'canonical' => row['canonical'].to_s == 'true', + 'sequence' => row['sequence'].presence&.to_i || i } + end + self.redirects = entries + end + + # Wraps each persisted hash in a `Hyrax::Redirect` value object for + # the form view. Mirrors how `BasedNearFieldBehavior` hydrates URI + # strings into `ControlledVocabularies::Location` instances. + def redirects_attributes_prepopulator + return unless respond_to?(:redirects) + return unless Hyrax.config.redirects_active? + self.redirects = Array(redirects).map { |entry| Hyrax::Redirect.wrap(entry) } + end + end +end diff --git a/app/forms/hyrax/forms/resource_form.rb b/app/forms/hyrax/forms/resource_form.rb index 5e243707aa..1c7b63e50d 100644 --- a/app/forms/hyrax/forms/resource_form.rb +++ b/app/forms/hyrax/forms/resource_form.rb @@ -27,6 +27,7 @@ class ResourceForm < Hyrax::ChangeSet # rubocop:disable Metrics/ClassLength end include BasedNearFieldBehavior + include RedirectsFieldBehavior class_attribute :model_class property :human_readable_type, writable: false @@ -112,9 +113,13 @@ def initialize(deprecated_resource = nil, resource: nil) class << self def inherited(subclass) - # this is a noop if based near is not defined on a given model - # we need these to be before and included properties + # Field Behaviors must be prepended onto every subclass so their + # `deserialize!` overrides land above Reform's base method on the + # ancestor chain. Each behavior gates itself internally and is a + # no-op when its feature is off or its property isn't on the + # subclass's model. subclass.prepend(BasedNearFieldBehavior) + subclass.prepend(RedirectsFieldBehavior) super end diff --git a/app/helpers/hyrax/collections_helper.rb b/app/helpers/hyrax/collections_helper.rb index 3fef59899d..09374db983 100644 --- a/app/helpers/hyrax/collections_helper.rb +++ b/app/helpers/hyrax/collections_helper.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true module Hyrax module CollectionsHelper # rubocop:disable Metrics/ModuleLength + include Hyrax::RedirectsTabHelper + ## # @since 3.1.0 # @return [Array] diff --git a/app/helpers/hyrax/redirects_tab_helper.rb b/app/helpers/hyrax/redirects_tab_helper.rb new file mode 100644 index 0000000000..28eee88544 --- /dev/null +++ b/app/helpers/hyrax/redirects_tab_helper.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Hyrax + ## + # Decides whether the Aliases tab should appear on a work or + # collection edit form. Included into `WorkFormHelper` and + # `CollectionsHelper` so both forms share one rule. + # + # The tab appears only when the redirects feature is fully active + # (`Hyrax.config.redirects_active?`), the form is a ResourceForm + # (the only form pipeline with the redirects populator, validator, + # and prepopulator wired in), AND the form's underlying resource + # carries the `redirects` attribute. The structural check guards + # against adopter work or collection classes that don't include + # the redirects schema; without it the tab would render and crash + # on `f.object.redirects`. + module RedirectsTabHelper + def redirects_tab?(form) + return false unless Hyrax.config.redirects_active? + return false unless redirects_supported_form?(form) + target = form.respond_to?(:model) ? form.model : form + target.respond_to?(:redirects) + end + + def redirects_supported_form?(form) + form.is_a?(Hyrax::Forms::ResourceForm) + end + end +end diff --git a/app/helpers/hyrax/work_form_helper.rb b/app/helpers/hyrax/work_form_helper.rb index 4ac993465a..d6f4b1faac 100644 --- a/app/helpers/hyrax/work_form_helper.rb +++ b/app/helpers/hyrax/work_form_helper.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true module Hyrax module WorkFormHelper + include Hyrax::RedirectsTabHelper + ## # @todo this implementation hits database backends (solr) and is invoked # from views. refactor to avoid @@ -30,11 +32,13 @@ def admin_set_options # @param form [Hyrax::Forms::WorkForm, Hyrax::Forms::ResourceForm] # @return [Array] the list of names of tabs to be rendered in the form def form_tabs_for(form:) - if form.instance_of? Hyrax::Forms::BatchUploadForm - %w[files metadata relationships] - else - %w[metadata files relationships] - end + tabs = if form.instance_of? Hyrax::Forms::BatchUploadForm + %w[files metadata relationships] + else + %w[metadata files relationships] + end + tabs << 'redirects' if redirects_tab?(form) + tabs end ## diff --git a/app/views/hyrax/base/_form_redirects.html.erb b/app/views/hyrax/base/_form_redirects.html.erb new file mode 100644 index 0000000000..0faa9e57b3 --- /dev/null +++ b/app/views/hyrax/base/_form_redirects.html.erb @@ -0,0 +1,52 @@ +<%# Aliases tab — see documentation/redirects.md %> +<% return unless redirects_tab?(f.object) %> +

<%= t('hyrax.works.form.redirects.heading') %>

+ +

<%= sanitize t('hyrax.works.form.redirects.help_html'), tags: %w[code] %>

+ + + + + + + + + + + <%# Wrap each row through Hyrax::Redirect.wrap so the view can rely on + the presenter API (.path, .canonical, .sequence). On initial render + the prepopulator already wrapped them; on a re-render after a + failed save, f.object.redirects holds the populator's plain hashes, + and we wrap them here. %> + <% rows = Array(f.object.redirects).map { |e| Hyrax::Redirect.wrap(e) } + [Hyrax::Redirect.new(path: nil)] %> + <% rows.each_with_index do |entry, index| %> + <% input_id = "#{f.object_name}_redirects_attributes_#{index}_path" %> + <% label_text = entry.path.present? ? t('hyrax.works.form.redirects.path_label') : t('hyrax.works.form.redirects.new_path_label') %> + + + + + <% end %> + +
<%= t('hyrax.works.form.redirects.caption') %>
<%= t('hyrax.works.form.redirects.header.path') %><%= t('hyrax.works.form.redirects.header.actions') %>
+ <%= label_tag input_id, label_text, class: 'sr-only' %> +
+
+ <%= request.base_url %> +
+ <%= text_field_tag "#{f.object_name}[redirects_attributes][#{index}][path]", + entry.path, + id: input_id, + class: 'form-control', + placeholder: t('hyrax.works.form.redirects.placeholder.path') %> +
+ 		+ <% if entry.path.present? %> + + <% end %> +
diff --git a/app/views/hyrax/dashboard/collections/_form.html.erb b/app/views/hyrax/dashboard/collections/_form.html.erb index 5e8c87441d..a19ee698b0 100644 --- a/app/views/hyrax/dashboard/collections/_form.html.erb +++ b/app/views/hyrax/dashboard/collections/_form.html.erb @@ -28,6 +28,13 @@ <% end %> + <% if redirects_tab?(@form) %> + + <% end %> <% end %> @@ -109,6 +116,16 @@ <% end %> + + <% if redirects_tab?(f.object) %> +
+
+
+ <%= render 'hyrax/base/form_redirects', f: f %> +
+
+
+ <% end %> <% end %>
diff --git a/config/locales/hyrax.en.yml b/config/locales/hyrax.en.yml index 3b183fe5e4..f001dc51ff 100644 --- a/config/locales/hyrax.en.yml +++ b/config/locales/hyrax.en.yml @@ -925,6 +925,7 @@ en: branding: Branding description: Description discovery: Discovery + redirects: Aliases relationships: Relationships sharing: Sharing form_branding: @@ -1809,9 +1810,24 @@ en: in_collections: Collections in_other_works: This Work in Other Works in_this_work: Other Works in this Work + redirects: + caption: List of URL aliases for this resource + errors_heading: Please correct the following alias errors + header: + actions: Actions + path: Path + heading: URL Aliases + help_html: 'Register legacy URL paths that should redirect to this record. Each path will resolve from this site''s root. Reserved Hyrax routes (e.g. /concern, /dashboard) cannot be used.' + new_path_label: New alias path + path_label: Alias path + placeholder: + path: Enter a legacy path... + remove_label: Remove + remove_with_path_label: 'Remove %{path}' tab: files: Files metadata: Descriptions + redirects: Aliases relationships: Relationships share: Sharing visibility_until: until diff --git a/documentation/forms/field_behaviors.md b/documentation/forms/field_behaviors.md new file mode 100644 index 0000000000..956003155b --- /dev/null +++ b/documentation/forms/field_behaviors.md @@ -0,0 +1,271 @@ +# Field Behaviors + +A **Field Behavior** is a Ruby module mixed into `Hyrax::Forms::ResourceForm` (and its subclasses) that wires up a single property whose persisted shape and submitted-form shape differ. It owns the populator, prepopulator, and `deserialize!` strip for that property. + +Use a Field Behavior when you have a property that: + +- Is rendered through `accepts_nested_attributes_for` semantics (`_attributes` payload from the form). +- Persists in a shape the view can't render directly (a hash of sub-fields, a URI string the view wants as a `ControlledVocabulary` instance, a triple of values, etc.). +- Needs the same wiring on every form subclass. + +Two reference exemplars ship with Hyrax: + +- `Hyrax::BasedNearFieldBehavior` — single-string-per-entry (URI), hydrated to a `ControlledVocabularies::Location` for the view. +- `Hyrax::RedirectsFieldBehavior` — multi-field-per-entry (path / canonical / sequence), hydrated to a `Hyrax::Redirect` presenter for the view. + +This document covers the contract a Field Behavior must satisfy, the decision points for a new behavior, and the worked examples. + +## Why this pattern exists + +Reform's `FormBuilderMethods#deserialize!` rewrites the submitted `_attributes` key to `` before `from_hash` runs. If the form *also* has a property named `` (e.g. from an `include Hyrax::Schema(:foo)`), Reform's `from_hash` writes the raw fragment hash onto the property — bypassing the populator entirely. + +`deserialize!` is the right hook to fix this: by the time it runs, the rename has happened, but `from_hash` hasn't. A Field Behavior strips the renamed key from `params`, leaving the `_attributes` payload as the only entry point. The populator then owns the write. + +## The contract + +A Field Behavior is a module that: + +1. **Registers a virtual `_attributes` property** in `self.included`, with a populator and prepopulator. + + ```ruby + def self.included(descendant) + descendant.property :_attributes, + virtual: true, + populator: :_attributes_populator, + prepopulator: :_attributes_prepopulator + end + ``` + +2. **Composes via `super` in `deserialize!`**, then deletes its own renamed key. + + ```ruby + def deserialize!(params) + result = super + if result.respond_to?(:delete) + result.delete('') + result.delete(:) + end + result + end + ``` + + Calling `super` first lets every behavior in the chain run its own delete before control reaches Reform's base `deserialize!`. **Never** call `from_hash` yourself — that breaks composition for any behavior added later. + + Mutate `params` in place. Reform exposes the same hash via `form.input_params`; replacing it would orphan downstream readers. + +3. **Provides a populator** that reads the `_attributes` fragment, normalizes to the persisted shape, and assigns to the property. + + ```ruby + def _attributes_populator(fragment:, **_options) + return unless respond_to?(:) + # turn fragment into the persisted shape, then: + self. = entries + end + ``` + + Drop empty rows and rows marked `_destroy` here. Normalize values here too — the persisted shape should be canonical so non-form callers (importers, console, validators) don't have to re-normalize. + +4. **Provides a prepopulator** that hydrates the persisted value into a render-friendly object. + + ```ruby + def _attributes_prepopulator + return unless respond_to?(:) + self. = Array().map { |entry| MyPresenter.wrap(entry) } + end + ``` + +5. **Is `prepend`ed onto every subclass** in `ResourceForm.inherited`: + + ```ruby + class << self + def inherited(subclass) + subclass.prepend(MyFieldBehavior) + super + end + end + ``` + + `prepend` (not `include`) places the module's `deserialize!` *above* the subclass's own method on the ancestor chain, so it actually overrides. Each behavior gates itself internally and is a no-op when its property isn't on the subclass's model, so the unconditional prepend is safe. + +## Feature-gated behaviors + +If your behavior is tied to a feature flag (Flipflop, env config, etc.), gate **inside** the methods, not around the include / prepend: + +```ruby +def self.included(descendant) + return unless Hyrax.config.my_feature_enabled? + descendant.property ... +end + +def deserialize!(params) + result = super + if Hyrax.config.my_feature_active? && result.respond_to?(:delete) + result.delete('') + result.delete(:) + end + result +end + +def _attributes_populator(fragment:, **_options) + return unless respond_to?(:) + return unless Hyrax.config.my_feature_active? + ... +end +``` + +The `self.included` gate uses the structural config (does this Hyrax build know about the feature at all?). The runtime methods use the combined gate (config + Flipflop) so the behavior turns off when an admin flips the feature off without restarting. + +## Decision points + +When adding a Field Behavior, work through these: + +### 1. What does each entry look like? + +- **Single value per entry** (URI string, scalar): see `BasedNearFieldBehavior`. +- **Multiple sub-fields per entry** (path + canonical + sequence; or label + value + lang): see `RedirectsFieldBehavior`. + +### 2. Persisted as what? + +- **Plain string / scalar** — fine for single-value entries. +- **Plain hash** with string keys — for multi-field entries. Add the `hash` shortcut to your YAML schema (`type: hash, multiple: true`). Use this *instead* of nesting a Valkyrie::Resource subclass; nested resources round-trip badly through Postgres JSONB (sub-fields strip, parent fields leak). + +### 3. Does the view need a presenter? + +- **Yes** if the view calls `.something` accessors on each entry. Build a small Ruby class with `path` / `value` / etc. readers and a `wrap(input)` class method that accepts both raw and already-wrapped input. The prepopulator wraps every entry; the view can rely on the presenter API. +- **No** if the view reads keys directly. Skip the presenter and let the view call `entry['key']`. + +### 4. What happens to invalid input? + +The populator drops empty rows and `_destroy` rows. Format validation belongs in an `ActiveModel::EachValidator` invoked by the form's `validation` block — *not* in the populator. The populator's job is shape conversion; the validator's job is correctness. + +### 5. Is the property optional on some forms? + +Always guard with `respond_to?(:)` at the top of populator and prepopulator. Adopter forms whose models don't include the schema get a no-op rather than a crash. + +## Exemplar: `BasedNearFieldBehavior` + +```ruby +module Hyrax + module BasedNearFieldBehavior + def self.included(descendant) + descendant.property :based_near_attributes, + virtual: true, + populator: :based_near_attributes_populator, + prepopulator: :based_near_attributes_prepopulator + end + + def deserialize!(params) + result = super + if result.respond_to?(:delete) + result.delete('based_near') + result.delete(:based_near) + end + result + end + + private + + def based_near_attributes_populator(fragment:, **_options) + return unless respond_to?(:based_near) + adds, deletes = [], [] + fragment.each do |_, h| + uri = RDF::URI.parse(h["id"]).to_s + h["_destroy"] == "true" ? deletes << uri : adds << uri + end + self.based_near = ((model.based_near + adds) - deletes).uniq + end + + def based_near_attributes_prepopulator + return unless respond_to?(:based_near) + self.based_near = based_near&.map { |loc| Hyrax::ControlledVocabularies::Location.new(RDF::URI.parse(loc)) } + self.based_near ||= [] + self.based_near << Hyrax::ControlledVocabularies::Location.new if self.based_near.blank? + end + end +end +``` + +- **Persisted shape:** array of URI strings. +- **View-side shape:** array of `Hyrax::ControlledVocabularies::Location` instances. +- **Diff from a plain `_attributes` setup:** `deserialize!` strips `based_near` after the rename so `from_hash` doesn't overwrite the property with raw fragment hashes; the populator merges adds/removes onto the existing `model.based_near` so partial form submissions are non-destructive. + +## Exemplar: `RedirectsFieldBehavior` + +```ruby +module Hyrax + module RedirectsFieldBehavior + def self.included(descendant) + return unless Hyrax.config.redirects_enabled? + descendant.property :redirects_attributes, + virtual: true, + populator: :redirects_attributes_populator, + prepopulator: :redirects_attributes_prepopulator + end + + def deserialize!(params) + result = super + if Hyrax.config.redirects_active? && result.respond_to?(:delete) + result.delete('redirects') + result.delete(:redirects) + end + result + end + + private + + def redirects_attributes_populator(fragment:, **_options) + return unless respond_to?(:redirects) + return unless Hyrax.config.redirects_active? + entries = Array(fragment&.values) + .reject { |row| row['_destroy'].to_s == 'true' || row['path'].to_s.strip.empty? } + .each_with_index.map do |row, i| + { 'path' => Hyrax::RedirectPathNormalizer.call(row['path']), + 'canonical' => row['canonical'].to_s == 'true', + 'sequence' => row['sequence'].presence&.to_i || i } + end + self.redirects = entries + end + + def redirects_attributes_prepopulator + return unless respond_to?(:redirects) + return unless Hyrax.config.redirects_active? + self.redirects = Array(redirects).map { |entry| Hyrax::Redirect.wrap(entry) } + end + end +end +``` + +- **Persisted shape:** array of plain hashes (`'path'`, `'canonical'`, `'sequence'`). Declared with `type: hash, multiple: true` in the YAML schema. +- **View-side shape:** array of `Hyrax::Redirect` presenters, exposing `.path` / `.canonical` / `.sequence`. +- **Diff from BasedNear:** entries carry multiple sub-fields, so the persisted shape is a hash rather than a string. The populator normalizes paths up front (canonical form lives in storage). The behavior is feature-gated — every callback consults `Hyrax.config.redirects_active?`. + +## Wiring on `ResourceForm` + +```ruby +module Hyrax + module Forms + class ResourceForm < Reform::Form + include BasedNearFieldBehavior + include RedirectsFieldBehavior + + class << self + def inherited(subclass) + subclass.prepend(BasedNearFieldBehavior) + subclass.prepend(RedirectsFieldBehavior) + super + end + end + end + end +end +``` + +Both behaviors compose. A subclass's ancestor chain ends up with both behaviors' `deserialize!` methods above Reform's base method; each runs its `super` then strips its own renamed key. + +## Common pitfalls + +- **Calling `from_hash` from inside `deserialize!`** — terminal. Breaks composition for any other behavior on the same form. Always call `super` and let Reform's base method do the rename + `from_hash`. +- **`include`-ing the behavior on subclasses** — the `included` callback runs but the `deserialize!` override doesn't take effect. Use `prepend` on subclasses so the override lands above the inherited method. +- **Conditional `prepend`** (`subclass.prepend(MyBehavior) if condition`) — the condition is evaluated at class-load time and never re-evaluated. Use unconditional `prepend` and gate inside the runtime methods. +- **Forgetting `respond_to?` guards** — adopter forms whose models don't include the schema raise `NoMethodError` instead of being a clean no-op. +- **Normalizing on read** — the persisted shape should be canonical so every consumer (validator, indexer, sync, importer) can compare without re-normalizing. Normalize in the populator (the form's write site) and add normalization to non-form write paths separately. diff --git a/documentation/redirects.md b/documentation/redirects.md index 3638361165..dd93144892 100644 --- a/documentation/redirects.md +++ b/documentation/redirects.md @@ -278,7 +278,9 @@ Alternatively, gate the inclusion of the calling code itself on `Hyrax.config.re ## See also - `documentation/flexible_metadata.md` — m3 profile fundamentals and how the redirects feature interacts with flexible metadata. -- `Hyrax::Redirect` (`app/models/hyrax/redirect.rb`) — the Valkyrie::Resource representing a single redirect entry. +- `documentation/forms/field_behaviors.md` — the Field Behavior pattern used by `Hyrax::RedirectsFieldBehavior` to wire the form's nested-attribute property. +- `Hyrax::Redirect` (`app/models/hyrax/redirect.rb`) — thin Ruby presenter for a single redirect entry; used on the form's render path. +- `Hyrax::RedirectsFieldBehavior` (`app/forms/concerns/hyrax/redirects_field_behavior.rb`) — form-side populator/prepopulator and the `deserialize!` strip for the `redirects` property. - `Hyrax::Indexers::RedirectsIndexer` (`app/indexers/hyrax/indexers/redirects_indexer.rb`) — the indexer mixin. - `Hyrax::RedirectsController` (`app/controllers/hyrax/redirects_controller.rb`) — the redirect resolver. - `Hyrax::FlexibleSchemaValidators::RedirectsValidator` (`app/services/hyrax/flexible_schema_validators/redirects_validator.rb`) — the m3 profile validator. diff --git a/spec/forms/concerns/hyrax/redirects_field_behavior_spec.rb b/spec/forms/concerns/hyrax/redirects_field_behavior_spec.rb new file mode 100644 index 0000000000..0bb0020c05 --- /dev/null +++ b/spec/forms/concerns/hyrax/redirects_field_behavior_spec.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +RSpec.describe Hyrax::RedirectsFieldBehavior do + let(:form_class) do + Class.new do + attr_accessor :redirects + + def self.property(*); end + + def from_hash(params) + params + end + + def deserialize!(params) + from_hash(params) + end + + include Hyrax::RedirectsFieldBehavior + prepend Hyrax::RedirectsFieldBehavior + end + end + + let(:form) { form_class.new } + + before do + allow(Hyrax.config).to receive(:redirects_enabled?).and_return(true) + allow(Hyrax.config).to receive(:redirects_active?).and_return(true) + end + + describe '.included' do + let(:property_target) do + Class.new do + def self.property(*); end + end + end + + it 'is a no-op when the config gate is closed' do + allow(Hyrax.config).to receive(:redirects_enabled?).and_return(false) + expect(property_target).not_to receive(:property) + property_target.include(described_class) + end + + it 'registers the redirects_attributes virtual property when enabled' do + expect(property_target).to receive(:property).with( + :redirects_attributes, + virtual: true, + populator: :redirects_attributes_populator, + prepopulator: :redirects_attributes_prepopulator + ) + property_target.include(described_class) + end + end + + describe '#deserialize!' do + it 'strips the renamed redirects key so the populator owns the write' do + result = form.deserialize!('redirects' => 'overwrite-me', :redirects => 'also', 'title' => 'kept') + expect(result).to eq('title' => 'kept') + end + + it 'is a no-op on the renamed key when the feature is inactive' do + allow(Hyrax.config).to receive(:redirects_active?).and_return(false) + result = form.deserialize!('redirects' => 'kept', 'title' => 'kept') + expect(result).to eq('redirects' => 'kept', 'title' => 'kept') + end + end + + describe '#redirects_attributes_populator' do + it 'normalizes paths and produces the persisted hash shape' do + fragment = { + '0' => { 'path' => ' /foo/ ', 'canonical' => 'true' }, + '1' => { 'path' => '/bar', 'canonical' => 'false', 'sequence' => '5' } + } + form.send(:redirects_attributes_populator, fragment: fragment) + + expect(form.redirects).to eq([ + { 'path' => '/foo', 'canonical' => true, 'sequence' => 0 }, + { 'path' => '/bar', 'canonical' => false, 'sequence' => 5 } + ]) + end + + it 'drops rows marked for destruction' do + fragment = { + '0' => { 'path' => '/keep', 'canonical' => 'false' }, + '1' => { 'path' => '/drop', 'canonical' => 'false', '_destroy' => 'true' } + } + form.send(:redirects_attributes_populator, fragment: fragment) + + expect(form.redirects.map { |e| e['path'] }).to eq(['/keep']) + end + + it 'drops rows with a blank path' do + fragment = { + '0' => { 'path' => '/keep', 'canonical' => 'false' }, + '1' => { 'path' => ' ', 'canonical' => 'false' } + } + form.send(:redirects_attributes_populator, fragment: fragment) + + expect(form.redirects.map { |e| e['path'] }).to eq(['/keep']) + end + + it 'is a no-op when the feature is inactive' do + allow(Hyrax.config).to receive(:redirects_active?).and_return(false) + form.redirects = 'untouched' + form.send(:redirects_attributes_populator, fragment: { '0' => { 'path' => '/foo' } }) + + expect(form.redirects).to eq('untouched') + end + end + + describe '#redirects_attributes_prepopulator' do + it 'wraps each persisted hash entry in a Hyrax::Redirect presenter' do + form.redirects = [{ 'path' => '/foo', 'canonical' => true, 'sequence' => 0 }] + form.send(:redirects_attributes_prepopulator) + + expect(form.redirects.first).to be_a(Hyrax::Redirect) + expect(form.redirects.first.path).to eq('/foo') + expect(form.redirects.first.canonical).to be(true) + end + + it 'is a no-op when the feature is inactive' do + allow(Hyrax.config).to receive(:redirects_active?).and_return(false) + original = [{ 'path' => '/foo' }] + form.redirects = original + form.send(:redirects_attributes_prepopulator) + + expect(form.redirects).to be(original) + end + end +end diff --git a/spec/helpers/hyrax/redirects_tab_helper_spec.rb b/spec/helpers/hyrax/redirects_tab_helper_spec.rb new file mode 100644 index 0000000000..0c1ecae6f1 --- /dev/null +++ b/spec/helpers/hyrax/redirects_tab_helper_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +RSpec.describe Hyrax::RedirectsTabHelper, type: :helper do + let(:helper_class) do + Class.new do + include Hyrax::RedirectsTabHelper + end + end + subject(:helper) { helper_class.new } + + let(:resource_with_redirects) { Struct.new(:redirects).new([]) } + let(:resource_without_redirects) { Object.new } + + describe '#redirects_tab?' do + before do + allow(Hyrax.config).to receive(:redirects_active?).and_return(true) + # The form-class gate is exercised by its own context below; stub it true + # for the other contexts so they isolate the redirects_active? and + # respond_to?(:redirects) checks. + allow(helper).to receive(:redirects_supported_form?).and_return(true) + end + + context 'when the redirects feature is inactive' do + before { allow(Hyrax.config).to receive(:redirects_active?).and_return(false) } + + it 'returns false' do + expect(helper.redirects_tab?(resource_with_redirects)).to be(false) + end + end + + context 'when the form is not a ResourceForm descendant' do + before { allow(helper).to receive(:redirects_supported_form?).and_return(false) } + + it 'returns false even when the resource carries redirects' do + expect(helper.redirects_tab?(resource_with_redirects)).to be(false) + end + end + + context 'when the resource carries the redirects attribute' do + it 'returns true' do + expect(helper.redirects_tab?(resource_with_redirects)).to be(true) + end + end + + context 'when the resource does not carry the redirects attribute' do + it 'returns false' do + expect(helper.redirects_tab?(resource_without_redirects)).to be(false) + end + end + + context 'when given a form-like wrapper that exposes #model' do + let(:form) do + Struct.new(:model).new(resource_with_redirects) + end + + it 'unwraps the form and inspects the underlying model' do + expect(helper.redirects_tab?(form)).to be(true) + end + + context 'when the wrapped model lacks the redirects attribute' do + let(:form) { Struct.new(:model).new(resource_without_redirects) } + + it 'returns false' do + expect(helper.redirects_tab?(form)).to be(false) + end + end + end + end + + describe '#redirects_supported_form?' do + it 'returns true for a Hyrax::Forms::ResourceForm instance' do + form = Hyrax::Forms::ResourceForm.new(Hyrax::Resource.new) + expect(helper.redirects_supported_form?(form)).to be(true) + end + + it 'returns false for an arbitrary object' do + expect(helper.redirects_supported_form?(Object.new)).to be(false) + end + end +end