Skip to content
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

BUG Fixing logout issues with steps #12

Closed
wants to merge 1 commit into from
Closed

BUG Fixing logout issues with steps #12

wants to merge 1 commit into from

Conversation

halkyon
Copy link

@halkyon halkyon commented Nov 28, 2013

Firstly, stepIAmNotLoggedIn() would previously just kill the test
session, but this has the side effect of losing the current testsession
temporary database, so this causes steps of logging out to fail the
rest of the steps in a scenario.

Secondly, if we're logging a user in with stepILogInWith(), make
sure they are logged out first, otherwise "You don't have permission"
message will appear, but there is no form elements available to log
in with in this case. Logging out first by visiting Security/login
fixes this.

Firstly, stepIAmNotLoggedIn() would previously just kill the test
session, but this has the side effect of losing the current testsession
temporary database, so this causes steps of logging out to fail the
rest of the steps in a scenario.

Secondly, if we're logging a user in with stepILogInWith(), make
sure they are logged out first, otherwise "You don't have permission"
message will appear, but there is no form elements available to log
in with in this case. Logging out first by visiting Security/login
fixes this.
@srizzling
Copy link

I had this in working as well. :) except I defined it as I will be logged out of the CMS instead because the test is to specific to the CMS and will probably cause lingering issues later if different users are required to log in and out of a website

@halkyon
Copy link
Author

halkyon commented Nov 28, 2013

@srizzling When you log out there's no difference between logging out of the CMS and the website, everything goes through Security/login or Security/logout. Could you explain what you mean (and what you did) by logging out of the CMS instead of website?

@srizzling
Copy link

So, if I created a website which provided a service where users login it would still go through Security/login?

@halkyon
Copy link
Author

halkyon commented Nov 28, 2013

@srizzling Yes. So if you had a "frontend" members page, users would go through Security/login. Likewise, logging into /admin takes the user to Security/login where they login and are redirected back to /admin.

@srizzling
Copy link

Ah, my bad. Maybe, we could extract that string and put it in context class similar to how $c->getLoginUrl() works?

@halkyon
Copy link
Author

halkyon commented Nov 28, 2013

@srizzling Could do, although Security class currently has no set_logout_url() functionality to match set_login_url() at the moment, so it wouldn't really make much sense to have getLogoutUrl() and setLogoutUrl() in Behat until that's been implemented.

halkyon pushed a commit that referenced this pull request Nov 28, 2013
stepIAmNotLoggedIn() would previously just kill the test
session, but this has the side effect of losing the current testsession
temporary database, so this causes steps of logging out to fail the
rest of the steps in a scenario.

See #12
@chillu
Copy link
Member

chillu commented Nov 28, 2013

I've pushed the setIAmNotLoggedIn() bit with 8141ced.
Requesting Security/logout every time the login step is executed will cause too much overheads, its a step for pretty much every CMS scenario. For now I've added a "Given I am not logged in" above the case where it relies on being logged out. Improvement suggestion: Check for existence of the login form in that step, and redirect to Security/logout only when required.

@chillu chillu closed this Nov 28, 2013
@halkyon halkyon deleted the login_changes branch November 29, 2013 00:32
jeffreyguo pushed a commit to jeffreyguo/silverstripe-behat-extension that referenced this pull request Jan 12, 2016
Fixing hardcoded queries that are MySQL specific.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants