ActiveRecord awesome #joins: now shows the #select'ed columns

[adrianomitre]

Display ActiveRecord#joins resulting objects as one would expect, i.e., with all the selected columns.

Example

Before

After

[gerrywastaken]

Rebased against master on my own fork to test the build status: https://travis-ci.org/gerrywastaken/awesome_print/builds/129061596

I'm still trying to figure out why refreshing the build is not making this build pass on this fork.

[adrianomitre]

Thank you.

2016-05-10 5:01 GMT-03:00 Gerry notifications@github.com:

Rebased against master on my own fork to test the build status:
https://travis-ci.org/gerrywastaken/awesome_print/builds/129061596

I'm still trying to figure out why refreshing the build is not making this
build pass on this fork.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/michaeldv/awesome_print/pull/211#issuecomment-218085851

[gerrywastaken]

Firstly, I'm very sorry that you have had to wait so long. This looks like a good fix, but I have a few questions.

1. What was your reason for adding the readonly? check? I ask, because that is coming back as false for me on Rails 5 and so your fix is never reached.
2. Do you think we could compare against object.class.column_names instead of object.class.new.attributes.keys, so that an extra object instantiation isn't required; or is there a problem with that approach?

Some things that either you or I will need to fix before merging:

1. It seems the second commit is modifying the first commit, so I think it would be less confusing for others to read if it were a single commit.
2. Rebasing on master.
3. Adding a test for the new logic branch.

[gerrywastaken]

@adrianomitre sorry, I forgot to tag you in my last comment.

[adrianomitre]

Hi, Gerry!
I am traveling right now, with limited computer access, so I'd kindly ask
you to wait until next week, ok?
Cheers,
Adriano Mitre
(enviado do celular)
Em 1 de jul de 2016 4:18 AM, "Gerry" notifications@github.com escreveu:

@adrianomitre https://github.com/adrianomitre sorry, I forgot to tag
you in my last comment.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#211 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAb4ZLW-tMXWoezVgOYb1iOM0IRUv4upks5qRHjwgaJpZM4GstXh
.

[gerrywastaken]

@adamjonas No problem at all, happy to wait. Your response time has been far better than ours. :)

I hope you are enjoying your travels.

[adrianomitre]

@gerrywastaken Thank you for your feedback. I followed your suggestions and it now works (probably a tad faster) on Rails 5.0.0 and 4.2.6, so I believe it can now be merged.

[gerrywastaken]

#211 (comment)

I still think this would read better as one commit, as the first three are essentially just drafts. As mentioned above, I would also need a test before merging.

[adrianomitre]

@gerrywastaken In the process of creating a test, I found some weird behaviours. The first is that I could not make the object.class.column_names != object.attributes.keys be true (well, at the very least not using an example such as the given in the beginning of the pull request). So the newly introduced test passes both with and without the new code. The second, doing manual tests, is that it seems to work only with pry (adding pry-rails to Gemfile), but not with IRB. Would you have any clue why this is so?

[gerrywastaken]

I'm not sure about the problem you are running into. I'm pretty sleepy right now, but I can't see where your test uses any code in awesome print, so it seems more like a test of active record.

How about we use your actual expected output as the test case?

describe 'Linked records' do
  before do
    @ap = AwesomePrint::Inspector.new(:plain => true)
  end

  it 'should show the entire record' do
    user = User.create
    user.emails.create(email_address: 'foo@bars.com')
    email_record = User.joins(:emails).select('emails.email_address')[0]

    out = @ap.awesome(email_record)
    raw_object_string = <<~EOS.strip
      #<User:placeholder_id> {
                     "id" => nil,
          "email_address" => "foo@bars.com"
      }
    EOS

    expect(out).to be_similar_to(raw_object_string)
  end
end

Although, now that I'm looking at this I think this code might actually be breaking the default formatting, but I'm not sure. need sleep, will look again tomorrow

[gerrywastaken]

p.s. Thanks for merging those drafts into one commit. 👍

[adrianomitre]

@gerrywastaken I believe it can finally be merged now. 😃

p.s. Thank you for being supportive!

[gerrywastaken]

Sorry squiggly heredocs were only added in the latest versions of ruby, so you will need to use this instead:

      raw_object_string = <<-EOS.strip
#<User:placeholder_id> {
               "id" => nil,
    "email_address" => "#{e.email_address}"
}
EOS

You can copy the format of the other docblocks in the file and then you can test it works by running bundle exec appraisal rails-4.2 rspec spec/ext/active_record_spec.rb after setting your ruby version to a version of Ruby earlier than 2.3.0.

e.g. chruby 1.9.3 or whatever you use to manage your Ruby versions.

[gerrywastaken]

@adrianomitre No worries. The squiggly dock block is breaking on earlier versions of Ruby. My fault I didn't mention that in the earlier suggestion. See my comment on your code change.

Also I'm noticing a second strange commit has crept into this PR, are you able to rebase on master so that that commit isn't there?

[adrianomitre]

@gerrywastaken Done!

[gerrywastaken]

@adrianomitre Awesome... well done and thank you for your work! 👍