Fwd: [GrinnellPlans] User registration (#47)

4 views
Skip to first unread message

Ian Young

unread,
Nov 6, 2011, 12:38:50 AM11/6/11
to grinnellplan...@googlegroups.com
Hey folks,

Shitanshu submitted a nice-looking bunch of code for account registration on the new Rails project. If you'd like to take a look and observe or join in the code review, click through.

Ian

---------- Forwarded message ----------
From: Shitanshu Aggarwal <reply+i-2140399-cd83f35bf591...@reply.github.com>
Date: Thu, Nov 3, 2011 at 7:59 PM
Subject: [GrinnellPlans] User registration (#47)
To: Ian Young <ian.gr...@gmail.com>


Hey, I finally got around to finishing the tests for the user registration. I am pleased with what I have; it is well tested.

I didn't think changing the behavior at all because of our feature freeze. We could work on making user registration more secure in the future. I really don't like sending out passwords in plain text.

I ended up using RAILS testing framework instead of rspec because (1) I wasn't motivated to learn rspec, and (2) the two frameworks can coexist.

Let me know if you want me to change something. Particularly, I settled on a style where I display stuff in a view depending on what flag was set in the controller. I probably should be using something like flash notices.

You can merge this Pull Request by running:

 git pull https://github.com/aggarwal/GrinnellPlans user-registration

Or you can view, comment on it, or merge it online at:

 https://github.com/annaswims/GrinnellPlans/pull/47

-- Commit Summary --

* added a test file
* Merge remote-tracking branch 'upstream/master'
* adding registration form (Shitanshu)
* added partial registration code and modified the tentative account table
* added partial registration code and modified the tentative account table
* added email confirmation, and updated the model and fixture for tentative account to be in line with the migration
* added resend confirmation email
* added more view code, DRYfied some code, added the default route
* reverting broken schema
* running tentative_accounts migration
* creating account on email confirmation link
* link to registration page on home page; show plan of currently logged in user on login; moved new account creation code into accounts model; new plan object created when new account is created
* brought back the default tests directory, yay!
* moving token generation function to accounts model
* added some unit tests for accounts model
* adding functional tests for accounting controller
* adding an integration test placeholder
* added test for when tentative account already exists
* added test for successful email confirmation and account creation
* added test for when account already exists
* added test for when expired tentative account should be cleared
* added assertions on expected email notification on account creation
* added a unit test file for mailer
* added test for when we want the confirmation email resent
* finished functional tests for accounts_controller
* fixing tests
* abstracting out allowed email domains to the accounts model and enforcing it
* updated routes file to be rails3 compliant; commented out default route and added specific routes for accounts controller
* added an integration test for successful user registration
* abstracting out hard coded domain name and email address into application controller
* moving allowed email domains to application controller too. (need to find a better place for all these)
* moving configuration variables to a YML file
* removing all references to grinnell.edu from tests and using a fictitious domain instead

-- File Changes --

M Gemfile (9)
M Gemfile.lock (110)
M app/controllers/account_sessions_controller.rb (2)
A app/controllers/accounts_controller.rb (86)
M app/controllers/application_controller.rb (1)
M app/controllers/plans_controller.rb (7)
A app/mailers/notifier.rb (17)
M app/models/account.rb (32)
M app/models/tentative_account.rb (16)
M app/views/account_sessions/new.haml (2)
A app/views/accounts/_sent_confirmation_email.html.erb (5)
A app/views/accounts/confirm.html.erb (2)
A app/views/accounts/create.html.erb (11)
A app/views/accounts/new.erb (25)
A app/views/accounts/resend_confirmation_email.html.erb (1)
A app/views/notifier/confirm.text.erb (9)
A app/views/notifier/send_password.text.erb (10)
M config/application.rb (3)
A config/config.yml (14)
A config/initializers/load_config.rb (2)
M config/routes.rb (85)
A db/migrate/20110905085915_modify_tentative_accounts.rb (24)
M db/schema.rb (17)
M db/seed/development/tentative_accounts.csv (4)
A spec/mailers/notifier_spec.rb (5)
M spec/models/account_spec.rb (3)
A test/fixtures/accounts.yml (37)
A test/functional/accounts_controller_test.rb (132)
A test/integration/user_registration_test.rb (61)
A test/performance/browsing_test.rb (9)
A test/test_helper.rb (13)
A test/unit/account_test.rb (60)
A test/unit/notifier_test.rb (8)

-- Patch Links --

 https://github.com/annaswims/GrinnellPlans/pull/47.patch
 https://github.com/annaswims/GrinnellPlans/pull/47.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/annaswims/GrinnellPlans/pull/47

Reply all
Reply to author
Forward
0 new messages