Default version field name should be "_version".

[gdlx]

Hi !

As version field is a meta-field, I think this field's name should defaults to _version, the same way Mongoid adds its own meta-fields (_id, _type).

I know this field name can be overridden, but, I think, default values should respect best conventions.

Regards,
Gauthier.

[msaffitz]

+1 version's a relatively common existing attribute name for our models

[thetron]

+1 Agreed, this would be a great default.

[johnnyshields]

@aq1018 @dblock any objection to this?

[dblock]

As long as it's easy to redefine it for existing projects.

[johnnyshields]

So, just to clarify this, there's an existing config option called version_field. This affects only the underlying database field (in my case i have it set to _v so it's minimized). This config should not be affected by this change.

How about we make a new config option version_alias. Then on trackable.rb on line 32, change:

   field options[:version_field].to_sym, :type => Integer

to

   field options[:version_field].to_sym, :as => options[:version_alias].to_sym, :type => Integer

This uses the :as (alias) option native to Mongoid. I think there may be some existing code which sets up an alias between the method #version and version_field field, this can be removed.

To get the existing behavior, you can set version_alias to "version" and you're golden. As it's a pain to do this on every track_history call, it would be nice if we could set global config defaults for :version_field and :version_alias.

If all are agreed on this, can someone else pick up the PR? I'm a bit tied up right now.

[gdlx]

@johnnyshields Your option seems complicated knowing that it's just about changing the default value (backward incompatible change) because the former one is too common.

@dblock It is already possible to redefine it (I had to do it because I'm using another version field). The change only concerns the default value. People (including myself since I only redefined to _version where I have a collision) who upgrade to the new version will have to redefine every mongoid-history model where version field wasn't redefined.

[johnnyshields]

@porecreat if we're going the backwards incompatible route, we still should do:

   field options[:version_field].to_sym, :as => :_version, :type => Integer

i.e. take advantage of Mongoid's built-in field name alias feature, and remove code from the gem itself.

[dblock]

I think we should change the default and document how to implement the old behavior in CHANGELOG. <--- I would take this PR.

[gdlx]

@johnnyshields I'm not sure why you want to use an alias here. This way we can change the field name in mongodb but both names will be available in the model and _version method will be statically reserved. I know _version is not likely to be used for something else, but the goal of customization is to allow using the gem in ANY application, even those that could use _version for something else (simple example : maybe some dudes using mongoid-history have renamed their own version field to _version instead of customizing the gem one...I could have done that if I had added my version field after having created records with mongoid-history'sversionfield).

I don't see the interest of coding something dynamic in the gem, and put something static beside it...why not being fully dynamic and customizable ? What will this _version alias be used for anyway ?

[johnnyshields]

@porecreat currently we have the version field as part of the tracked doc:

# current state
class MyTrackedDoc
  include Mongoid::History::Trackable

  track_history on: [ :foo, :bar ],
                version_field: :_v
end

I'd propose to move the version option to a global setting in /config/initializers/mongoid_history.rb and remove it from each tracked model:

# proposed state - initializer
Mongoid::History.version_field_name = :_v
Mongoid::History.version_field_alias = :_version

# proposed state - doc
class MyTrackedDoc
  include Mongoid::History::Trackable

  track_history on: [ :foo, :bar ]  # no more :version_field option
end

Then the Mongoid-History gem would automatically set the field on the Tracker class:

# automatically set in Tracker class
field Mongoid::History.version_field_name, :as => Mongoid::History.version_field_alias, :type => Integer

I think this is the cleanest/simplest implementation, as long as we don't need use different aliases for the version field on each tracked model.

[gdlx]

@johnnyshields This sounds fine like this.

The alias isn't a problem anymore if it's dynamic too but I think it should be opt-in (no alias by default), and default field name should be _version, not _v, minimzing field names being a developper choice, and subject to many collisions: clarity should have priority in default values.

I think it's necessary to keep the ability to set model specific field name (and alias), at least to allow migrations from past versions (I concerned : only one of my models use _version as history version field, the one with another version field).

[johnnyshields]

@porecreat its awkward to have a different version alias for each trackable model--in the end it's just extra code/complexity inside this gem. Do you really, really need this?

[gdlx]

@johnnyshields I don't need the alias feature, so I don't need it to be model specific ;o) As it doesn't exists yet, it won't be a regression. (But you'll always find some dude to ask for it...)

[johnnyshields]

Dudes are always asking for everything 😎

Are you ok to do the PR for this?

[johnnyshields]

@porecreat any progress on this?