Allows the definition of a customized whodunnit in a small scope #334

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
3 participants

lucasas commented Feb 22, 2014

It's very useful when you want to define who is responsible to change some object and don't considering the global scope or request scope.

@seanlinsley seanlinsley commented on the diff Feb 23, 2014

lib/paper_trail/has_paper_trail.rb
@@ -232,6 +232,11 @@ def touch_with_version(name = nil)
save!
end
+ def whodunnit(whodunnit)
+ @whodunnit = whodunnit
+ yield if block_given?
+ end
@seanlinsley

seanlinsley Feb 23, 2014

Contributor

It seems like this would make more sense instead of having whodunnit and _whodunnit

def whodunnit(whodunnit = PaperTrail.whodunnit)
  @whodunnit = whodunnit
  yield if block_given?
end
@lucasas

lucasas Feb 23, 2014

Hi @seanlinsley, what about the case where I don't want to use my_object.whodunnit?

batter added the Not a bug label Mar 3, 2014

batter self-assigned this Mar 4, 2014

batter added this to the 3.0.1 milestone Mar 4, 2014

lucasas commented Mar 10, 2014

Did you have plans to merge this?

Collaborator

batter commented Mar 11, 2014

@lucasas - Yes. After reviewing the PR a little more closely just now I'm thinking it probably makes more sense to drop the instance variable store and only allow this method to be called with a block argument to prevent confusion.

As in:

PaperTrail.whodunnit = 'Andy Stewart'

widget = Widget.create!
widget.versions.last.whodunnit # 'Andy Stewart'

# This will work
widget.whodunnit 'Lucas Souza' do
  widget.update_attributes :name => 'Wibble'
end

widget.versions.last.whodunnit # 'Lucas Souza'

# This won't
Widget.whodunnit = 'Lucas Souza' # Throws Error

I just think trying to store the value as an instance variable could create some confusion both with usage and with code readability in terms of adding an additional level of complexity. Does that make sense?

batter closed this in 44a632e Mar 11, 2014

lucasas commented Mar 12, 2014

Hi @batter, thanks for accept my PR. I think make sense your changes.

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