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

Miranda Frudd - chitter challenge (unfinished) #235

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

MirandaFrudd
Copy link

Your name

Miranda Frudd

User stories

Please list which user stories you've implemented (delete the ones that don't apply).

  • User story 1: "I want to see all the messages (peeps) in a browser"

README checklist

Does your README contains instructions for

  • how to install,
  • how to run,
  • and how to test your code?

-> yes but I've mainly used what was already given to us

Here is a pill that can help you write a great README!

Copy link
Contributor

@siellsiell siellsiell left a comment

Choose a reason for hiding this comment

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

Hey Miranda,

I know you felt some confusion about how to connect an app to a database, but you were actually really close to connecting it all up -- all the ingredients are there. I left a few comments with some pointers.

Let me know if you have any questions! You can reply to my comments on this PR or DM me.

Simo

fill_in 'Message', with: 'First Message'
click_on 'Submit'
visit '/messages'
expect(page).to have_content "First Message"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good example of a feature test, well done!

# context "so that I can create a new message" do
# it "#create adds a new message" do
# message = Message.create(message: 'Another message')
# expect(message.message).to eq "Another message"
Copy link
Contributor

Choose a reason for hiding this comment

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

You were really close! One way to check that create has really added the message to the database would be to call Message.all after calling create and check that your message is now part of the result set.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you! I think I understand how the class variables can be used to query the database now. I think I was trying to add class objects to the database but that's not right - I needed to gather params from a form and add them to a row of the database table using an SQL query within my class

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct!

app.rb Outdated
end

post '/messages' do
# Message.create(message: params[:message])
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you were thinking of creating the message in your POST route above (the commented out bit), which is good. Where would the right place be to retrieve it the message again so that you can display it to the user? You already have all the methods you need to achieve that in your Message class!

Copy link
Author

Choose a reason for hiding this comment

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

I should have used Message.all to display the messages from the db table in the get route

Copy link
Contributor

Choose a reason for hiding this comment

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

Exactly! Nice one.

Comment on lines +25 to +29
if ENV['ENVIRONMENT'] == 'test'
connection = PG.connect(dbname: 'chitter_test')
else
connection = PG.connect(dbname: 'chitter')
end
Copy link
Contributor

Choose a reason for hiding this comment

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

You're using this same piece of code in more than one place in this file. How might you get rid of this duplication?

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