search

rapila / cms-base

The rapila cms’ internals. Please file all issues here if they’re not directly related to a plugin or the sample site.
http://www.rapi.la
3 stars 1 forks source link

replaceIdentifierMultiple context does not function correctly #171

Closed juergmessmer closed 10 years ago

juergmessmer commented 10 years ago

The dashboard shows a admin_sidebar now.

When I check the code in AdminManager::doAdmin() where the sidebar_content is replaced, the $mSidebarContent is null, but the admin_sidebar element is added to the main template anyway.

I guess this issue is related to the recent refactoring of the Template class.

sabberworm commented 10 years ago

Can you add two unit tests (one for replaceIdentifier and one for replaceIdentifierMultiple) where the content is null? I’ll fix the template code.

juergmessmer commented 10 years ago

Should it not be a TemplateIdentifierContextTest test?

sabberworm commented 10 years ago

I don’t really care for the method names and the classes they’re in as long as they’re at least somewhat logical.

juergmessmer commented 10 years ago

I made a test method called "testReplaceIdentifierMultipleNullWithContext". (Sorry: how do I run the unit tests :-[ )

sabberworm commented 10 years ago

./base/scripts/run-tests.sh

sabberworm commented 10 years ago

You said (correctly) that this should be a TemplateIdentifierContextTest. So why then do you not put the method in that class? Also, I said to add two unit tests (one with multiple, one without). You only added one.