-
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
Joe M - Chitter #234
base: main
Are you sure you want to change the base?
Joe M - Chitter #234
Changes from all commits
f805bc8
ebefd7a
ef8c59a
ee1c196
c09557f
c503e40
c22c57f
2c312a2
71508fb
298ef53
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 'webrick' | ||
|
||
group :test do | ||
gem 'capybara' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,18 +34,25 @@ You should see 1 passing test. | |
|
||
## User stories | ||
|
||
|
||
``` | ||
As a Maker | ||
So that I can see what people are doing | ||
I want to see all the messages (peeps) | ||
in a browser | ||
``` | ||
|
||
|
||
![alt text](story1diagram.png "See all messages diagram") | ||
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 really like the diagrams, they're super helpful for visualising the data flow in your program 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 agree! It's always a good idea to diagram things out. One thing I would note is that in these diagrams you've included both That's not quite how the MVC model is meant to be understood. The database isn't really a part of the MVC model, the MVC model only covers the code you write:
|
||
|
||
``` | ||
As a Maker | ||
So that I can let people know what I am doing | ||
I want to post a message (peep) to chitter | ||
``` | ||
# Second Story diagram | ||
|
||
![alt text](story2diagram.png "Post a message (peep) diagram") | ||
|
||
``` | ||
As a Maker | ||
|
@@ -54,6 +61,13 @@ I want to see the date the message was posted | |
``` | ||
(Hint the database table will need to change to store the date too) | ||
|
||
# To do - Date of message | ||
|
||
* to faciltate time of peep add column to the databases(dev and test) | ||
* Update Readme with how to set up database tables to include date and time column | ||
* add date and time to tests | ||
* working through the tests, amend the app.rb and new peep files to capture the date and time of peeps | ||
|
||
``` | ||
As a Maker | ||
So that I can easily see the latest peeps | ||
|
@@ -64,3 +78,6 @@ As a Maker | |
So that I can find relevant peeps | ||
I want to filter on a specific keyword | ||
``` | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,34 @@ | ||
require 'sinatra/base' | ||
require './lib/peeps' | ||
require 'time' | ||
|
||
class Chitter < Sinatra::Base | ||
get '/test' do | ||
'Test page' | ||
end | ||
Comment on lines
6
to
8
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). It keeps everything nice and clean :) |
||
|
||
get '/' do | ||
@peeps = Peeps.all | ||
p @peeps | ||
erb :'index' | ||
end | ||
|
||
get '/new' do | ||
erb :'new' | ||
end | ||
|
||
post '/' do | ||
message = params['message'] | ||
p params['message'] | ||
time = Time.now.httpdate | ||
connection = PG.connect(dbname: 'chitter') | ||
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. One next step here would be to refactor this code into your Peeps class, similar to this step from the Bookmark Manager challenge: https://github.com/makersacademy/course/blob/main/apprenticeships_bookmark_manager/walkthroughs/10.md#refactoring 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. Yes, that would be a good next step. It would allow you to test this code in a unit test just like you tested the |
||
connection.exec("INSERT INTO peeps (message, posted) VALUES('#{message}', '#{time}')") | ||
redirect '/' | ||
end | ||
|
||
|
||
|
||
|
||
|
||
run! if app_file == $0 | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
CREATE TABLE peeps(id SERIAL PRIMARY KEY, message VARCHAR(60)); | ||
CREATE TABLE peeps(id SERIAL PRIMARY KEY, message VARCHAR(60), posted VARCHAR(60)); | ||
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'm not sure I 100% follow their reasoning, but Makers advise adding updates to the database table as a separate migration file: https://github.com/makersacademy/course/blob/main/apprenticeships_bookmark_manager/walkthroughs/11.md#updating-the-bookmarks-table Also, depending on how you record the date / time, you could use a different SQL data type for the posted column: https://www.w3schools.com/sql/sql_datatypes.asp 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. Some good links there @olliefreeman94 The reason for a separate migration file is that in a real system, you would want to be able to run the migration separately from the initial creation of the table, as a DB will typically throw an error if you try to create a table that doesn't exist. If all the migrations are in the same file, they will never run because the error will happen before we even get to them. 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. When we want to add a field to a new table, we typically want to use an |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
|
||
require 'pg' | ||
require 'time' | ||
|
||
class Peeps | ||
def self.all | ||
if ENV['ENVIRONMENT'] == 'test' | ||
connection = PG.connect(dbname: 'chitter_test') | ||
else | ||
connection = PG.connect(dbname: 'chitter') | ||
end | ||
|
||
result = connection.exec('SELECT message, posted FROM peeps') | ||
# binding.irb | ||
result.map { |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 part of the Bookmark Manager walkthrough outlines wrapping the database data in an instance of the class: https://github.com/makersacademy/course/blob/main/apprenticeships_bookmark_manager/walkthroughs/11.md#wrapping-returned-data |
||
|
||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
# in spec/features/creating_bookmarks_spec.rb | ||
|
||
feature 'Post a new peep' do | ||
scenario 'A user can post a new peep to chitter' do | ||
visit('/new') | ||
fill_in('message', with: 'hey peeps. just a peep to say I am posting a new peep. so peep this!') | ||
click_button('post your peep') | ||
|
||
expect(page).to have_content 'hey peeps. just a peep to say I am posting a new peep. so peep this!' | ||
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. Good feature test! |
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
feature 'Viewing peeps' do | ||
scenario 'A user can see peeps' do | ||
connection = PG.connect(dbname: 'chitter_test') | ||
|
||
# Add the test data | ||
connection.exec("INSERT INTO peeps VALUES(1, 'Just completed SQL Zoo. It was fun to do!');") | ||
connection.exec("INSERT INTO peeps VALUES(2, 'Testing adding chitter messages');") | ||
connection.exec("INSERT INTO peeps VALUES(3, 'testing chitter.');") | ||
|
||
visit('/') | ||
expect(page).to have_content "Just completed SQL Zoo. It was fun to 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. 💥 |
||
expect(page).to have_content "Testing adding chitter messages" | ||
expect(page).to have_content "testing chitter." | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
# in spec/peeps_spec.rb | ||
|
||
require 'peeps' | ||
|
||
describe Peeps do | ||
describe '.all' do | ||
it 'returns all peeps' do | ||
connection = PG.connect(dbname: 'chitter_test') | ||
|
||
# Add the test data | ||
connection.exec("INSERT INTO peeps VALUES(1, 'Just completed SQL Zoo. It was fun to do!');") | ||
connection.exec("INSERT INTO peeps VALUES(2, 'Testing adding chitter messages');") | ||
connection.exec("INSERT INTO peeps VALUES(3, 'testing chitter.');") | ||
|
||
|
||
peeps = Peeps.all | ||
|
||
expect(peeps).to include "Just completed SQL Zoo. It was fun to do!" | ||
expect(peeps).to include "Testing adding chitter messages" | ||
expect(peeps).to include "testing chitter." | ||
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 and thorough! |
||
end | ||
end | ||
end | ||
|
||
|
||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<ul> | ||
<% @peeps.each do |peep| peep['message'] %> | ||
<li><%= peep %></li> | ||
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 part of the Bookmark Manager walkthrough outlines updating the view to show different properties of the Peeps object: https://github.com/makersacademy/course/blob/main/apprenticeships_bookmark_manager/walkthroughs/11.md#showing-title-in-the-bookmarks-view |
||
<% end %> | ||
</ul> | ||
|
||
<img src='/chitter.png' /> | ||
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 logo is a really nice addition! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<!-- inside views/new.erb --> | ||
|
||
<form action = "/" method = "post" /> | ||
<input type="text" name="message" /> | ||
<input type="submit" value="post your peep" /> | ||
</form> | ||
|
||
<img src='/chitter.png' /> |
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.
I found this framework really helpful when writing my README: https://github.com/makersacademy/course/blob/main/pills/readmes.md