Change the behavior of Repository create/persist

proposals
1

I did not realize that there is also a discuss site for lots, sorry.
I created this as an issue in https://github.com/lotus/model/issues/215 first.

As already mentioned in #206 (https://github.com/lotus/model/issues/206) i think it is strange to not assign the ID to the passed in entity. I created a PR to fix the documentation so it is aligned but i’d be interested to discuss this. I’d confess that i’m very much used to having the ID assigned because of ActiveRecord. But that does not necessarily make this a bad thing:-)

Here are some questions/comments:

Associations

What is planned when associations are added to lotus model? Will they be cloned as well? Just re-assigned to the cloned entity?

Inconsistent return values

create currently returns an entity or nil depending on success or failure of the call.
persist always returns an entity.

I’d prefer those calls to actually return success/failure state (or nothing at all).

Assignment

Currently one needs to assign the return value. It needs to be assigned to a different variable in order to keep track of the entity in case the call did not succeed:

another_article = ArticleRepository.create(article)
if another_article
puts "Yay, i persisted article for you"
else
puts "Something is wrong with #{article}"
end
This looks rather ugly.

Even worse the persist case where you probably need to compare id_s before and after _persist to see if:

newly created
done nothing at all
error (not sure…could persist return an entity with id unassigned?)
Exceptions

I was wondering if it’d be a good idea to raise an exception if an already persisted entity is passed to create. I’d prefer this, actually. Strict API.

Wishlist

Make create assign the id to the entity
Make persist assign the id to the entity
Make create throw an exception if entity is already persisted
Make create/persist return either a truthy value to indicate success/failure or nothing at all (and raise exceptions)

Sidenote

Currently at least one Repository test is broken (i assume that this is because you changed the way create/persist works…but nevertheless)

Since user1 is only passed in to create in the before block and not assigned, it is actually unpersisted:

    before do
      @users = [
        UserRepository.create(user1),
        UserRepository.create(user2)
      ]
    end

I added some comments/lines to demonstrate:

    it 'does nothing when already persisted' do
      id = user1.id
      assert user1.id # added this line...breaks test
      UserRepository.create(user1) # this is also wrong, would need to re-assign as parameter is not modifed
      user1.id.must_equal id
    end