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

Amy's Chitter #233

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Your name
# Amy O'Connor

Please write your full name here to make it easier to find your pull request.

Expand Down
5 changes: 4 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ GEM
mini_mime (1.1.1)
mustermann (1.1.1)
ruby2_keywords (~> 0.0.1)
nokogiri (1.12.3-arm64-darwin)
racc (~> 1.4)
nokogiri (1.12.3-x86_64-darwin)
racc (~> 1.4)
parallel (1.20.1)
Expand Down Expand Up @@ -83,6 +85,7 @@ GEM
nokogiri (~> 1.8)

PLATFORMS
arm64-darwin-21
x86_64-darwin-20

DEPENDENCIES
Expand All @@ -98,4 +101,4 @@ RUBY VERSION
ruby 3.0.2p107

BUNDLED WITH
2.2.26
2.3.17
18 changes: 16 additions & 2 deletions app.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
require 'sinatra/base'
require './lib/peep'

class Chitter < Sinatra::Base
get '/test' do
'Test page'
Comment on lines -4 to -5
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! It's always a good idea to remove test code from code you submit for code review (anywhere, not just at Makers). Make sure you remember to also remove code you added for debugging.

get '/' do
@peeps = Peep.all
erb :'index'
end

get '/new' do
erb :"new_peep"
end

post '/' do
peep = params['peep']
connection = PG.connect(dbname: 'chitter')
connection.exec("INSERT INTO peeps (message) VALUES('#{peep}')")
Comment on lines +16 to +17
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you already have aPeep class, it would make sense for this code to be part of that class. I see you already have a method in there called Peep.create, which would be a good place to put it.

redirect '/'
p "Peep sent!"
Copy link

Choose a reason for hiding this comment

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

everything clear and neatly laid out

Copy link

Choose a reason for hiding this comment

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

no repetitions either

end

run! if app_file == $0
Expand Down
5 changes: 5 additions & 0 deletions db/migrations/.last_run.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"result": {
"line": 100.0
}
}
Copy link

Choose a reason for hiding this comment

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

What does this do?

13 changes: 13 additions & 0 deletions lib/peep.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
require 'pg'

class Peep
def self.create

end

def self.all
connection = PG.connect(dbname: 'chitter')
result = connection.exec("SELECT * FROM peeps;")
result.map { |peep| peep['message'] }
end
Copy link

Choose a reason for hiding this comment

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

class follows the single responsibility principle

end
17 changes: 17 additions & 0 deletions spec/features/chitter_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
feature 'Viewing all peeps' do
scenario 'visiting the home page' do
visit('/')
peeps = Peep.all
expect(page).to have_content "This is a peep!"
end
end

feature 'Adding a peep' do
scenario 'Adding a peep to Chitter feed' do
visit('/new')
fill_in('peep', with: "It is a bit warm!")
click_button('Peep')
expect(page).to have_content "It is a bit warm!"
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice feature tests! It would be good to see a unit test for your Peep class as well so that you can test it in isolation.


Copy link

Choose a reason for hiding this comment

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

I think this should pass!

6 changes: 0 additions & 6 deletions spec/features/test_page_spec.rb

This file was deleted.

5 changes: 0 additions & 5 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@
setup_test_database
end

config.after(:suite) do
puts
puts "\e[33mHave you considered running rubocop? It will help you improve your code!\e[0m"
puts "\e[33mTry it now! Just run: rubocop\e[0m"
end
end

RSpec.configure do |config|
Expand Down
3 changes: 3 additions & 0 deletions views/index.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<% @peeps.each do |peep| %>
<%= peep %> <br>
<% end %>
4 changes: 4 additions & 0 deletions views/new_peep.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<form action="/" method="post">
<input type="test" name="peep" />
<input type="submit" value="Peep" />
</form>