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

Remove mock request object from jet.utils.get_model_queryset #115

Merged
merged 14 commits into from
Sep 21, 2016
Merged

Remove mock request object from jet.utils.get_model_queryset #115

merged 14 commits into from
Sep 21, 2016

Conversation

darccio
Copy link
Contributor

@darccio darccio commented Sep 6, 2016

jet.utils.get_model_queryset created a mock request using django.test.client.RequestFactory while the real request object was available in the calling function jet.templatetags.jet_tags' context variable.

This causes problems when we access to request's members added by middlewares, like request.user or request.session.

@coveralls
Copy link

coveralls commented Sep 7, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.3%) to 54.251% when pulling bb9c353 on imdario:master into fea0704 on geex-arts:master.

4 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 54.251% when pulling bb9c353 on imdario:master into fea0704 on geex-arts:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 54.251% when pulling bb9c353 on imdario:master into fea0704 on geex-arts:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 54.251% when pulling bb9c353 on imdario:master into fea0704 on geex-arts:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 54.251% when pulling bb9c353 on imdario:master into fea0704 on geex-arts:master.

@coveralls
Copy link

coveralls commented Sep 7, 2016 •
edited
Loading

Coverage Status

Coverage increased (+0.3%) to 54.251% when pulling bb9c353 on imdario:master into fea0704 on geex-arts:master.

@hermanocabral
Copy link

Any reason why this is not being accepted? The mock request is breaking a lot of plugins that depend on request.user or request.session.

@mord4z
Copy link
Contributor

mord4z commented Sep 20, 2016

Please, accept this! The mock request breaking a lot things here.

@SalahAdDin
Copy link
Contributor

SalahAdDin commented Sep 20, 2016 •
edited
Loading

@f1nality Man!
Guys, @f1nality some times haven't any time for review an merge PR, he had five months for update this package, please wait.

While you can fork this pr and work with it :D

@f1nality f1nality changed the base branch from master to dev September 21, 2016 16:25
@f1nality f1nality merged commit de786b8 into geex-arts:dev Sep 21, 2016
@f1nality
Copy link
Contributor

f1nality commented Sep 21, 2016 •
edited
Loading

@imdario merged, thanks for PR!
The only thing made me confused was a one more dependency on request context processor. I tried to decrease number of request context processor usages during development to remove it finally out of project dependencies. But in this case I don't see any better solutions.

@SalahAdDin that time I had to rewrite the whole project to merge PRs, so yes, it took long, but not it is rewritten already

@f1nality f1nality mentioned this pull request Sep 21, 2016
Closed
@f1nality f1nality added this to the 1.0.2 milestone Sep 21, 2016
@zodman zodman mentioned this pull request Sep 21, 2016
Closed
@SalahAdDin
Copy link
Contributor

@f1nality Thanks man!

@darccio
Copy link
Contributor Author

darccio commented Sep 21, 2016

Thanks @f1nality!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0.2
Development

Successfully merging this pull request may close these issues.
6 participants
@darccio @coveralls @hermanocabral @mord4z @SalahAdDin @f1nality