Skip to content

Ampers Steffany#40

Open
steffnay wants to merge 44 commits into
Ada-C9:masterfrom
steffnay:master
Open

Ampers Steffany#40
steffnay wants to merge 44 commits into
Ada-C9:masterfrom
steffnay:master

Conversation

@steffnay

Copy link
Copy Markdown

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe a custom model method you wrote. I made Publication#get_highest, which sorts the category of publication by most votes and returns the specified number of top publications. It takes the parameters "number" and "category."
Describe how you approached testing that model method. What edge cases did you come up with? I spent a long time trying to work on testing custom model methods but need some more help with that.
What are session and flash? What is the difference between them? They are both hash-like objects that Rails creates for making data persist from one request to another. With session, data is stored indefinitely between request cycles until the browser is closed or the session is ended. Flash stores data only for the next request cycle.
Describe a controller filter you wrote. I added find_user before filter to the Application controller. This method finds the user information before running any controller method in the program. This makes sure that the program keeps track of whether a user is logged in and who that user is.
What was one thing that you gained more clarity on through this assignment? I understand a lot more about the different relationships between classes, models, and controllers in Rails. I think I did a better job of keeping business logic out of the controller compared to RideShare, though I am still working on this. I also understand sessions a lot more than I did when we were going over it in class.
What is the Heroku URL of your deployed application? Link
Do you have any recommendations on how we could improve this project for the next cohort? I think you all did a good job of covering a lot of information. Using Ada-books was extremely helpful for learning and those examples were crucial to getting through the project. I do think it would be helpful to go over more examples of testing in Rails, though. It would have helped me to see some examples of testing custom model methods.

steffnay added 30 commits April 9, 2018 15:11
…ex view to display votes for each publication
@CheezItMan

Copy link
Copy Markdown

Media Ranker

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene Good git hygiene
Comprehension questions Check
General
Rails fundamentals (RESTful routing, use of named paths) You have routes for votes that you don't need.
Semantic HTML Yes, although your header and main elements are not nested inside the body element.
Errors are reported to the user Errors are not reported for errors creating or updating publications. They are reported for duplicate upvotes or upvoting without being logged in.
Business logic lives in the models Check, I really like your highest methods, they make good use of the order method and limit.
Models are thoroughly tested, including relations, validations and any custom logic Overall pretty good, I left some notes in your code. You are missing some tests for custom methods in publications.
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, although you need to check for some errors in the create action.
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 Nicely done
Look and feel is similar to the original Looks pretty close, not perfect but close.
Overall Pretty good, you need to do more checking for errors and reporting them with flash, and more through testing, but you did hit all the major learning goals. I left comments in your code slack me if you have questions.

end

def create
@publication = Publication.new(publication_params)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What about errors/flash notices here and in the update, destroy etc actions?

end

def show
@user = User.find(params[:id])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should probably have some error handling here incase you can't find that specific user.

@publication.save ? (redirect_to publication_path(@publication[:id])) : (render :new)
end

def destroy

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

shouldn't I need to be logged in to delete, upvote, update or create a work?

Comment thread config/routes.rb
post '/login', to: 'sessions#create', as: 'login'
delete '/logout', to: 'sessions#destroy', as: 'logout'

resources :votes

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do you need all the routes here for votes? I think you should only have a route to create a vote, maybe nested under publications You wouldn't need a route for index, or new

<%= csrf_meta_tags %>
<link rel="shortcut icon" type="image/x-icon" href="https://media-ranker-2-0.herokuapp.com/assets/favicon-910822ff6cd5f6d37119171c4b716f1d4e5fe0e8ad5e408627659ebf8d7f16b1.ico", alt: "site logo, an owl">
</head>
<header class="page-header">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The header, and main must be inside the body element.

describe Publication do

describe 'publication' do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You also need to have a test that a valid publication is valid, maybe using a fixture.

end
end

describe 'self.find_votes' do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You also need to test edge-cases, maybe where a work has no votes, or there are no votes in the system, etc.

Comment thread test/models/user_test.rb

it "requires a name" do
user = users(:bob)
user.name = ""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Odd, I was able to log into the site without giving my name, so something is strange here. Maybe you need to make sure that user.name must not be nil

number_votes = Publication.find_votes(publication.category)
number_votes.length.must_equal 2
end
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What about testing the spotlight, and highest?

session[:user_id] = @user.id
flash[:success] = "Successfully logged in as existing user #{@user.name}."
else
@user = User.create(name: params[:user][:name])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should have a check here to see if the create worked. What if someone tried to login with a space in their name?

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