-
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
User stories 1 and 2 completed (temporary solution to user story 4) #237
base: main
Are you sure you want to change the base?
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 |
---|---|---|
@@ -1,9 +1,26 @@ | ||
require 'sinatra/base' | ||
require 'sinatra/reloader' | ||
require_relative 'lib/peep' | ||
|
||
class Chitter < Sinatra::Base | ||
configure :development do | ||
register Sinatra::Reloader | ||
end | ||
|
||
get '/test' do | ||
'Test page' | ||
end | ||
|
||
get '/' do | ||
@all_peeps = Peep.all | ||
erb(:home) | ||
end | ||
|
||
post '/' do | ||
Peep.create(params[:message]) | ||
redirect("/") | ||
end | ||
|
||
|
||
run! if app_file == $0 | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
require 'pg' | ||
|
||
class Peep | ||
def self.all | ||
if ENV['ENVIRONMENT'] == 'test' | ||
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. There could be a method for this, in order to avoid repeating code! 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. Good observation! If you hadn't commented on it yourself, I would have 😁 |
||
connection = PG.connect(dbname: 'chitter_test') | ||
else | ||
connection = PG.connect(dbname: 'chitter') | ||
end | ||
|
||
result = connection.exec "SELECT * FROM peeps" | ||
return result.map{ |elem| elem["message"]}.reverse | ||
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. You can actually use the database query to sort the peeps in the right order and save the extra reversal step: https://www.postgresql.org/docs/current/queries-order.html |
||
end | ||
|
||
def self.create(peep) | ||
if ENV['ENVIRONMENT'] == 'test' | ||
connection = PG.connect(dbname: 'chitter_test') | ||
else | ||
connection = PG.connect(dbname: 'chitter') | ||
end | ||
|
||
connection.exec "INSERT INTO peeps (message) VALUES('#{peep}')" | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
feature 'Creating a new peep' 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. Test coverage is good, but the types of input (try invalids too?) aren't being tested. Only one type of input is being tested here... 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. That's a good point. To make your application more robust, you'd want to make sure you caught invalid inputs and test that too. In the context of this challenge, it's a good enough feature test though :) |
||
scenario 'A user can write and send new Peeps' do | ||
|
||
visit('/') | ||
fill_in('message', with: "This is a new Peep!") | ||
click_button('Submit') | ||
|
||
expect(page).to have_content "This is a new Peep!" | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
require 'pg' | ||
require 'peep' | ||
|
||
feature 'Viewing Peeps' do | ||
scenario 'A user can see Peeps' do | ||
connection = PG.connect(dbname: 'chitter_test') | ||
|
||
# Add the test data | ||
Peep.create('New Peep 1!') | ||
Peep.create('Yoyoyo!') | ||
Peep.create('http://www.google.com') | ||
|
||
visit('/') | ||
|
||
expect(page).to have_content "New Peep 1!" | ||
expect(page).to have_content "Yoyoyo!" | ||
expect(page).to have_content "http://www.google.com" | ||
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. Always include a new line at the end of a code file! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
require 'pg' | ||
require 'peep' | ||
|
||
describe '.all' do | ||
it 'returns all peeps in reverse chronological order' do | ||
connection = PG.connect(dbname: 'chitter_test') | ||
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. Do you actually need this line? |
||
|
||
# Add the test data | ||
Peep.create('New Peep 1!') | ||
Peep.create('Yoyoyo!') | ||
Peep.create('http://www.google.com') | ||
|
||
peeps = Peep.all | ||
|
||
expect(peeps).to include('New Peep 1!') | ||
expect(peeps).to include('Yoyoyo!') | ||
expect(peeps).to include('http://www.google.com') | ||
Comment on lines
+15
to
+17
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. As you're using |
||
end | ||
end | ||
|
||
describe '.create' do | ||
it 'adds peeps to the database' do | ||
connection = PG.connect(dbname: 'chitter_test') | ||
Peep.create("Peep peep!") | ||
peeps = Peep.all | ||
expect(peeps).to include('Peep peep!') | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<div style="text-align: center"> | ||
<h1>Chitter:</h1> | ||
|
||
<form action="/" method="post"> | ||
<input name="message" type="text" placeholder="Write a new Peep..."/> | ||
<button value="Submit" type="submit">Peep!</button> | ||
</form> | ||
|
||
<p><%= @all_peeps.join('/n') %></p> | ||
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 wouldn't work - ('/n') would just show up as a string. Maybe "/n" (full quotation marks) would work? 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. What's the HTML equivalent of a line break? That's the thing to research. |
||
</div> |
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.
It's always a good idea to remove test code from code you submit for code review (anywhere, not just at Makers). Just keeps things clean and easy to follow.