-
Notifications
You must be signed in to change notification settings - Fork 47
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
Space - Antonia #47
base: master
Are you sure you want to change the base?
Space - Antonia #47
Conversation
…nk from homepage to works that are displayed by category
…, delete, and add works
…en things in works controller, and added some styling
…still a bug in logout
…es that display it
… views that use them
Media RankerFunctional Requirements: Manual Testing
Major Learning Goals/Code Review
Previous Rails learning, Building Complex Model Logic, DRYing up Rails Code
Testing Rails Apps
Overall Feedback
Code Style Bonus AwardsWas the code particularly impressive in code style for any of these reasons (or more...?)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good effort!
It looks like you really struggled with some of the code requirements though. Specifically using newer features like partials and views and also with doing testing.
You did manage to get almost all of the functionality built in though, so well done on that front.
<section class="top-ten__container"> | ||
<section class="top-ten__list-container"> | ||
<h2 class="top-ten__header">Top Albums</h2> | ||
<ul class="list-group top-ten__list"> | ||
<% Work.top_ten("album").each do |work| %> | ||
<h4><%= link_to work.title, work_path(work.id)%></h4> | ||
<p>by <%= work.artist%></p> | ||
<p> <%= work.votes.count %> votes</p> | ||
<br> | ||
<% end %> | ||
</div> | ||
</section> | ||
</ul> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something funky is going on with your tags here.
<ul>
s should contain <li>
s not a mix of tags and your closing tags are scrambled and you have closing tags for two <div>
s that aren't open. It looks like maybe a conversion of <div>
s into <sections>
that only made it halfway.
<section class="top-ten__container"> | |
<section class="top-ten__list-container"> | |
<h2 class="top-ten__header">Top Albums</h2> | |
<ul class="list-group top-ten__list"> | |
<% Work.top_ten("album").each do |work| %> | |
<h4><%= link_to work.title, work_path(work.id)%></h4> | |
<p>by <%= work.artist%></p> | |
<p> <%= work.votes.count %> votes</p> | |
<br> | |
<% end %> | |
</div> | |
</section> | |
</ul> | |
</div> | |
<section class="top-ten__container"> | |
<section class="top-ten__list-container"> | |
<h2 class="top-ten__header">Top Albums</h2> | |
<ul class="list-group top-ten__list"> | |
<% Work.top_ten("album").each do |work| %> | |
<li> | |
<h4><%= link_to work.title, work_path(work.id)%></h4> | |
<p>by <%= work.artist%></p> | |
<p> <%= work.votes.count %> votes</p> | |
</li> | |
<% end %> | |
</ul> | |
</section> | |
</section> |
</thead> | ||
<tbody> | ||
<% @user.votes.each do |vote|%> | ||
<% work = Work.find_by(id: vote.work_id) %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use vote.work
instead of doing a find_by
here:
<% work = Work.find_by(id: vote.work_id) %> | |
<% work = vote.work %> |
Media Ranker
Congratulations! You're submitting your assignment!
Comprehension Questions
session
andflash
? What is the difference between them?