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

Redirect source to /lookup if visiting /generate and already logged in #1165

Merged
merged 1 commit into from
Nov 17, 2015

Conversation

pwplus
Copy link
Contributor

@pwplus pwplus commented Oct 30, 2015

Add ability to log out at /logout.

Add test to ensure that /generate handles users who are already logged in appropriately

Improve test for generate when already logged in

Add log out button to source_templates/base.html

This is the exact same thing as this PR, but squashed and rebased to the current source since mine was 2 months outdated: #1080
I ended up getting the job at the company I interviewed for and had to get managerial approval before I could continue working on this project. Luckily, I got managerial approval!

# page, or inform them that they're logged in.
session.pop('logged_in', None)
if logged_in():
flash("You were redirected from {} because you are already logged in.".format(url_for('generate')), "notification")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a better message here would be "You were redirected because you are already logged in. If you want to create a new account, you should log out first".

@pwplus
Copy link
Contributor Author

pwplus commented Nov 9, 2015

I intend to write these tests in my free time over the next week.

# page, or inform them that they're logged in.
session.pop('logged_in', None)
if logged_in():
flash("You were redirected because you are already logged in. If you want\
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, for long strings, I prefer to use Python's line continuation behavior, which merges multiple quoted strings together automatically. Like this:

flash("You were redirected because you are already logged in. If you want"
      "to create a new account, you should log out first.", "notification")

@garrettr
Copy link
Contributor

@pwplus I added some more line comments. If you need help writing the additional tests, let me know!

@pwplus pwplus force-pushed the source_redirect branch 3 times, most recently from 69d5753 to 7d29887 Compare November 15, 2015 21:06
When a source is already logged in, clicking the back button could log
them out and generate a new codename. This could cause the source to lose
their original codename if it was not memorized yet. This commit will
redirect the source to the look page if they accidentally go back to the
generate page when logged in. It also adds an explicit logout button when
the source is logged in. This adds both confidence and professionalism to
the appearance.
@garrettr
Copy link
Contributor

Ah, I missed that you'd implemented the functional tests because you rebased. Lgtm!

garrettr added a commit that referenced this pull request Nov 17, 2015
Redirect source to /lookup if visiting /generate and already logged in
@garrettr garrettr merged commit 96d06a7 into freedomofpress:develop Nov 17, 2015
@redshiftzero redshiftzero mentioned this pull request Feb 7, 2017
14 tasks
@redshiftzero redshiftzero added this to the 0.3.11 milestone Feb 13, 2017
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.

3 participants