-
Notifications
You must be signed in to change notification settings - Fork 234
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 - Eleanor CS #218
base: main
Are you sure you want to change the base?
Chitter - Eleanor CS #218
Changes from all commits
e6b5085
2514c07
f656b78
15d08c7
70326bb
1a296dd
d311841
2c3999e
d4f0a50
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,39 @@ | ||
require 'sinatra/base' | ||
require './lib/peeps' | ||
|
||
class Chitter < Sinatra::Base | ||
get '/test' do | ||
'Test page' | ||
end | ||
|
||
get '/' 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. these are generally good RESTful routes – nice work! You could consider ways of combining the routes for all the requests that show multiple Peeps but, then again, you could also make the argument that such an approach would break SRP. |
||
'Welcome to Chitter!' | ||
erb :index | ||
end | ||
|
||
get '/peeps' do | ||
@peeps = Peeps.all | ||
erb :peeps | ||
end | ||
|
||
get '/peeps/new' do | ||
erb :new_peep | ||
end | ||
|
||
post '/peeps' do | ||
Peeps.create(message: params[:message], entry_date: params[:entry_date]) | ||
redirect '/peeps' | ||
end | ||
|
||
get '/peeps/reverse' do | ||
@peeps = Peeps.reverse_chronology | ||
erb :peeps_reverse | ||
end | ||
|
||
get '/peeps/filter' do | ||
@filtered_peeps = Peeps.filter(params[:filter]) | ||
erb :peeps_filter | ||
end | ||
|
||
run! if app_file == $0 | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
ALTER TABLE peeps ADD entry_date date; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
require 'pg' | ||
|
||
class Peeps | ||
|
||
attr_reader :id, :message, :entry_date, :filter | ||
|
||
def initialize(id:, message:, entry_date:) | ||
@id = id | ||
@message = message | ||
@entry_date = entry_date | ||
end | ||
|
||
def self.open_connection | ||
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 possible to refactor this into its own class and have an always on db connection while the app is running. There's a walkthrough on how to do this in Ex.15 on the Bookmark task. |
||
if ENV['ENVIRONMENT'] == 'test' | ||
connection = PG.connect(dbname: 'chitter_test') | ||
else | ||
connection = PG.connect(dbname: 'chitter') | ||
end | ||
end | ||
|
||
def self.row_to_peep(row) | ||
Peeps.new(id: row['id'], message: row['message'], entry_date: row['entry_date']) | ||
end | ||
|
||
def self.all | ||
connection = open_connection | ||
|
||
result = connection.exec("SELECT * FROM peeps;") | ||
result.map do |peep| | ||
row_to_peep(peep) | ||
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 line being refactored into its own method is nice |
||
end | ||
end | ||
|
||
def self.create(message:, entry_date:) | ||
connection = open_connection | ||
|
||
result = connection.exec_params("INSERT INTO peeps (message, entry_date) VALUES($1, $2) RETURNING id, message, entry_date;", [message, entry_date]) | ||
row_to_peep(result[0]) | ||
end | ||
|
||
def self.reverse_chronology | ||
connection = open_connection | ||
|
||
result = connection.exec("SELECT * FROM peeps ORDER BY entry_date DESC;") | ||
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. Oh interesting, I did this with a .reverse method on the array. 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. Which approach is more computationally efficient? How would you test it? |
||
result.map do |peep| | ||
row_to_peep(peep) | ||
end | ||
end | ||
|
||
def self.filter(filter) | ||
connection = open_connection | ||
result = connection.exec("SELECT * FROM peeps WHERE lower(message) LIKE '%#{filter.downcase}%';") | ||
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. Using downcase on everything is better than what I did 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 a nice approach, I agree! |
||
result.map do |peep| | ||
row_to_peep(peep) | ||
end | ||
end | ||
end |
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
feature 'Creating peeps' do | ||
scenario 'user can add peeps and dates' do | ||
setup_test_database | ||
|
||
Peeps.create(message: 'Test peep!', entry_date: "2021-03-18") | ||
Peeps.create(message: 'Second test peep!', entry_date: "2021-02-15") | ||
|
||
visit ('/peeps') | ||
|
||
expect(page).to have_content('Test peep!') | ||
expect(page).to have_content('Second test peep!') | ||
expect(page).to have_content("2021-03-18") | ||
expect(page).to have_content("2021-02-15") | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
feature 'Filtering peeps' do | ||
scenario 'user can filter peeps by keyword' do | ||
setup_test_database | ||
|
||
Peeps.create(message: 'Test peep!', entry_date: "2021-03-18") | ||
Peeps.create(message: 'Second test peep!', entry_date: "2021-02-15") | ||
|
||
visit ('/peeps') | ||
fill_in('filter', with: 'peep') | ||
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. Given that both test peeps contain the word 'peep' maybe another word could have been used to test the filter function, such as 'Second'? Even better would be 'second' as then you test your downcase code as well. 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 points! |
||
click_button "Filter" | ||
|
||
expect(page).to have_content("Test peep!") | ||
expect(page).to have_content("Second test peep!") | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
feature 'Reverse chronological peeps' do | ||
scenario 'user can see most recent peeps first' do | ||
setup_test_database | ||
|
||
Peeps.create(message: 'Test peep!', entry_date: "2021-03-18") | ||
Peeps.create(message: 'Second test peep!', entry_date: "2021-02-15") | ||
|
||
visit ('/peeps') | ||
|
||
click_button "View Newest First" | ||
|
||
page.body.index("2021-03-18").should < page.body.index("2021-02-15") | ||
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. I have never seen page.body.index before. Nice. 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. Neither have I :) |
||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
feature 'Viewing peeps' do | ||
scenario 'user can view all peeps' do | ||
setup_test_database | ||
|
||
Peeps.create(message: 'Test peep!', entry_date: "2021-03-18") | ||
Peeps.create(message: 'Second test peep!', entry_date: "2021-02-15") | ||
|
||
visit ('/peeps') | ||
|
||
expect(page).to have_content('Test peep!') | ||
expect(page).to have_content('Second test peep!') | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
require 'peeps' | ||
|
||
describe '.all' do | ||
it 'returns a list of peeps' do | ||
setup_test_database | ||
|
||
chitter = Peeps.create(message: "Test peep!", entry_date: "2021-02-15") | ||
Peeps.create(message: "Second test peep!", entry_date: "2021-02-13") | ||
Peeps.create(message: "Third test peep!", entry_date: "2021-10-16") | ||
|
||
chitter = Peeps.all | ||
|
||
expect(chitter.length).to eq 3 | ||
expect(chitter.first).to be_a Peeps | ||
expect(chitter.first.message).to eq "Test peep!" | ||
expect(chitter.first.entry_date).to eq "2021-02-15" | ||
end | ||
end | ||
|
||
require 'database_helpers' | ||
|
||
describe '.create' do | ||
it 'creates a new peep' do | ||
chitter = Peeps.create(message: "Test peep!", entry_date: "2021-02-15") | ||
persisted_data = persisted_data(id: chitter.id) | ||
|
||
expect(chitter).to be_a Peeps | ||
expect(chitter.id).to eq persisted_data['id'] | ||
expect(chitter.message).to eq "Test peep!" | ||
expect(chitter.entry_date).to eq "2021-02-15" | ||
end | ||
end | ||
|
||
describe '.filter' do | ||
it 'filters peeps by keyword' do | ||
chitter = Peeps.create(message: "Test peep!", entry_date: "2021-02-15") | ||
Peeps.create(message: "Second test peep!", entry_date: "2021-02-13") | ||
Peeps.create(message: "Third say hello!", entry_date: "2021-10-16") | ||
|
||
filtered_results = Peeps.filter("Peep") | ||
expect(filtered_results[0].message).to eq "Test peep!" | ||
expect(filtered_results[1].message).to eq "Second test peep!" | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
<h1> Welcome to Chitter! </h1> | ||
<img src="https://gl.audubon.org/sites/default/files/styles/hero_mobile/public/aud_apa-2020_barn-swallow_a1-11258-1_nape_photo-xianwei-zeng.jpg?itok=aTU-t6nJ" alt="Chitter Birds"> | ||
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. Maybe this picture could link to your /peeps page? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
<form action="/peeps" method="post"> | ||
<input type="text" name="message" placeholder="Message" /> | ||
<input type="date" name="entry_date" /> | ||
<input type="submit" value="Submit" /> | ||
</form> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<h1> Welcome to Chitter! </h1> | ||
|
||
|
||
<% @peeps.each do |peep| %> | ||
<div class="Peeps" style="border: 3px solid aqua; width:fit-content; width:-webkit-fit-content; width:-moz-fit-content;"> | ||
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. The styling would be better placed in a css file I reckon, especially given that this code will repeat on every iteration of the loop. |
||
<q><%= peep.message %></q> | ||
<i><%= peep.entry_date %></i> | ||
</div> | ||
<% end %> | ||
|
||
<form action="/peeps/reverse" method="get"> | ||
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. Love that this is a button (I did mine the lazy way!) |
||
<input type="submit" value="View Newest First" /> | ||
</form> | ||
|
||
<form action="/peeps/filter" method="get"> | ||
<input type="text" name="filter" placeholder="Filter keyword" /> | ||
<input type="submit" value="Filter" /> | ||
</form> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<h1> Welcome to Chitter! </h1> | ||
|
||
<h2> Here are your filtered results: </h2> | ||
|
||
<% @filtered_peeps.each do |peep| %> | ||
<div class="Peeps" style="border: 3px solid aqua; width:fit-content; width:-webkit-fit-content; width:-moz-fit-content;"> | ||
<q><%= peep.message %></q> | ||
<i><%= peep.entry_date %></i> | ||
</div> | ||
<% end %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<h1> Welcome to Chitter! </h1> | ||
|
||
<% @peeps.each do |peep| %> | ||
<div class="Peeps" style="border: 3px solid aqua; width:fit-content; width:-webkit-fit-content; width:-moz-fit-content;"> | ||
<q><%= peep.message %></q> | ||
<i><%= peep.entry_date %></i> | ||
</div> | ||
<% end %> |
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.
You could probably delete this test route.