kolibri-discuss team mailing list archive

[Frode Danielsen <frode@xxxxxxx>]

Review: Needs Fixing
No true showstoppers, but I noticed a few things. Let's start with the new mapping and testing code:

- core/DefaultActionMapper:57 is supposed to trim '/' from the beginning of the URI, but doesn't specify '/' to ltrim(), so only whitespace is trimmed.
- core/DefaultActionMapper:141 has a type specific check against null, but $previousPart is initialized with '' which !== null. Incidentally I think it'll have the same end result all in all, but by "accident". Seems inconsistent?
- specs/KolibriContext:75 Shouldn't models be allowed to exist directly within models/? And thus we need a check for $mainDir == 'models' as well?

Then there's a few minor things in the updated interceptors:

- ModelInterceptor: The private method convertType is not used anymore. Either remove or use in setProperty?
- AuthInterceptor:98 What if loginUri is defined with GET params already?

And for extra brownie points, fix these minor things in the example application:

- examples/wishlist/actions/items/ItemsDel:11 Comment needs an update (+ class comment)
- examples/wishlist/views/index.xsl:32 could reuse $have variable as in line 23

-- 
https://code.launchpad.net/~asteinlein/kolibri/nested-routes-mapper/+merge/6966
Your team Kolibri Discuss is subscribed to branch lp:kolibri.