Rails Fundamentals - Active Record Queries

Exercise Questions
1

This topic is for the [Active Record Queries] exercise in the [Rails Fundamentals] trail. Post any questions, corrections, or pointers you have to share with other Upcase subscribers.
[Active Record Queries]: https://exercises.upcase.com/exercises/active-record-queries
[Rails Fundamentals]: Rails Fundamentals | Online Tutorial by thoughtbot

2

Hi there,

It seems there is some undesirable functionality in this iteration of the app which is not being caught by the tests. The visitor_views_guestbook_entries_spec tests for proper scoping of entries on the home page. But if an invalid entry is made (and the home page re-rendered), this behavior is not tested for scoping.

In the else branch of the create action in the controller, the following line causes all the entries to be displayed when the page is rendered:

@guestbook_entries = GuestbookEntry.all

It would appear that this is not the intended behavior of the app.

As a suggestion, adding the following example to the visitor_views_guestbook_entries_spec.rb file catches this:

  scenario "properly scopes entries in case of errors" do
    create_old_guestbook_entry("Hi from a long time ago!")
    (1..6).each { |n| create_recent_guestbook_entry("Entry #{n}") }
    visit root_path
    fill_in "Guestbook Entry:", with: ""
    click_button "Submit"

    expect(page).to have_content "can't be blank"
    expect(page).not_to have_content("Hi from a long time ago!")
    expect(page).not_to have_content("Entry 1")
  end
1 Like
3

Hey Josh,

Thanks for pointing this out and suggesting a fix.

I think youā€™re right that in a real app this might be unwanted behavior. However, Iā€™m not 100% convinced yet that we should add your new test to the exercise. I worry that it makes the exercise a bit less clear.

Did you find the existing behavior confusing as you were trying to complete it, or is this just something you happened to notice as you were working on it?

Thanks!

4

Hey Ben,

Actually, I didnā€™t see it when working on the exercise myself. But I noticed the issue in someone elseā€™s solution, and I realized that the display output for the ā€œunhappy pathā€ effectively breaks the view in cases where the db contains more than a dozen or so entries.

Point 4 of the exercise instructions says this: ā€œChain these scopes together in the controller so that the GuestbookEntry records are displayed correctly on the guestbook home page.ā€ If my controller action looks like this:

  def create
    @guestbook_entry = GuestbookEntry.new(guestbook_entry_params)

    if @guestbook_entry.save
      redirect_to root_path, notice: "Thank you for your entry."
    else
      @guestbook_entries = GuestbookEntry.all # No scoping here
      render :index
    end
  end

ā€¦(which is all the spec calls for) then when I am redirected to the index page, the error message and indeed the form itself are well off the screen. For demonstration, I added 60 guestbook entries to the db. See screenshots here:

To my mind, this is a classic mistake that I can see myself making in development, and then discovering later. Which, as it turns out, is exactly what happened here.

That said, if you think the extra test muddies the exercise, I certainly understand. Simplicity vs real-world/completeness is, Iā€™m sure, a fine line to walk when creating these things. But given the instructions, I felt that the requirements were not fully met without applying the scope chain to both possible paths in the controller. The suggested spec would simply serve as an indicator that both successful and unsuccessful outcomes are important.

So thereā€™s my two cents. Hope itā€™s helpful either way. And by the way, may I say that on the whole, these exercises are amazing! Iā€™ve never seen anything like this in any other Rails training Iā€™ve done. I procrastinated in starting them for a while, but now that Iā€™ve begun, Iā€™m enjoying them immensely. Great work!

5

i donā€™t think it would be confusing to replace GuestbookEntry.all with GuestbookEntry.limited in the create block.

6

GuestbookEntry.limited is not the answer. But whatever the answer is could be used in the create block.