[TYPO3-core] RFC: Bug #3903, Edit content doesn't work with TV

[Bernhard Kraft]

Dmitry Dulepov [typo3] wrote:

>> Solution:
>> Add a hook in t3lib_tcemain.php which allows an extension method to 
>> perform the check if it is valid to modify a specific table.
> 
> This is a good idea. Some time ago I thought about different solution: hook
 > into TCEforms somehow and tell user that he needs also to set page
 > modification rights for user. But your solution is better.


One change to my initial patch request would be that the hook should not 
reside in the method "checkRecordUpdateAccess" but only before the call 
to this method in "processDatamap" - at about line 734 (see the change 
in this line in the sent patch)

I think this would be cleaner - as we would not have to pass along the 
hookObjectsArr array containing the hook - classes.


>> I hope this is ok for you ...
> 
> Fine if it goes to 4.2 only.

Only trunk ... also my suggestion.



> There are some things in the patch that I would like to change or get comment from you first. Here is a piece of TCEmain:
> 
> ---------------
> 	$res = 0;
>  	if ($TCA[$table] && intval($id)>0)	{
> +		if (is_array($hookObjectsArr))	{
> +			foreach($hookObjectsArr as $hookObj)	{
> +				if (method_exists($hookObj, 'checkRecordUpdateAccess')) {
> +					$res = $hookObj->checkRecordUpdateAccess($table, $id, $data, $res, $this);
> +				}
> +			}
> +		}
> +		if ($res)	{
> +			return $res;
> +		}
>  		if (isset($this->recUpdateAccessCache[$table][$id]))	{	// If information is cached, return it
> ---------------
> 
> 1. Last line uses cache. Should hook be executed after the cache? Hooks may perform complex processing, there is no need for it to repeat. What do you think?
> 2. I think hook may return true/false/null. This is equivalent to PostFix'es OK/REJECT/DONTKNOW. So if hook returns null, we continue looping. If it returns false, we return false. If it returns true, we mark it and continue looping (because access is always restrictive: one forbids -> no access). I do not insist on this but it would be interesting to here responses about it.

The problem is that the hook can take "anything" into account when 
giving rights for editing or not - meaning you could probably check if 
the page-Type is set to "Advanced" and user is "editor3" and only then 
give access - so the result of the hook method can't get cached.

But in the hook method the first thing checked is if the edited table is 
"pages" - if this is not the case the hook will return instantly.

Only if the edited table is "pages" it will perform the check with the 
many "$res=0;break" to see if only "eType==ce" fields get modified.

I guess it would be possible to replace those "break"'s by "return"'s 
... so the method returns as soon as it notices that a field other than 
a FlexForm Field and other than "eType==ce" gets modified - this would 
speed things up.

And at least this hook will only get executed when TV is installed - so 
no impact on overall T3-core speed (except the new hook)


> 3. Should hook be called as last resort, when all others said "no access"? This would minimize calculation time in hooks if user already has edit rights.

This is a good idea in our case:

case #1: when access-rights should get granted if they would not 
normally (because editPage has been disallowed)

case #2: But it could also be the other case that somebody wants to deny 
access under special circumstances - even if the right has been given.

The best solution would be to have two hooks - one before (for case #2) 
and another below for our case (case #1)

but I agree to have only one below the current checks (case #1)


>> Next to the patch for the core there is a patch for the t3lib_tcemain 
>> hook of templa-voila. It adds the required method which performs the 
>> check by having a look at the posted data and if only "eType=ce" 
>> flexfields and no other fields get modified ... i know the code there 
>> looks a litte bit strange but I couldn't find a better solution. 
>> Pherhaps one of you has a better idea.
> 
> I will review this. In particular I want to find a better solution
 > than any "$res=0;break;" in the code. Nothing bad with it
 > technically, just makes function fat...

well ... like written above: I guess they could be replaces by:

return 0;



greets,
Bernhard