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

Add AuthenticationMiddleware to El Proxito tests #6416

Merged
merged 1 commit into from Dec 2, 2019

Conversation

humitos
Copy link
Member

@humitos humitos commented Nov 27, 2019

This middleware is required to run the tests in Corporate because the allowed_user method uses request.user to make the decision.

I'm not sure if there is a better way to do this by overriding one of this setting in the corporate site. We have been skipping these kind of tests, but I think El Proxito is important in corporate and it's better to run these tests as well.

This middleware is required to run the tests in Corporate because the
`allowed_user` method uses `request.user` to make the decision.
@humitos humitos added the PR: work in progress Pull request is not ready for full review label Nov 27, 2019
@humitos
Copy link
Member Author

humitos commented Nov 27, 2019

This is also failing because

django.urls.exceptions.NoReverseMatch: Reverse for 'features' not found. 'features' is not a valid view function or pattern name.

We may want to use a variable for the urlconf as we do in other test cases. I need to check this more in deep.

@humitos humitos removed the PR: work in progress Pull request is not ready for full review label Dec 2, 2019
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

Seems reasonable. We should perhaps be doing a full set of default middleware, so it's closer to prod, but this is fine for now.

@humitos humitos merged commit 48d6a6b into master Dec 2, 2019
@humitos humitos deleted the humitos/proxito-s3-auth branch December 2, 2019 11:46
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

2 participants