Skip to content

pull request for adding reason for update/comment to each saved version#5

Open
elzoiddy wants to merge 23 commits into
adamcooper:masterfrom
elzoiddy:master
Open

pull request for adding reason for update/comment to each saved version#5
elzoiddy wants to merge 23 commits into
adamcooper:masterfrom
elzoiddy:master

Conversation

@elzoiddy

@elzoiddy elzoiddy commented Nov 9, 2010

Copy link
Copy Markdown

Hi Adam,

I've made a change to add a new column in the versions record called "reason_for_update". It can be used to document the reasons for updating a versioned record or simply make a comment about the change. For many applications (particularly financial) it might be nice to also document the reason as well as what changed for a record. I've added a new test case and modified the migration templates. Everything seems to work as designed. All tests passes with ruby 1.9.2 p0 as well as ruby-1.8.7-p302

Let me know what you think.

Thanks

Example usage:

user = User.create(:first_name => "mary", :last_name => "Jane")

user.update_attributes(:last_name => "Jane-Smith", :updated_by => "csr #1",  
     :reason_for_update => "Name change due to marriage")

user.update_attributes(:last_name =>"Jane", :updated_by => "csr #2",  
     :reason_for_update => "name change due to separation")

@adamcooper

Copy link
Copy Markdown
Owner

Thanks for the pull request, I just looked at the changes and like them.

However, I'm not sure how many people will want/require this reason_for_update. I think part of the appeal of this plugin is that it is light-weight in nature - it only stores the changes. Adding a new reason column may make this seem a little more overkill.

What would be ideal in my mind is if we could make this module optional. This would also align with my long-term goal that pieces like reasons, reverts, etc. won't be mandatory but can be opt-in.

I don't think it would require many changes right now to adapt this patch to that type of mentality. It should only require:

  • Add a config option
  • Check the config option in the 'include Comments' statement in the main versioned method
  • Change the migration schema to comment it out and include some help text on how to get it included
  • Documentation to tell users about this new feature.

It will probably take a me a week or so to get around to this.

Thanks,
Adam

elzoiddy and others added 21 commits December 29, 2010 16:01
 "number" is a reserved word in oracle, so we need alternate column name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants