joomlatools / joomlatools-todo

Todo extension for Joomla build using Joomlatools Framework.
https://www.joomlatools.com/developer/framework/
GNU General Public License v3.0
7 stars 3 forks source link

Improve state based requests overrides #12

Open ercanozkaya opened 8 years ago

ercanozkaya commented 8 years ago

We are currently pushing internal states to the request in the dispatcher. Instead we should do that in the controller while using the mixed set of permissions.

Work is in branch: feature/17-request done by @amazeika

ercanozkaya commented 8 years ago

Discussion: Comment by @amazeika: @johanjanssens, @ercanozkaya The idea I was after didn't work. I ran into problems.

For starters, the dispatcher passes its request to the controller, so no setRequest call will be made for dispatched controller unless we do this in the constructor, which is not something we want to do.

Secondly we cannot reliably make overrides that rely on the model state (some permission methods do). In this case scenario I ran into infinite loops because the request was being asked for before the model was set (its value was still null). For example Editable behavior makes a getRequest call when its isSupported method is called ... at this point $this->_model is still null ... $this->getModel() returns a controller instance as a consequence ... etc, etc.

Anyway, what I was after cannot be done. Our best hope would be to do this kind of overrides before rendering, like I'm doing right now.

How can we improve this further?.

Comment by @johanjanssens : Cleanup the code and finished the implementation. These two lines where the lines I was looking for.

Comment by @amazeika : @johanjanssens Why are you doing this on _beforeBrowse only?. This would mean that todo items can be read even if they are un-published.

Comment by @johanjanssens : @arunasmazeika You are correct moving it to _beforeBrowse is wrong. Need to review this change again.