Amanda Ungco-MediaRanker by unkcodesquick · Pull Request #29 · Ada-C10/MediaRanker

unkcodesquick · 2018-10-15T06:21:54Z

Media Ranker

Congratulations! You're submitting your assignment!

Comprehension Questions

Question	Answer
Describe a custom model method you wrote.	I wrote two costum class methods to sort the works by category and top vote.
Describe how you approached testing that model method. What edge cases did you come up with?	I tested the model method by using the website due to limited time. Some edge cases I would've tested would be what happens when there is a tie in the number of votes in a given category.
What are `session` and `flash`? What is the difference between them?	Flash sends one time messages from the controller to the view. A session is used to keep track of data while a user has the given browser window open. The data is automatically deleted when the window is closed.
Describe a controller filter you wrote.	I created a controller filter for my entire website that sets the user_id of the session to the current user. This allows the website to know when someone is logged in or not and provides the information needed for the different actions to behave accordingly.
What was one thing that you gained more clarity on through this assignment?	How to use inspect/write class methods.
What is the Heroku URL of your deployed application?	https://aungco-media-ranker.herokuapp.com
Do you have any recommendations on how we could improve this project for the next cohort?	Since the css styling is optional, it would be nice if the CSS was provided and then students just had to match the HTML to it. I spent a lot of time copy and pasting from the Heroku inspector that I should've spent on writing tests but felt pressured to have the site match the example.

…lter

…table relationshps

Match2 constructor media ranker

CheezItMan · 2018-10-23T21:33:17Z

Media Ranker

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene	Good number of commits and good commit messages
Comprehension questions	Check, "costum" class methods? :p Sorry you found styling to be a big hassle.
General
Rails fundamentals (RESTful routing, use of named paths)	Overall good, but you have some unused routes for `UsersController`.
Views are well-organized (DRY, use of semantic HTML, use of partials)	You could definitely use more partials to avoid having to repeat view code for some of the tables.
Errors are reported to the user	For the most part, the edit-update options for work do not deliver flash notices.
Business logic lives in the models	Some logic in the models, the upvoting is all done in the controller which isn't a big issue, but it could also be done in the model.
Models are thoroughly tested, including relations, validations and any custom logic	There's some good stuff here, but it looks like you ran out of time. See my inline notes.
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
The ID of the current user is stored in the session	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
Wave 3 - Users and Votes
Media pages contain lists of voting users	Check
Individual user pages and the user list are present	Check
Optional - Styling
Bootstrap is used appropriately	For the most part yes
Look and feel is similar to the original	Pretty close, nice work
Overall	Overall, nice job, it does look like you put testing off till the end and didn't get finished, but you handled all the functionality and most of your tests are pretty good. See my inline comments and let me know any questions you have.

CheezItMan · 2018-10-23T21:02:41Z

config/routes.rb

+Rails.application.routes.draw do
+
+  root 'works#main'
+  resources :users


Do you need the full CRUD routes for users? You're not using :edit or :destroy for example.

CheezItMan · 2018-10-23T21:03:46Z

app/models/user.rb

@@ -0,0 +1,5 @@
+class User < ApplicationRecord
+  has_many :works


No validations? Username can be blank? What about requiring unique usernames?

This isn't essential, but something to think about.

CheezItMan · 2018-10-23T21:05:10Z

app/models/vote.rb

+  belongs_to :user
+  belongs_to :work
+
+  validates :user_id, presence: true, uniqueness: { scope: :work, message: "You can only add one vote per work"}


Since a vote belongs to a user by default user_id is required. The uniqueness of combining user_id and work_id is good. The custom message is great!

CheezItMan · 2018-10-23T21:05:51Z

app/models/work.rb

+  has_many :votes, dependent: :destroy
+  # has_many :users, through :votes
+
+  validates :title, presence: true, uniqueness: {scope: :category}


I like the uniqueness using the scope category here.

CheezItMan · 2018-10-23T21:06:37Z

app/models/work.rb

+  validates :publication_year, numericality: true, length: {is: 4}
+  validates :creator, presence: true
+
+  def self.sort_by_category(category)


The name sort_by_category doesn't sound right since this method doesn't really do sorting. Instead it does filtering.

CheezItMan · 2018-10-23T21:28:43Z

test/models/work_test.rb

+    end
+
+    it 'can only be voted on by a unique user once' do
+      # first_vote = Vote.first


I would suggest here, creating a new vote which is identical to one from your fixtures.

CheezItMan · 2018-10-23T21:29:15Z

test/models/work_test.rb

+    it 'must have a publication_year with a 4 integers' do
+      work.publication_year = 0
+        # Arrange
+        work.publication_year += 999


Why add 999 here?

CheezItMan · 2018-10-23T21:30:36Z

test/models/work_test.rb

+      expect(poodr.errors.messages).must_include :title
+    end
+  end
+


No test for category being present?

CheezItMan · 2018-10-23T21:31:06Z

test/fixtures/votes.yml

+# below each fixture, per the syntax in the comments below
+#
+one:
+  user_id: 1


You should use the nicknames of existing fixtures in users.yml and works.yml

CheezItMan · 2018-10-23T21:31:21Z

test/fixtures/works.yml

+  category: movie
+  description: magical movie
+
+


unkcodesquick and others added 20 commits October 9, 2018 15:50

set up new rails project

ab05076

created work model and seeded database

a101ae5

added validations/flash errors for adding new work

889fa78

fixed error messages, added tests for works model, added controler fi…

6ede564

…lter

passed all tests wave 1

7e7e667

added user/votes models and added references to works

2b62681

added votes controller, sessions controller, users controller, fixed …

8572bdf

…table relationshps

added upvote and fixed user login

0a22ead

updated views and controllers, for all show pages

09edabe

updated votes view in work show page

c940c5c

updated list of users view

f7ce69e

started css styling

9e97c7b

fixed buttons

e220d8c

fixed dependency between deleting work and votes

47c4095

updated work controller

6124bab

fixed button on index page

df4bf36

fiexed buttons on user show page

2f0b742

updated work tests

6f91818

Merge pull request #1 from amandaungco/Match2ConstructorMediaRanker

72ea2db

Match2 constructor media ranker

Merge branch 'master' of https://github.com/amandaungco/MediaRanker

f6a3a10

CheezItMan reviewed Oct 23, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Amanda Ungco-MediaRanker#29

Amanda Ungco-MediaRanker#29
unkcodesquick wants to merge 20 commits intoAda-C10:masterfrom
unkcodesquick:master

unkcodesquick commented Oct 15, 2018

Uh oh!

CheezItMan commented Oct 23, 2018

Uh oh!

CheezItMan Oct 23, 2018

Uh oh!

CheezItMan Oct 23, 2018

Uh oh!

CheezItMan Oct 23, 2018

Uh oh!

CheezItMan Oct 23, 2018

Uh oh!

CheezItMan Oct 23, 2018

Uh oh!

CheezItMan Oct 23, 2018

Uh oh!

CheezItMan Oct 23, 2018

Uh oh!

CheezItMan Oct 23, 2018

Uh oh!

CheezItMan Oct 23, 2018

Uh oh!

CheezItMan Oct 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,5 @@
		class User < ApplicationRecord
		has_many :works

Conversation

unkcodesquick commented Oct 15, 2018

Media Ranker

Comprehension Questions

Uh oh!

CheezItMan commented Oct 23, 2018

Media Ranker

What We're Looking For

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants