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

Review #36

Open
wants to merge 353 commits into
base: review
Choose a base branch
from
Open

Review #36

wants to merge 353 commits into from

Conversation

stevecass
Copy link

No description provided.

belongs_to :response

validates :user, uniqueness: { scope: :response,
message: "User can only vote on this response once."}
Copy link
Author

Choose a reason for hiding this comment

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

Nice use of scope

@stevecass
Copy link
Author

Tests and seeding are failing - it looks like a migration to add a "caption" column to questions might be missing? Or have you edited an old migration then re-run the db:create db:migrate steps? Questions table has no caption column but both seeds and specs expect it to exist.

def destroy
@question.destroy

redirect_to questions_url, notice: 'Question was successfully destroyed.'
Copy link
Author

Choose a reason for hiding this comment

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

Handle failure here, even if you think it very unlikely. You are telling the user the question was destroyed. What if the delete failed for some reason?

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.

4 participants