Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Delegate more methods to plucky [Fixes #379, #266] #387

merged 2 commits into from Mar 30, 2012


None yet
3 participants

brianhempel commented Feb 11, 2012

Delegate more methods to plukcy

:distinct, :size, :reverse, :offset, :order, :empty?,
:filter, :find_one, :per_page, :ignore, :only, :to_a

A few user problems are fixed by this, e.g. people wanting to write MyModel.order(:key).

In the has_many association, to_a, size, and empty? need to hit the loaded association array instead of going to the database. Otherwise, documents created with build don't show up. I updated the tests to reflect this.

I opted not to delegate remove since that's covered by .delete_all and I guess I'm afraid of it. If you think it should be there, let me know...it's easy enough to add.

brianhempel added some commits Feb 11, 2012

@brianhempel brianhempel Delegate :distinct, :size, :reverse, :offset, :order, :empty?, :filte…
…r, :find_one, :per_page, :ignore, :only to the query.

A few user problems are fixed by this. To honor unpersisted documents, the methods without arguments (:size and :empty?) need to hit the loaded association array in the ManyDocumentsProxy instead of going to the database...tests updated to reflect this.
@brianhempel brianhempel Also delegate :to_a to plucky 9633972

Plucky now has a Methods constant for what needs to be delegated. We should probably use that in MM.



bkeepers commented Mar 30, 2012

I tried switching this to use Plucky::Methods, but it blows up in a couple spots…

  • :find_one, :find, and :find! aren't in Plucky::Methods
  • MM's PluckyMethods is used in ManyDocumentsProxy, so delegating methods like :empty? and :size breaks a bunch of the tests

I'm just going to pull this as is since it makes MM better. If someone wants to refactor it to use Plucky::Methods later, they can.

@bkeepers bkeepers added a commit that referenced this pull request Mar 30, 2012

@bkeepers bkeepers Merge pull request #387 from brianhempel/delegate_more_to_plucky
Delegate more methods to plucky [Fixes #379, #266]

@bkeepers bkeepers merged commit 2d807c0 into mongomapper:master Mar 30, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment