[TYPO3-core] RFC: #11019: User Setup Rewrite #4

Rupert Germann rupi at gmx.li
Wed May 6 20:02:37 CEST 2009


hi Steffen,

I had a look to your v2 patch and have some remarks:

+       protected function checkAccess($level) {
+               if ($level = 'admin') {
+                       return $GLOBALS['BE_USER']->isAdmin(); 
+               }
+       }

this will always be TRUE ;-)

and as Olly said: please move the $GLOBALS[...]-> changes to another patch.
Or do them at least consequently - and not like in this line:

<input type="submit" name="data[setValuesToDefault]" value="'
$LANG->getLL('setToStandard').'" onclick="return confirm(\''
$GLOBALS['LANG']->getLL('setToStandardQuestion').'\');" />'
 
greets
rupert




Steffen Kamper wrote:
> Hi olly,
> 
> first thanks for review. I made a new patch where i completed phpDok,
> added protected to new functions, renamed functionNameOptions to
> functionNameSelect as they all deliver complete selectbox. This is
> required as there is the possibility (used by language) to return a
> warning box instead of selct.
> 
> I removed former cleanups so the patch is more easy to read. I will take
> care of cleaning up rest of code after this goes through.
> 
> I removed LANG: from $TUS-array as we talked today it's not good to
> introduce a new key for language handling. So usage of "label" in TUS
> conf-array is same like TCA: LLL:EXT:... or plain text.
> 
> remark: There is a simplified access check. Now it only works with
> 'access' => 'admin'
> could be extended in future. It's possible to set access level for a
> complete tab or a single field.
> 
> vg Steffen
> 
> Oliver Hader schrieb:
>> Hi Steffen,
>> 
>> Steffen Kamper schrieb:
>>> Hi,
>>>
>>> This is SVN patch request.
>>>
>>> Type: Bugfix
>>>
>>> Branches: trunk
>>>
>>> BT reference: http://bugs.typo3.org/view.php?id=11019
>>>
>>> Benni already started top use a configuration array for user settings
>>> with a TCA-like array.
>>>
>>> This is the next step using a real configuration array in
>>> $GLOBALS['TUS'], where TUS stands for TYPO3 User Settings
>>>
>>> This allows to manipulate the render of settings without hook like you
>>> can do it with TCA and simplify it.
>> 
>> Some comments on reviewing:
>> * before using another abbreviation in a new global variable, I'd like
>> to a registry singleton that delivers the same configuration by e.g.
>> "$userSettings = t3lib_Registry::getUserSettings();"
>> * "LANG:" introduces a new key that does not really tell, where to get
>> the label from
>> * please use e.g. $backendUserData instead of $BE_USER_data
>> * instead of using $GLOBALS['TUS']['columns'][$fieldname] multiple times
>> in the same method creat a copy of that information to e.g.
>> $fieldSettings and use that one
>> * please use proper PHPdoc comments and protected/public definitions on
>> methods (e.g. "function simulateUserOptions()")
>> * using e.g. $GLOBAL['LANG'] is not required in global context
>> ! please keep functional changes separated from functional changes (e.g.
>> converting all $BE_USER to $GLOBALS['BE_USER'] would go to a separate
>> patch)
>> 
>> Thanks!
>> 
>> olly



More information about the TYPO3-team-core mailing list