Coding Challenge #14

pyreta · 2016-08-26T16:52:49Z

No description provided.

mecampbellsoup · 2016-08-26T18:39:18Z

README.md

+## Instructions
+  - bundle install
+  - npm install
+  - bundle exec rake db:reset to re-seed database


@pyreta the only diff in this PR should be new files you add to write the web server/web application; probably don't want to overwrite this README

@mecampbellsoup Had an error pushing with the existing license and README, so I removed it.

dannyc5 · 2016-08-26T19:37:15Z

frontend/actions/actions.js

@@ -0,0 +1,20 @@
+const AppDispatcher = require("../dispatcher/dispatcher");


I believe there's a way to set another lookup path in your webpack config so that you don't have to use relative paths in your require statements

In other words, instead of require('../dispatcher/dispatcher'), you could require('dispatcher/dispatcher')

Require also looks for dispatcher/index.js and dispatcher/package.json if it doesn't find dispatcher.js right away, so your require statement could just become require('dispatcher') if you rename dispatcher.js to index.js

@danielchangNYC Ah this is a great call Daniel. I believe I could useresolve.moduleDirectoriesthe way I resolved the extensions.

Even better, this webpack contributor recommends using resolve.root

@danielchangNYC Thanks for this tip! I updated the webpack config to resolve.root and removed all those ugly "../"'s. I also changed dispatcher.js to index.js to see if it defaulted to that file (which it did). This is definitely something I will use moving forward. Thanks again!

mecampbellsoup · 2016-08-29T18:26:12Z

db/migrate/20160825222150_create_banks.rb

+class CreateBanks < ActiveRecord::Migration
+  def change
+    create_table :banks do |t|
+      t.integer :routing_num, null:false


This should be a string not integer probably.

@mecampbellsoup This is corrected in a later migration.

mecampbellsoup · 2016-08-29T19:17:46Z

@pyreta I'd like you to work on an enhancement before we have any sort of phone call. Here goes:

Add a script or rake task (e.g. something akin to rake db:seed in Rails land) which goes to the URL containing the list of banks, parses it, and either stores those records in memory or writes them to the application's database. This way anyone can clone this repo, run your rake task, and use the application (i.e. after the rake task any requests for a specific bank should serve the bank's data as JSON)

mecampbellsoup · 2016-08-29T19:32:26Z

db/seeds.rb

@@ -0,0 +1,45 @@
+require 'http'
+


@pyreta nice work, you got the fixed width thing.

Overall comments: would love to see some specs for the behavior contained in this file. It would probably be easier to write those tests if you encapsulated the logic in seeds.rb into a class or classes.

So in seeds.rb you can just do something like BankDataParser.perform!, and the you'll have the actual (unit-tested) logic in lib/<your-app-name>/bank_data_parser.rb.

It also would be nice to see a separation of concerns, where the concerns of seeds.rb at the moment is twofold: fetching the data, parsing it by row/entry in the sheet, and persisting those records to the database. The last two can probably be combined into a single concern, but the former (fetching) should standalone.

lib/<app-name>/actions/fetch_bank_data.rb

lib/<app-name>/actions/parse_bank_data.rb

You can name them whatever you want though just providing an example.

pyreta · 2016-08-29T19:34:22Z

@mecampbellsoup just runrake db:seed and the database should be populated with data that is fetched and parsed. You may need to run rake db:create first, and bundle install. See seeds.rb.

dannyc5 · 2016-08-29T19:49:20Z

app/views/banks/show.json.jbuilder

+  json.extract! @bank, :zipcode, :zipcode_ext
+  json.street_address @bank.address
+end
+json.last_change_date @bank.last_change_date.to_s[0...10]


Since this is the view layer, I might opt for creating a more direct interface for the bank object instead of manipulating data here.

dannyc5 · 2016-08-30T23:55:20Z

app/views/banks/show.json.jbuilder

+  :data_view_code,
+  :office_code,
+  :last_change_date,
+  :inst_status__code


Much cleaner!

mecampbellsoup · 2016-08-31T13:43:37Z

@pyreta how's the 'fetch, parse and store' rake task functionality coming along?

Also, can you make this deployable to Heroku? Feel free to ask questions of us in this PR.

pyreta · 2016-08-31T14:05:24Z

@mecampbellsoup Are you referring to the changes I made in the seed file? I divided the functionality of the fetching and parsing into separate classes (which tests are written for in the spec folder). The seed file uses these classes when you run rake db:seed. See seeds.rb. Is there something else I'm missing?
Yes I could make this deployable to Heroku.

pyreta · 2016-08-31T16:42:46Z

@mecampbellsoup Should be Deployable on Heroku now...

mecampbellsoup · 2016-08-31T16:56:25Z

@pyreta sorry I meant deployed, ha. Once deployed just tell us the public URL.

pyreta · 2016-08-31T17:38:24Z

@mecampbellsoup http://bankfetcher.herokuapp.com/

mecampbellsoup · 2016-08-31T18:27:47Z

@pyreta getting some 500 errors: https://www.dropbox.com/s/c25act8ytdc8ehr/Screenshot%202016-08-31%2014.27.30.png?dl=0

pyreta · 2016-08-31T19:16:17Z

@mecampbellsoup As of right now there is no error handling for invalid routing numbers.

pyreta · 2016-09-01T00:54:27Z

@mecampbellsoup Added some error handling to account for invalid routing numbers. Should be all good now.

pyreta added 3 commits August 26, 2016 10:03

Basic Search with Flux

fd44166

Remove readme and license

58c5e58

Update README

ea895aa

mecampbellsoup reviewed Aug 26, 2016
View reviewed changes

pyreta added 3 commits August 26, 2016 14:56

Fix int conversion

81e755e

Add routing number display field

ffbdad7

Remove commented code

7e3a87b

dannyc5 reviewed Aug 26, 2016
View reviewed changes

Resolve root for relative directories

26da22d

mecampbellsoup reviewed Aug 29, 2016
View reviewed changes

dannyc5 reviewed Aug 29, 2016
View reviewed changes

pyreta added 2 commits August 29, 2016 22:23

Create BankDataParser class and abstract out Bank Display component

b64beda

Add specs and change jbuilder/DB formatting

9c41e40

dannyc5 reviewed Aug 30, 2016
View reviewed changes

app/views/banks/show.json.jbuilder

:data_view_code,

:office_code,

:last_change_date,

:inst_status__code

Copy link

dannyc5 Aug 30, 2016

Choose a reason for hiding this comment

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

Much cleaner!

pyreta added 2 commits August 31, 2016 12:12

Make Heroku Deployable

c9a3470

Add Production Gems

76c8854

Handle no bank error

0bc38cb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Coding Challenge #14

Coding Challenge #14

pyreta commented Aug 26, 2016

mecampbellsoup Aug 26, 2016

pyreta Aug 26, 2016

dannyc5 Aug 26, 2016

pyreta Aug 26, 2016

dannyc5 Aug 26, 2016

pyreta Aug 29, 2016

mecampbellsoup Aug 29, 2016

pyreta Aug 29, 2016

mecampbellsoup commented Aug 29, 2016

mecampbellsoup Aug 29, 2016

mecampbellsoup Aug 29, 2016

pyreta commented Aug 29, 2016

dannyc5 Aug 29, 2016

dannyc5 Aug 30, 2016

mecampbellsoup commented Aug 31, 2016

pyreta commented Aug 31, 2016

pyreta commented Aug 31, 2016 •

edited

Loading

mecampbellsoup commented Aug 31, 2016

pyreta commented Aug 31, 2016

mecampbellsoup commented Aug 31, 2016

pyreta commented Aug 31, 2016

pyreta commented Sep 1, 2016

		@@ -0,0 +1,20 @@
		const AppDispatcher = require("../dispatcher/dispatcher");

Coding Challenge #14

Are you sure you want to change the base?

Coding Challenge #14

Conversation

pyreta commented Aug 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mecampbellsoup commented Aug 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pyreta commented Aug 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mecampbellsoup commented Aug 31, 2016

pyreta commented Aug 31, 2016

pyreta commented Aug 31, 2016 • edited Loading

mecampbellsoup commented Aug 31, 2016

pyreta commented Aug 31, 2016

mecampbellsoup commented Aug 31, 2016

pyreta commented Aug 31, 2016

pyreta commented Sep 1, 2016

pyreta commented Aug 31, 2016 •

edited

Loading