-
Notifications
You must be signed in to change notification settings - Fork 235
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
Chitter-challenge #210
base: main
Are you sure you want to change the base?
Chitter-challenge #210
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ ruby '3.0.2' | |
|
||
gem 'pg' | ||
gem 'sinatra' | ||
gem 'sinatra-contrib' | ||
|
||
group :test do | ||
gem 'capybara' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,6 +84,7 @@ GEM | |
|
||
PLATFORMS | ||
x86_64-darwin-20 | ||
x86_64-darwin-21 | ||
|
||
DEPENDENCIES | ||
capybara | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,29 @@ | ||
require 'sinatra/base' | ||
require 'sinatra/reloader' | ||
require './lib/peep' | ||
|
||
class Chitter < Sinatra::Base | ||
configure :development do | ||
register Sinatra::Reloader | ||
end | ||
|
||
get '/test' do | ||
'Test page' | ||
end | ||
Comment on lines
10
to
12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's always a good idea to remove test code from code you submit for code review (anywhere, not just at Makers) |
||
|
||
get '/peeps' do | ||
@peeps = Peep.all | ||
erb :"peeps/index" | ||
end | ||
|
||
get '/peeps/new' do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice use of RESTful route names! |
||
erb :"peeps/new" | ||
end | ||
|
||
post '/peeps' do | ||
Peep.create(message: params[:message]) | ||
redirect '/peeps' | ||
end | ||
|
||
run! if app_file == $0 | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
require 'pg' | ||
|
||
class Peep | ||
|
||
attr_reader :id, :message | ||
|
||
def initialize(id:, message:) | ||
@id = id | ||
@message = message | ||
end | ||
|
||
def self.all | ||
if ENV['ENVIRONMENT'] == 'test' | ||
connection = PG.connect(dbname: 'chitter_test') | ||
else | ||
connection = PG.connect(dbname: 'chitter') | ||
end | ||
|
||
result = connection.exec("SELECT * FROM peeps;") | ||
result.map do |peep| | ||
Peep.new(id: peep['id'], message: peep['message']) | ||
end | ||
end | ||
|
||
def self.create(message:) | ||
if ENV['ENVIRONMENT'] == 'test' | ||
connection = PG.connect(dbname: 'chitter_test') | ||
else | ||
connection = PG.connect(dbname: 'chitter') | ||
end | ||
Comment on lines
+26
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This piece of code appears twice in this file. How might you remove this duplication? |
||
|
||
result = connection.exec("INSERT INTO peeps (message) VALUES('#{message}') RETURNING id, message") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line 32 could be open to SQL injection attack. |
||
Peep.new(id: result[0]['id'], message: result[0]['message']) | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Newline |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
require 'pg' | ||
|
||
def persisted_data(id:) | ||
connection = PG.connect(dbname: 'chitter_test') | ||
result = connection.query("SELECT * FROM peeps WHERE id = #{id};") | ||
result.first | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Newline |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
feature 'adding a new peep' do | ||
scenario 'a user can add a new peep' do | ||
visit('/peeps/new') | ||
fill_in('message', with: 'This is another peep') | ||
click_button('Submit') | ||
|
||
expect(page).to have_content 'This is another peep' | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Newline |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
require 'pg' | ||
|
||
feature 'Viewing peeps' do | ||
scenario 'A user can see all posted peeps' do | ||
Peep.create(message: 'This is a peep!') | ||
|
||
visit('/peeps') | ||
|
||
expect(page).to have_content 'This is a peep!' | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Newline |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
require 'peep' | ||
require 'database_helpers' | ||
|
||
describe Peep do | ||
|
||
describe '.all' do | ||
it "returns a list of peeps" do | ||
peep = Peep.create(message: 'This is a peep!') | ||
Peep.create(message: 'This is another peep') | ||
|
||
peeps = Peep.all | ||
|
||
expect(peeps.length).to eq 2 | ||
expect(peeps.first).to be_a Peep | ||
expect(peeps.first.id).to eq peep.id | ||
expect(peeps.first.message).to eq 'This is a peep!' | ||
Comment on lines
+13
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally you'd check that both results in the array are correct |
||
end | ||
end | ||
|
||
describe '.create' do | ||
it 'creates a new peep' do | ||
peep = Peep.create(message: 'This is a peep!') | ||
persisted = persisted_data(id: peep.id) | ||
|
||
expect(peep).to be_a Peep | ||
expect(peep.id).to eq persisted['id'] | ||
expect(peep.message).to eq 'This is a peep!' | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<ul> | ||
<% @peeps.each do |peep| %> | ||
<li class="peep" id="peep-<%= peep.id %>"> | ||
<%= peep.message %> | ||
</li> | ||
<% end %> | ||
</ul> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Newline |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
<form action="/peeps" method="post"> | ||
<input type="text" name="message" /> | ||
<input type="submit" name="Submit" /> | ||
</form> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Newline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 87 interesting, what does this do?