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

Clean view context when controller changes #799

Conversation

barthez
Copy link
Contributor

@barthez barthez commented May 5, 2017

Description

When we reassign controller it is the right thing to do to clear current view context stored in request store. Consider a case when view context was built before the request was made (usage of h helper in class level context). View context is built from default controller and has no information about the request. During first request Draper::ViewContext.current.controller will be different than Draper::ViewContext.controller.

Testing

Example Rails app. Use h.content_tag in class context of some decorator, make this class be eagerly loaded.

Perform some request that will execute some decorator to refer to e.g. request.env values that are set up by some middleware (e.g. warden manager).

See that env is missing that key.

With that fix, view_context is regenerated when a controller is already available.

To-Dos

  • tests
  • documentation

When we reassign controller it is the right thing to do to clear current
view context stored in request store. Consider a case when view context
was built before the request was made (usage of `h` helper in class level
context). View context is built from default controller and has no
information about the request. During first request
`Draper::ViewContext.current.controller` will be different than
`Draper::ViewContext.controller`.
@barthez
Copy link
Contributor Author

barthez commented May 5, 2017

Fix for failure can be found here: #800

Copy link
Collaborator

@codebycliff codebycliff left a comment

Choose a reason for hiding this comment

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

Thanks for this. I think the change makes a lot of sense.

@codebycliff codebycliff merged commit 387878f into drapergem:master May 8, 2017
codebycliff added a commit to codebycliff/draper that referenced this pull request May 8, 2017
codebycliff added a commit that referenced this pull request May 8, 2017
* Release 3.0.0

* Add breaking change #768 to changelog.

* Add recently merged changes to the changelog.

* Add configuration and custom default controller to the changelog.

* Add entry to changelog for pull request 720

* Add entry to changelog for Rails 5 API-only support.

* Add changelog entry for pull request #795.

* Add changelog entry for pull request #796.

* Add changelog entry for enumerable optimization.

* Update maintainer information in README.

* Add pull request #799 to changelog.

* Add compatability documentation to readme.
@barthez barthez deleted the clean-view-context-on-controller-change branch May 16, 2017 14:04
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