-
Notifications
You must be signed in to change notification settings - Fork 7
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
ENH Update folders + namespaces, remove unused class #74
ENH Update folders + namespaces, remove unused class #74
Conversation
cd953d4
to
4fd81e9
Compare
4fd81e9
to
b8afcdd
Compare
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.
Causes a fatal error if there are LoginSessions
existing already before applying this as it's processing them in the middleware: [Emergency] Uncaught SilverStripe\ORM\ValidationException: Object is of class 'SilverStripe\SessionManager\Model\LoginSession' which doesn't exist - you need to change the ClassName before you can write it
.
Might be okay if no one is using the module yet.
After removing LoginSessions
from the database, I'm getting this error: [Emergency] Uncaught Error: Call to undefined method SilverStripe\Security\RememberLoginHash::setLogoutAcrossDevices()
. Not sure why this method is gone.
Seems like it should now be RememberLoginHash::config()->update('logout_across_devices', false);
@bergice you probably need to run The new API was added specifically so we did not run |
|
||
// TODO: move this to Controllers folder and extend Controller instead of LeftAndMain | ||
class LoginSessionController extends LeftAndMain | ||
class LoginSessionController extends Controller |
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.
@emteknetnz @bergice Changing this class from LeftAndMain
to Controller
completely nukes the functionality. I understand this is a big PR, but you should at least try to run these things before merging them.
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'll fix it through #67
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.
Sorry, it was my screw up. I change something else on my set up ... just ignore me.
Issue #42