valkyrie icon indicating copy to clipboard operation
valkyrie copied to clipboard

Have ChangeSet save throw a meaningful exception

Open elrayle opened this issue 6 years ago • 3 comments

Description

ChangeSets inherit a #save method from Reform/Disposable which call #sync on the change set and #save on the resource. Since Valkyrie resources do not have a #save method, this raise undefined method exception. It would be better for this to raise a more meaningful exception that provides information on how to avoid the exception.

Rational

Provide developers with information on how to avoid exceptions.

Expected behavior

Calling change set #save method should throw...

NoMethodError (undefined method `save' for #<_ChangeSet_>)

OR something more descriptive...

NoMethodError (undefined method `save' for #<_ChangeSet_>); call #sync method on change set and then use persister to save resource

Actual behavior

Calling change set #save method throws...

NoMethodError (undefined method `save' for #<_Resource_>)

To reproduce

# frozen_string_literal: true
class Book < Valkyrie::Resource
  attribute :title, Valkyrie::Types::Set.of(Valkyrie::Types::String).meta(ordered: true)
end

# frozen_string_literal: true
class BookChangeSet < Valkyrie::ChangeSet
  property :title
end

book = Book.new
book_cs = BookChangeSet.new(book)
book.title = 'test title'
book_cs.save # NoMethodError (undefined method `save' for #<Book:0x00007fafcea8c278>)

elrayle avatar Dec 06 '19 16:12 elrayle

:+1: to the more descriptive option.

tpendragon avatar Dec 06 '19 17:12 tpendragon

is this due to the inclusion of Reform::Form::ActiveModel? are there other methods included from Reform that will dead-end this way?

no-reply avatar Dec 19 '19 00:12 no-reply

I don't think so. The interface for Reform is pretty small.

tpendragon avatar Dec 29 '19 06:12 tpendragon