Skip to content

Conversation

@Lasiorhine
Copy link

@Lasiorhine Lasiorhine commented Apr 14, 2018

NOTE: THIS IS AN UPDATED PULL REQUEST. I spotted several major bugs after deployment (the worst having to do with a problem on form for Work, and a couple of other having to do with bad redirects, plus one to do with an improper tag on the Work show page, and one to do with a raise left in the vote model.) I fixed them as of 4/15.

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
**Describe a custom model method you wrote. ** I wrote several custom methods, but the one with the best automated tests is report_works_voted_for, which renders up a set of Work instances that a given user has voted for.
Describe how you approached testing that model method. What edge cases did you come up with? ** With that one, testing with the YAML actually worked out pretty well. Since so much that has to do with votes relies on ids for user and work -- which you cannot know in advance w/re much of the test data-- I thought this might end up being really gnarly. Turns out, it was actually pretty easy. I could make a bunch of votes in the YML with integers as user and work ids and they worked just fine.
**What are session and flash? What is the difference between them? ** Session and Flash are both hash-like objects that persist in the browser for a set period of time. Session lasts from login until the browser is shut down, while Flash is much more transient: It lasts for only one request.
**Describe a controller filter you wrote. ** Full disclosure: I did not actually do that. I could have done it (and if I'd had more time to refactor, I would have done it) for the user-finding first step of the Show and Create methods within User. But, alas, I ran out of time. (That I ran out of time to do something designed to save time is fairly hilarious, I thnk. Granted, It might not be quite so funny when i get the assessment back.,, )
**What was one thing that you gained more clarity on through this assignment? ** I feel like, in the past two days, I've developed a much better understanding of some parts of Rails that were flummoxing me. I'm a lot more comfortable with coding in the models now, and I'm starting to get a bit of a handle on testing. I should, however, disclose that I turned this in with a non-passing test for a custom Vote method (even though I know the method works) because I can't quite figure out how to make the test do what I want. I also have about ten stubbed-but-unwritten tests for some custom methods in Work. I also gained a VERY LEGIT understanding of why you have to deploy early and often, because I just got to spend the last 3 or so hours learning how to read Heroku logs and troubleshoot deployment all by myself, while both my Chihuahua and my husband snored audibly in the background. (Such a weird combination of peaceful and stressful that was-- like getting audited by the IRS while sitting beside a mountain stream.)So ultimately, this was grueling but highly rewarding, and I'm feeling like I'll be able to make a reasonable contribution to my team's Betsy project.
**What is the Heroku URL of your deployed application? ** https://pretty-darned-rank.herokuapp.com/
**Do you have any recommendations on how we could improve this project for the next cohort? ** My brain is a dial tone right now. if I think of anything, I'll bring it up at retro.

…to the params methods in the users and works controllers to accommodate the actual fields in the various tables.
… Works is officially functional; YAMLs for Vote and User are not yet tested, but written
…breaking up login methods in Session to allow appropriate redirection.
… have to be both the last commit and the last branch.
@CheezItMan
Copy link

Media Ranker

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good git hygiene!
Comprehension questions Check, please be careful in editing the comprehension questions, it's hard to read if the table formatting is messed with. I fixed it, but it took a minute or three.
General
Rails fundamentals (RESTful routing, use of named paths) You've got some extra routes in the VotesController.
Semantic HTML Good use of section, header, main etc.
Errors are reported to the user Check
Business logic lives in the models Check, some of the Work methods seem a bit overly complicated. See my notes in-line.
Models are thoroughly tested, including relations, validations and any custom logic MISING TESTS for the Work model. Looks like you stubbed out quite a bit however. I left some notes for your existing tests.
Wave 1 - Media
Splash page shows the three media categories Check
Basic CRUD operations on media are present and functional It will let me create work without being logged in. That's a bug. It will also let me delete works without being logged in. Otherwise it works as advertised.
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, but the show pages does not work! See my in-code note.
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 Looks pretty good! Nice work
Look and feel is similar to the original " It's Fun to Judge Things" LOL. You did a nice job overall, not perfect, but very good.
Overall Overall nice work, you didn't get all the testing done, and as you noted you finished a few things late, and you have a bug when viewing a user. However you hit the major learning goals. You demonstrated session, and flash notices and some testing. Have fun with bEtsy this week, I'm glad you found it a challenging, but worthwhile learning experience.


resources :users, except: [:edit, :update]

resources :votes, except: [:new, :create, :destroy]

Choose a reason for hiding this comment

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

So people can edit, and update votes? What about index and show?

<%= link_to "See all Users", users_path, class: 'button' %>
<%= link_to "Back to Media List", works_path, class: 'button' %>

<h1><%= Work.find_by(title: "Dogzilla").title %></h1>

Choose a reason for hiding this comment

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

This is causing the site to crash. Did you leave it in by accident?


validate :one_vote_per_user_per_work

def one_vote_per_user_per_work

Choose a reason for hiding this comment

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

Neat custom validation. You can also do something similar with uniqueness and scope like in the rails api guide

end

def self.report_works_voted_for(id_for_user)
@upvoted_works = self.where(user_id: id_for_user)

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 method is really very useful as any Work instance has a votes method.

validates :title, length: { in: 1..35 }


def self.works_with_vote_tallies

Choose a reason for hiding this comment

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

These methods seems overly complicated. Couldn't you do a method to get the works in order by the number of votes with:

works = Work.all.to_a
return works.sort_by { |work| -work.total_votes }

And you can always get the vote tally for a work with: work.votes.count

end


it "contains elements in which index position 1 is the correct tally of votes for the element at index position 0" do

Choose a reason for hiding this comment

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

missing test?

work.valid?.must_equal false
end

describe "works_with_vote_tallies" do

Choose a reason for hiding this comment

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

You should also test this method when no votes have been issued for any work, and when there are no works period.


describe "works_ordered_by_popularity" do

it "returns an array" do

Choose a reason for hiding this comment

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

Missing tests


def show
id = params[:id]
@work = Work.find(id)

Choose a reason for hiding this comment

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

Hmm a filter would go swell here!

flash[:alert] = @vote.errors
end
end
redirect_to work_path(params[:work_id])

Choose a reason for hiding this comment

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

Maybe redirect_back would be better here.

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