When saving forms with collections, item forms save/sync methods are not called.

[lastobelus]

Complete Description of Issue

When you save (or sync) a form that has a collection, and that collection specifies a form object ItemForm, and ItemForm overrides save or sync methods, those methods are never called.

Do you think that for consistency/control that when saving/sync'ing the tree that all the Form wrappers' save/sync methods should be called, not just that of the root object?

I mentioned on gitter that I thought this was only with non-virtual collections, but it seems I misrembered and it is with any collection.

Steps to reproduce

require 'rails_helper'

module ChildSaveSyncSpec
  context 'saving/syncing collections with item forms' do
    class Song
      aattr_initialize [:title]

      def save(*)
        puts "Song.save"
        true
      end
    end

    class Album
      aattr_initialize [:title, :songs] do
        @songs = []
      end

      def save(*)
        puts "Album.save"
        true
      end
    end

    class SongForm < Reform::Form
      property :title
      cattr_accessor :save_count, :sync_count


      def sync(*)
        SongForm.sync_count = SongForm.sync_count + 1
        puts "SongForm.sync"
        super
      end

      def save(*)
        SongForm.save_count = SongForm.save_count + 1
        puts "SongForm.save"
        super
      end
    end
    SongForm.save_count = 0
    SongForm.sync_count = 0

    class VirtualSongsAlbumForm < Reform::Form
      property :title

      collection :songs, virtual: true, form: SongForm, populate_if_empty: Song

      def sync(*)
        puts "VirtualSongsAlbumForm.sync"
        super
      end

      def save(*)
        puts "VirtualSongsAlbumForm.save"
        super
      end
    end

    class AlbumForm < Reform::Form
      property :title

      collection :songs, form: SongForm, populate_if_empty: Song

      def sync(*)
        puts "AlbumForm.sync"
        super
      end

      def save(*)
        puts "AlbumForm.save"
        super
      end
    end

    context 'virtual collection' do
      it "should run save/sync on SongForms" do
        form = VirtualSongsAlbumForm.new(Album.new(title: nil, songs: []))
        form.songs = []

        form.validate(
          "title" => "album with virtual songs",
          "songs_attributes" => {
            "0" => {"title" => "bob"},
            "1" => {"title" => "song-top"},
          }
        )

        expect { form.save }.to change { SongForm.sync_count }.by(2)
          .and(change {SongForm.save_count}.by(2))
        expect { form.sync }.to change { SongForm.sync_count }.by(2)

      end
    end

    context 'non virtual collection' do
      it "should run save/sync on SongForms" do
        form = AlbumForm.new(Album.new(title: nil, songs: []))

        form.validate(
          "title" => "album with non virtual songs",
          "songs_attributes" => {
            "0" => {"title" => "bob"},
            "1" => {"title" => "song-top"},
          }
        )

        expect { form.save }.to change { SongForm.sync_count }.by(2)
          .and(change {SongForm.save_count}.by(2))

        expect { form.sync }.to change { SongForm.sync_count }.by(2)

      end
    end
  end
end

Expected behavior

I'd expect the specs to pass and print the following:

VirtualSongsAlbumForm.save
VirtualSongsAlbumForm.sync
Album.save
SongForm.save
SongForm.sync
Song.save
SongForm.save
SongForm.sync
Song.save

AlbumForm.save
AlbumForm.sync
Album.save
SongForm.save
SongForm.sync
Song.save
SongForm.save
SongForm.sync
Song.save

Actual behavior

specs fail, and following is printed:

VirtualSongsAlbumForm.save
VirtualSongsAlbumForm.sync
Album.save
Song.save
Song.save

AlbumForm.save
AlbumForm.sync
Album.save
Song.save
Song.save

System configuration

Reform version: 2.3.0.rc1

Full Backtrace of Exception (if any)

[lastobelus]

oh, spec as written requires attr_extras, and cattr_accessor from ActiveSupport

[lastobelus]

I'm kinda desperate for a workaround for this short of ignoring Reform#save and rewriting save logic for the whole object graph myself. Is there perhaps a way to customize the Twin objects used behind the scenes to only save when some property is true?