Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Queues - Marisol Lopez - Media Ranker #45

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

marisol-lopez
Copy link

@marisol-lopez marisol-lopez commented Apr 17, 2017

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer Instructor Feedback
Describe a custom model method you wrote. I wrote a method called by_category, which would be passed in a category and only show the works for that specific category.
Describe how you approached testing that model method. What edge cases did you come up with? I don't think I came up with any edge cases, I just wanted to test to make sure my validations were being required.
Describe an edge case test you wrote for a controller I didn't write any controller tests.
What are session and flash? What is the difference between them? Session is keeping track of the http request for the user that requested it, whereas flash is used to display a message of a completed action to the user. This isn't quite right. Both session and flash are data that is sent to the browser along with every response, and which the browser is supposed to send back with its next request. That means you can store different information for each browser. Data stored in the session persists until the browser is closed, while data in the flash is only sent for the next HTTP request (which is why it's great for sending messages to the user).
Describe a controller filter you wrote. I wrote a filter when it came to my login feature.
What was one thing that you gained more clarity on through this assignment? my routes, being creative with using different URI's for controller actions with different names.
What is the Heroku URL of your deployed application https://salty-basin-93659.herokuapp.com
Do you have any recommendations on how we could improve this project for the next cohort? More available time with tutors and TA's for help. It was difficult to get help from instructors because they were busy completing bi-weekly checkins and on some days where we had the entire day to the project, we would have to wait until 5 for tutors because there were no TA's that day.

@droberts-sea
Copy link

Media Ranker

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes
Comprehension questions yes - see responses in the PR above
General
Rails fundamentals (RESTful routing, use of named paths) yes
Semantic HTML mostly good, but missing in a few places
Errors are reported to the user no - when validations fail, you do a good job of re-rendering the form for the user, but don't tell them what went wrong. I had to go read your model code to figure out why I couldn't create a book.
Business logic lives in the models yes - good work
Model testing See below
Controller testing no
Wave 1 - Media
Splash page shows the three media categories yes
Basic CRUD operations on media are present and functional mostly - out of the box, the show page for media didn't work, seemed to be referencing a URL helper that didn't exist for the delete button.
Wave 2 - Users and Votes
Users can log in and log out yes
The ID of the current user is stored in the session yes
Individual user pages and the user list are present yes
A user cannot vote for the same media more than once yes
All media lists are ordered by vote count no
Splash page contains a media spotlight no
Media pages contain lists of voting users It looks like you started on this, but it consistently gives me an error if the media has any votes.
Wave 3 - Styling
Foundation is used appropriately yes
Look and feel is similar to the original good start with the layout, but there's still a way to go
Overall

Good work overall! This project was quite big with many moving parts, and it seems most of the core functionality is there. I'm not too worried about the more advanced voting features, but I do want to address your test coverage.

You've made a good start on model testing, but I would like to see a little more here. First, for your validations you should be testing why the validations failed. For example:

it "requires a title" do
  work = Work.new
  result = work.valid?
  result.must_equal false

  work.errors.messages.must_include :title
end

The last line ensures that it was in fact the title that was the problem, as opposed to some other validation.

Second, I would like to see some testing around the Work.by_category method. That's one of the big advantages of moving logic to the model, is it becomes easier to test! Some interesting test cases might include:

  • Does it return all the works with that category
  • Does it omit any works that don't have that category
  • What does it do if there are no matching works?

Writing good tests forces you to follow your code all the way through and think about all the possible states and interactions, and is an excellent learning tool. Make sure you spend some time practicing testing, especially controller testing, in bEtsy!

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