Do not "touch" models when not updating the base table content.

[jmfederico]

I use this GEM next to PAPER TRAIL, and found too may versions being stored by PAPER TRAIL with no changes between them. I tracked it down to this gem saving a widget after destroying a draft when the record was changed back to the original values.

To me it makes sense not to save the widget, as no changes are being saved to it.

Makes sense?

[chrisdpeters]

@jmfederico I don't believe your solution works because it does not save the nilified draft_id to the database. So then the records will still "think" they're drafts, even if they're not.

One thing to try would be to use a persistence method that doesn't run callbacks when nillifying the draft_id column. (Perhaps update_column?) The goal would be to implement something that doesn't cause failing tests.

[jmfederico]

At this point there are only 2 tests failing.

How can I detect that there are skipped fields that need saving on a model?

[jmfederico]

After studying the code of Draftsman I decided that PAPERTRAIL saving or not saving versions has nothing to do with how this gem works. Instead I have to implement my own "versioning" logic.

But, in the process, I noticed that the "updated_at" column of the original model was always being changed, even when the model itself wasn't changing.

I then decided to change this so that it only is changed when the model itself changes.

The idea is:

• When not published, always update updated_at
• When published, do not updated updated_at unless
  • There are skipped columns
  • changes are not being stashed
  • draft is published

I wrote the necessary tests, BUT, for some reason there is one test failing on Travis, which is not failing on my local machine, and, from what I understand, it shouldn't be failing. There is an issue for it on Rails.

Until I know which environment is misbehaving (my laptop or Travis) I am not making any changes to that specific test.

[jmfederico]

And: thanks!
Awesome Gem!

[chrisdpeters]

Thank you. I will take a look here in a week or two to see if I can see something that you're not seeing.

[jmfederico]

Honestly, this is driving me crazy:

[chrisdpeters]

@jmfederico This is next on my list. I'm getting a Docker container set up so I can try running the specs on Linux.

[jmfederico]

I have to say, Ruby and Rails are not my strong suit, but this makes no sense to me.
Shouldn't this two snippets be equivalent?

time = skipper.updated_at
expect(subject.updated_at).to eq time

expect(subject.updated_at).to eq skipper.updated_at

If they should behave differently, I would appreciate if you let me know why. (I feel a bit ignorant right now).

Anyway, tests pass.

I hope you find this useful.

[chrisdpeters]

@jmfederico I believe that those two snippets should behave differently.

Let me explain what's going on in the first example with comments:

# Caches `skipper.updated_at` into a variable named `time` before query is
# run.
time = skipper.updated_at
# Runs query, reloads `skipper` record, and then compares its `updated_at`
# value to the cached `time` value.
expect(subject.updated_at).to eq time

And the second example:

# Runs query, reloads `skipper` record, then compares it with itself.
expect(subject.updated_at).to eq skipper.updated_at

To me, this is evidence that perhaps the functionality is wrong, even if the test is passing. What do you think?

[jmfederico]

First, thanks for the explanation, it helped a lot.

Second, I think I found the problem. It has to do with microseconds precision. I will try to explain it to best of my abilities.

When creating a new Object that has not been persisted, the updated_at property has 7 decimals for microseconds, but as soon as it gets reloaded, the precision changes to 6 decimals. But this only happens in Linux, not on MacOS, thats why it wasn't failing for me.

Example, given this code (Line 155 of spec/models/skipper_spec.rb):

      context 'with existing `create` draft' do
        before do
          skipper.save_draft
          print skipper.updated_at.to_f
          print "\n"
          skipper.reload
          print skipper.updated_at.to_f
          print "\n"
        end

Output for Docker:

1496540796.3237731
1496540796.323773

Output for MacOS:

1496540795.650563
1496540795.650563

The test was ok before, but skipper needs to be reloaded as soon as save_draft is called.
This was the only test where reload was not being called.

[chrisdpeters]

@jmfederico I think I am going to name the next release after you. Many thanks for your persistence on this.

[jmfederico]

@chrisdpeters I am glad you find this useful.

Thank you for a great gem!