Raise an exception if database connection is not established before using acts_as_authentic #290

Merged
1 commit merged into from Jan 6, 2012

Conversation

Projects
None yet
4 participants

This would have helped us resolve an issue that took quite a while to debug because we didn't receive a meaningful exception message.

We are using Authlogic in a Sinatra based app and were including the model that uses acts_as_authentic before we established the database connection. Currently, instead of raising an exception the acts_as_authentic method just returns without error. A few lines later when we called:

if user.valid_password?(password)

which resulted in the exception:

undefined method `valid_password?' for #<User:0x007fd29dd269b0>

since the model wasn't initialized with the Authlogic modules.

ghost pushed a commit that referenced this pull request Jan 6, 2012

Merge pull request #290 from brianviveiros/master
Raise an exception if database connection is not established before using acts_as_authentic

@ghost ghost merged commit 309187b into binarylogic:master Jan 6, 2012

Contributor

james2m commented Jul 11, 2012

This is a poorly thought through change and should not have made it into Authlogic. The code contains assumptions that cause other problems.

Firstly it assumes that there is no database connection if Authlogic::ActsAsAuthentic#db_setup? returns false and secondly it assumes that your database has been created and migrations run.

Because it makes those assumptions migrations no longer run.

james2m added a commit to james2m/authlogic that referenced this pull request Jul 12, 2012

Fixes #318 and fixes #322 broken migrations caused by #290.
Signed-off-by: James McCarthy <james2mccarthy@gmail.com>

maletor commented Jul 17, 2012

Agree with @james2m

+1. in the meantime i'm rescuing the exception raised when calling acts_as_authentic to allow migrations to complete.

binarylogic added a commit that referenced this pull request Oct 23, 2012

Merge pull request #323 from james2m/fix-migrations
Fix broken migrations caused by #290

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment