Skip to content

Jessica Owens -- Carets#36

Open
vertige wants to merge 113 commits intoAda-C8:masterfrom
vertige:master
Open

Jessica Owens -- Carets#36
vertige wants to merge 113 commits intoAda-C8:masterfrom
vertige:master

Conversation

@vertige
Copy link

@vertige vertige commented Oct 16, 2017

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a custom model method you wrote. I wrote some Work model methods to help sort by category.
Describe how you approached testing that model method. What edge cases did you come up with? WIP
Describe an edge case test you wrote for a controller I wrote edge cases for IDs that didn't exist.
What are session and flash? What is the difference between them? Session is a hash that rails provides that is persistent throughout the browser session. Flash is a hash that rails provides that is persistent through 2 (or 1 for flash.now) browser requests.
Describe a controller filter you wrote. There's a #find_work method that I set to happen before #show, #edit, #update and #destroy to find the Work at the params ID.
What was one thing that you gained more clarity on through this assignment? How to laugh and cry with git.
What is the Heroku URL of your deployed application WIP
Do you have any recommendations on how we could improve this project for the next cohort? Save it for a full week, not enough class time/@ada time to cover new subject matters that were used in Media Ranker with compressed lecture. Maybe introduce the concept of Controller testing right after Ride Share? Would have preferred this to be a pair project, as with all the new material, I was significantly derailed with troubleshooting by my lonesome.

…ch category as well as a spotlight section for the top media overall
@CheezItMan
Copy link

Media Ranker

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good # of commits, good commit messages.
Agile practices (trello board) NA
Comprehension questions Check, I understand it was a compressed with with lots of newer material
General
Rails fundamentals (RESTful routing, use of named paths) Well done, nicely RESTful paths where possible.
Semantic HTML Nicely done
Errors are reported to the user Check
Business logic lives in the models Sorting done in the Model methods
Models are thoroughly tested, including relations, validations and any custom logic You're missing tests on business logic methods, pretty good testing on Validations and relationships. You also need to test that a user can vote for multiple different works.
Controllers are thoroughly tested Some good stuff here, you're obviously not done, but you had a good start on it.
Wave 1 - Media
Splash page shows the three media categories Check
Basic CRUD operations on media are present and functional Check
Wave 2 - Users and Votes
Users can log in and log out Check
The ID of the current user is stored in the session Check
Individual user pages and the user list are present Check
A user cannot vote for the same media more than once Check
All media lists are ordered by vote count Check
Splash page contains a media spotlight Check
Media pages contain lists of voting users Check
Wave 3 - Styling
Foundation is used appropriately Check
Look and feel is similar to the original Pretty close, nice work! }
Overall Nice!, you are definitely still learning testing and not finished with it, but there's a lot of good stuff here. You did a good job learning Foundation in a few days and getting the basic functionality up. I'd focus on spending some time with both Controller and Model testing on the bEtsy project to solidify those skills. Overall nice work.

Copy link

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few notes.

new_book = Work.new(title: book.title, category: book.category)
another_book = Work.new(title: book.title, category: book.category)

(new_book && another_book).valid?.must_equal false

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this does what you think it does.

Instead (new_book && another_book) returns another_book which has valid? called on it. It doesn't use new_book at all.

Instead you should have

another_book.valid?.must_equal false
new_book.valid?.must_equal false

end

describe "if the user doesn't exist" do
it "should have an error in session" do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This really isn't what the describe block describes. Better would be, describe 'if the user field is nil, or empty string the user is presented with an error message'

must_respond_with :success
end

describe "if a user exists" do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also test if a new user logs in successfully the number of users goes up and if an existing user logs in the number remains constant.

end

describe "show" do
it "should get user detail page (#show) or render a 404" do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test isn't about rendering a 404.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants