Skip to content
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

Ports -- Kate #45

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Ports -- Kate #45

wants to merge 14 commits into from

Conversation

goblineer
Copy link

Task List

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe in your own words what the Model is doing in Rails The Model is how the data comes into the app and the structure for how it's organized into a database.
Describe in your own words what the Controller is doing in Rails The controller contains all of the business logic in the app. It mediates between the view and the model and sends requests to the proper routes.
Describe in your own words what the View is doing in Rails The View dictates how the data and accompanying HMTL will be seen and perhaps manipulated by the user. It includes form views the index page and all of the visible wrappers (navbars, adornments, etc).
Describe an edge-case controller test you wrote
What is the purpose of using strong params? (i.e. the params method in the controller) Strong params prevent things from going ... off the rails.
How are Rails migrations related to Rails models? Migrations are done when structural changes are made to the databases, when the model has been updated.
Describe one area of Rails that are still unclear on TDD

@CheezItMan
Copy link

Task List

What We're Looking For

Feature Feedback
Baseline
Appropriate Git Usage with no extraneous files checked in You do need more commits for this project.
Answered comprehension questions A less flippant answer to strong params would be appreciated.
Successfully handles: Index, Show Yes, when I manually create the tasks
Index & Show tests pass No
Successfully handles: New, Create No, you are not setting some fields you've made as required.
New & Create tests pass No
Successfully handles: Edit, Update Check
Tests for Edit & Update test valid & invalid task ids No
Successfully handles: Destroy, Task Complete Destroy works, marking complete doesn't seem to
Tests for Destroy & Task Complete include tests for valid and invalid task ids No
Routes follow RESTful conventions Check
Uses named routes (like _path) Check
Overall Did you change your migrations after they were run? Your schema doesn't match the migrations and so I'm having a hard time getting it to run. Generally once you've made a migration, don't touch the file, make another to undo that change. You also did no testing. This isn't a good start to Rails. I'd like to go through things on this project with you to make sure you have a solid foundation for rails. I know you were out a few days, and we need to get this handled soon.

<button type="button" class="left btn-outline-success"><%= link_to "EDIT", edit_task_path(task[:id]) %></button>
<button type="button" class="right btn-outline-danger"><%= link_to "DELETE", task, method: :delete, data: { confirm: "Really, though? This can't be undone." }%></button></div>
<p><button type="button" class="btn btn-info"><%= link_to "Uncomplete", complete_task_path(task), method: :patch %></button></p>
</p>Completed <%= time_ago_in_words(task.completed_at)%> ago.</p>

Choose a reason for hiding this comment

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

Your Task model doesn't have a completed_at field. Instead I would simply make this line:

<p><%= task.completed ? "Completed": "Incomplete" %></p>


class Task < ApplicationRecord
def completed?
!completed_at.blank?

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 completed_at field, instead maybe just return completed.

end

def create
@task = Task.create task_params

Choose a reason for hiding this comment

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

I'm getting an error that ERROR: null value in column "completed_at" violates not-null constraint, this means you have a field completed_at that can't be nil, but you're not setting it in your strong params.

@@ -0,0 +1,5 @@
class AddDueDateToTasks < ActiveRecord::Migration[5.2]
def change
add_column :tasks, :due_date, :datetime

Choose a reason for hiding this comment

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

This migration doesn't seem to be in your schema.

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