Skip to content

Christina MediaRanker#42

Open
Peacegypsy wants to merge 17 commits intoAda-C10:masterfrom
Peacegypsy:master
Open

Christina MediaRanker#42
Peacegypsy wants to merge 17 commits intoAda-C10:masterfrom
Peacegypsy:master

Conversation

@Peacegypsy
Copy link

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a custom model method you wrote. Still working- Let Chris know, family emergency this weekend.
Describe how you approached testing that model method. What edge cases did you come up with?
What are session and flash? What is the difference between them?
Describe a controller filter you wrote.
What was one thing that you gained more clarity on through this assignment?
What is the Heroku URL of your deployed application? https://xtina-media-ranker.herokuapp.com/
Do you have any recommendations on how we could improve this project for the next cohort?

@CheezItMan
Copy link

Media Ranker

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Not enough commits, but it also looks like you didn't get as far as you wanted.
Comprehension questions Missing I understand you had a family emergency.
General
Rails fundamentals (RESTful routing, use of named paths) You have a lot of extra unused routes. Take a look at my inline comments
Views are well-organized (DRY, use of semantic HTML, use of partials) You've got hardcoded data in your works#index and homepage#index views. I don't understand why you're not using model data.
Errors are reported to the user You aren't using form_with and so the app won't create new works.
Business logic lives in the models You have some business logic methods, but they aren't being used as far as I can see.
Models are thoroughly tested, including relations, validations and any custom logic Some tests, but not many and not testing validations, or custom methods.
Wave 1 - Media
Splash page shows the three media categories You have it, but only with hardcoded data.
Basic CRUD operations on media are present and functional INCOMPLETE
Wave 2 - Users and Votes
Users can log in and log out MISSING The route is broken, see my note in routes.rb
The ID of the current user is stored in the session MISSING
A user cannot vote for the same media more than once MISSING
All media lists are ordered by vote count MISSING
Splash page contains a media spotlight MISSING
Wave 3 - Users and Votes
Media pages contain lists of voting users MISSING
Individual user pages and the user list are present MISSING
Optional - Styling
Bootstrap is used appropriately You do have some bootstrap styling going on.
Look and feel is similar to the original Some progress has been made here.
Overall I understand your family emergency, however this app shows a serious misunderstanding of Rails. You weren't using form_with to create forms to make new works, didn't have the login routes set up, and didn't really use Rails models in your controllers/views. You did have a start at some testing. We really need you to demonstrate a more solid understanding of Rails fundamentals with this app. I would like you to try to update this app and let me know when you would like me to take a second look.


root "welcome#index"

resources :users

Choose a reason for hiding this comment

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

Do you really need all routes for each of these controllers? Limit the routes to only the ones you're using.

Example:

  resources :users, only: [:index, :show]

fields = [:movie, :book, :album ]

fields.each do |field|
expect(vote).must_respond_to field

Choose a reason for hiding this comment

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

This test is failing because votes don't have a separate field for the different categories, it should be:

fields = [:work, :user]

let(:vote) { Vote.new }

it "must be for a valid category" do
value(vote).must_be_kind_of :work

Choose a reason for hiding this comment

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

This doesn't make sense because votes aren't work.

end

it 'user has the required fields' do
fields = [:user_name ]

Choose a reason for hiding this comment

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

You should also include the field :votes in this list.

require "test_helper"

describe User do
let(:user) { users(:simple) }

Choose a reason for hiding this comment

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

You also need a validation test for user_name

top_ten("movie")
end

def self.top_ten(category)

Choose a reason for hiding this comment

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

These are some great custom methods!

<%= stylesheet_link_tag 'application', 'https://fonts.googleapis.com/css?family=Gudea:400,700|Hammersmith+One' %>
<%= stylesheet_link_tag 'application', media: 'all', 'data-turbolinks-track': 'reload' %>
<%= javascript_include_tag 'application', 'data-turbolinks-track': 'reload' %>
<%= favicon_link_tag 'favicon.ico' %>

Choose a reason for hiding this comment

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

I'm not seeing this favicon in your files, so it's not running for me.


<tr>
<td>25</td>
<td><a href="/works/134">The Hipster and the Beast</a></td>

Choose a reason for hiding this comment

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

Why were you hardcoding this here rather than using model data?

<h2>Add a new work</h2>
<section>
<label>Category</label>
<select class="form-control" name="work[category]"><option value="album">album</option>

Choose a reason for hiding this comment

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

You are not using form_with or other Rails form controls.


get "/:page" => "welcome#index"

get '/login', to: 'sessions#new'

Choose a reason for hiding this comment

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

You don't have a new method in your sessions controller.

# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html

one:
category: movie

Choose a reason for hiding this comment

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

Why do votes have a category?

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