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

Return False for is_flagged(anonymous_user) #13

Merged
merged 5 commits into from Oct 4, 2022
Merged

Return False for is_flagged(anonymous_user) #13

merged 5 commits into from Oct 4, 2022

Conversation

bioworkflows
Copy link
Contributor

explained in #12

siteflags/models.py Outdated Show resolved Hide resolved
@idlesign
Copy link
Owner

Thank you. There's a note though.

@coveralls
Copy link

coveralls commented Sep 30, 2022

Coverage Status

Coverage increased (+0.07%) to 95.0% when pulling c8b9472 on bioworkflows:master into 04cbf97 on idlesign:master.

@idlesign
Copy link
Owner

A test case would be nice too.

@BoPeng
Copy link
Contributor

BoPeng commented Sep 30, 2022

I did not add a test because pytest-django does not have a fixture directly for anonymous user. Do you have an easy way to create an anonymous user?

I guess it is easiest just create an AnonymousUser instance from django.contrib.auth.models.

@bioworkflows
Copy link
Contributor Author

Used is_anonymous and added a test.

@idlesign
Copy link
Owner

idlesign commented Oct 2, 2022

I did not add a test because pytest-django does not have a fixture directly for anonymous user.

But this project doesn't use pytest-django.
pytest-djangoapp has user_create fixture which accepts anonymous param: https://pytest-djangoapp.readthedocs.io/en/latest/fixtures_users.html#pytest_djangoapp.fixtures.users.user_create

@BoPeng
Copy link
Contributor

BoPeng commented Oct 3, 2022

Revised to use user_create(anaonymous=True). This PR should be ready to merge.

@idlesign idlesign merged commit f42ed75 into idlesign:master Oct 4, 2022
@idlesign
Copy link
Owner

idlesign commented Oct 4, 2022

Thank you. Merged.

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

Successfully merging this pull request may close these issues.

None yet

4 participants