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.

Sign up for GitHub

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

Jump to bottom

reverted and fixed current_auth booleans #460

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

reverted and fixed current_auth booleans #460

wants to merge 2 commits into from

Conversation

iambibhas
Copy link
Contributor 

if g.user:                   -   if current_auth.not_anonymous
if not g.user:               -   if current_auth.is_anonymous
if g.user or g.anon_user:    -   if current_auth.is_authenticated
if g.anon_user:              -   if current_auth.is_authenticated and current_auth.is_anonymous

@iambibhas iambibhas requested a review from jace June 26, 2018 08:35
iambibhas
iambibhas commented Jun 26, 2018
View reviewed changes
hasjob/views/helper.py Outdated
@@ -128,7 +128,7 @@ def load_user_data(user):
g.anon_user = anon_user

# Prepare event session if it's not already present
if current_auth or g.anon_user and not g.esession:
if current_auth and not g.esession:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

because current_auth checks for g.user or g.anon_user.

Copy link
Member

Choose a reason for hiding this comment

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

This is only true if Hasjob is reporting g.anon_user to current_auth as an actor, which has not been handled yet in this PR. This was reserved for a future PR.

jace
jace requested changes Jun 27, 2018
View reviewed changes
hasjob/views/campaign.py
@@ -180,7 +180,7 @@ def action_new(self, **kwargs):
@route('views.csv')
def view_counts(self, **kwargs):
campaign = self.obj
timezone = current_auth.actor.timezone if current_auth else 'UTC'
timezone = current_auth.actor.timezone if current_auth.not_anonymous else 'UTC'
Copy link
Member

Choose a reason for hiding this comment

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

This one can be current_auth.actor.timezone if current_auth else 'UTC' provided the AnonUser object reports a timezone (which it should).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're not setting and using g.anon_user until we close that other PR that moved AnonUser to mouse interaction. So gonna leave it like this for now and fix it when I close that other PR.

hasjob/views/helper.py Outdated
@@ -128,7 +128,7 @@ def load_user_data(user):
g.anon_user = anon_user

# Prepare event session if it's not already present
if current_auth or g.anon_user and not g.esession:
if current_auth and not g.esession:
Copy link
Member

Choose a reason for hiding this comment

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

This is only true if Hasjob is reporting g.anon_user to current_auth as an actor, which has not been handled yet in this PR. This was reserved for a future PR.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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

@jace jace jace requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iambibhas @CLAassistant @jace