we should provide a console rake task so that students can drop into pry console and access/manipulate their models and database tables, to better understand the associations, how to use the code they are building.

An instructor and I spent almost an hour trying to figure out why my controller's route's instance variable wasn't showing anything in the view. It would have been nice if the instructions mentioned running rake db:migrate so that the list would be populated, otherwise nothing shows up in the view pages.

We don't really care if the status code is 200. We care what is on the page? Is it the correct stuff.

This will require using capybara

There is no redirect happening here. If you follow the spec description not the implementation it suggests to use a redirect not a render. In Sinatra there is a difference between redirect and render. Please be more explicit.

    context "changing a song's artist" do

      it "updates the song's artist" do
        fill_in "Artist Name", with: "Some Nobody"
        click_on "Save"

        expect(page).to have_content("Song successfully updated.")
        expect(page).to have_content(song_name)
        expect(page).to have_content("Some Nobody")
      end

      it "redirects to the song show page" do
        fill_in "Artist Name", with: "That singer"
        click_on "Save"
        expect(page.current_path).to eq("/songs/that-one-with-the-guitar")
      end
    end

Why make "Roles" table and not stick with figure titles? Also model associations regarding roles are wrong. Role class says has_many titles and figures. should be belongs to.

Inside the instructions for the lab, the lab wants us to create a column for the "Landmarks" table called year_completed. In the instructions, rows for the chart show the year 50 B.C., which would imply there needs the column value needs to be a string, but the test "has a name and a year_completed" is looking for number values. Either the test or that entry on the chart needs to be changed.

The README.md's table for the landmark table is missing a year_completed column. Not a big deal, I know, but it should probably reflect what the tests require.

either add seed data and make clear that students will be writing their own seed file and talk about how to do that.

PROBLEM: Refactor specs. See issues #38, #37
NOTIFY: @itzsaga, @bhollan

this repo has one test file, for Landmarks Controller. Missing tests for Figures Controller and Titles Controller. Is this intentional? Older versions of this lab had those files.

The Landmarks View Controller Spec doesn't have a .destroy_all for figures, leading to crazy amounts of Beyonce.

[1] pry(#)> Figure.all
=> [#<Figure:0x000000000449fe40 id: 1, name: "Beyonce">,
#<Figure:0x000000000449fcb0 id: 2, name: "Beyonce">,
#<Figure:0x000000000449fb70 id: 3, name: "Beyonce">,
#<Figure:0x000000000449f918 id: 4, name: "Beyonce">,
#<Figure:0x000000000449f788 id: 5, name: "Beyonce">,
#<Figure:0x000000000449f508 id: 6, name: "Beyonce">,
#<Figure:0x000000000449f378 id: 7, name: "Beyonce">,
#<Figure:0x000000000449f1c0 id: 8, name: "Beyonce">,
#<Figure:0x000000000449eea0 id: 9, name: "Beyonce">,
#<Figure:0x000000000449ed38 id: 10, name: "Beyonce">,
#<Figure:0x000000000449ebf8 id: 11, name: "Beyonce">,
#<Figure:0x000000000449ea40 id: 12, name: "Beyonce">,
#<Figure:0x000000000449e8b0 id: 13, name: "Beyonce">,
#<Figure:0x000000000449e770 id: 14, name: "Beyonce">,
#<Figure:0x000000000449e630 id: 15, name: "Beyonce">,
#<Figure:0x000000000449e4a0 id: 16, name: "Beyonce">,
#<Figure:0x000000000449e338 id: 17, name: "Beyonce">,
#<Figure:0x000000000449e158 id: 18, name: "Beyonce">,
#<Figure:0x000000000449dfc8 id: 19, name: "Beyonce">,
#<Figure:0x000000000449de60 id: 20, name: "Beyonce">,
#<Figure:0x000000000449dcd0 id: 21, name: "Beyonce">,
#<Figure:0x000000000449db40 id: 22, name: "Beyonce">,
#<Figure:0x000000000449d910 id: 23, name: "Beyonce">,
#<Figure:0x000000000449d758 id: 24, name: "Beyonce">,
#<Figure:0x000000000449d5f0 id: 25, name: "Beyonce">,
#<Figure:0x000000000449d438 id: 26, name: "Beyonce">,
#<Figure:0x000000000449d2a8 id: 27, name: "Beyonce">,
#<Figure:0x000000000449d0a0 id: 28, name: "Beyonce">,
#<Figure:0x000000000449cec0 id: 29, name: "Beyonce">,
#<Figure:0x000000000449cd30 id: 30, name: "Beyonce">,
#<Figure:0x000000000449cbc8 id: 31, name: "Beyonce">,
#<Figure:0x000000000449ca10 id: 32, name: "Beyonce">,
#<Figure:0x000000000449c858 id: 33, name: "Beyonce">,
#<Figure:0x000000000449c650 id: 34, name: "Beyonce">,
#<Figure:0x000000000449c4c0 id: 35, name: "Beyonce">,
#<Figure:0x000000000449c358 id: 36, name: "Beyonce">,
#<Figure:0x000000000449c1a0 id: 37, name: "Beyonce">,
#<Figure:0x000000000449c010 id: 38, name: "Beyonce">,
#<Figure:0x00000000044a3d88 id: 39, name: "Beyonce">,
#<Figure:0x00000000044a3bf8 id: 40, name: "Beyonce">,
#<Figure:0x00000000044a3a68 id: 41, name: "Beyonce">,
#<Figure:0x00000000044a38d8 id: 42, name: "Beyonce">,
#<Figure:0x00000000044a3748 id: 43, name: "Beyonce">,
#<Figure:0x00000000044a3568 id: 44, name: "Beyonce">,
#<Figure:0x00000000044a3388 id: 45, name: "Beyonce">,

both the patch and delete methods are unneeded to pass the tests in this lab.

In the readme for this lab it states that we should create migrations to match the tables that are displayed. Also in /db/migrate/01_create_landmarks.rb there are instructions to "Write CreateLandmarks migration here"

however

In /db/schema.rb we can see that the landmarks table has already been created prior to the start of the student beginning the lab. Also of note is that the table in schema.rb is not the same as the table represented in the readme . It has datetime columns.

Can we add a screenshot of a working new figure form to the readme to qualify the instructions for students? Something like this:

It might be helpful to move this explanation of join tables to the previous lab "Sinatra Playlister". The concept is used in both labs, but causes confusion in the previous lab when students are setting up their associations / db (questions were popping up in the "Ask a Question" feature). Then, it is described in detail here (in "Sinatra NYC"), after the student has already struggled through it and no longer needs the help.

Just a thought.

The controller tests for this lab should be re-factored to use the view mechanics a bit less.

CC: @aturkewi

Landmark.figure shouldn't be nil. It should be filled out so that the show page doesn't fail

I'm not sure if this is the proper place to do this, but I think the tests on this, and the playlister lab, are too specific. I write my code, get everything to work, and then I have to spend the same, if not more, time changing my code to "fit" the tests. I spend a lot of time just trying to get the tests to pass, even though my code is working fine. I also feel that the tests restrict creativity.

I just wanted to let someone know how.

We were passing down the prop as suggested in the reading for the lab but we also needed to pass the pet object as well in order to get the functionality we needed. the state of the data was changing each time we would adopt a pet and then filter through the animals.

Hi,

I found the lab prior to sinatra NYC (sinatra playlister) to be significantly more difficult due to the addition of the flash and some other items. I would suggest switching the order of the two labs.

add sort to config/environment Dir[File.join(File.dirname(__FILE__), "../app/controllers", "*.rb")].each {|f| require f}, because file order isn't guaranteed and sometimes the application controller is required after the other controller (causing an uninitialized constant error).

fix: Dir[File.join(File.dirname(FILE), "../app/controllers", "*.rb")].sort.each {|f| require f}

a few issues with the solution:

Figure Controller should be Figures Controller, config.ru should reflect this.

Figure Controller missing routes and views for editing and creating figures. There are no tests for this, or for anything Figure-related or Title-related although Readme asks students to build out features for CRUD for each model.

This should be a patch not post -- https://github.com/learn-co-curriculum/nyc-sinatra/blob/solution/app/controllers/figures_controller.rb#L37

The form is making a patch request

Note: In order for it to interpret the patch in the controller, use Rack::MethodOverride needs to be added to the config.ru

Might be worth adding something like this as a resource in the readme: https://stackoverflow.com/questions/5166484/sending-a-delete-request-from-sinatra

@cernanb @ruthmesfun

So in the instructions it's recommended to run rspec ./spec/models/01_thing_spec.rb so you can build things in order. Since RSpec by default will run files\folders in aphabetical order you can prepend numbers to your folders to fix this issue. Then the students just have to run learn or rspec and the tests will happen in order. This lab has no controller tests obviously but I would usually do 01_models, 02_controllers, 03_features since the features are a result of a working model and controller. I'd assume to work with the controller you could use a double or web_mock to insure it's receiving requests and handling them properly in return sending out the expected response.

recommended folder naming convention.

./spec
  /01_models
  /02_features

#30

it couldn't find ApplicationController on my Linux box. worked fine in Nitrous but I like to develop locally.

Resources video has been removed, consider changing to a different video.

http://screencasts.org/episodes/activerecord-with-sinatra

This lab requires users to write their own Capybara specs but we don't teach that at all

• The README mentions checkboxes, but the tests never once check for their presence.
• The README mentions having only "available" things be listed, but neither explicitly defines that, nor do the tests check for that.
• The README mentions a JOINS table, but the tests never really check for joins functionality (look through student pull requests to see who actually populates it).
• The README also says the lab is "test-driven", but you can quite easily get these tests to pass without building much real functionality in, e.g. a test might check for (page.body).to include('figure[name]'), but that spec can easily pass with just a comment, or any text that has that.

The specs/controllers/models folder should be directly under specs.

There are also multiple spelling/grammar typos in the README.

Normally, I would just create a pull request with my suggested changes, but these seem a bit steep for the schedule I'm on.