Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debug Toolbar error not found tpl error and fix #2275

Closed
demortx opened this issue Sep 27, 2019 · 23 comments
Closed

Debug Toolbar error not found tpl error and fix #2275

demortx opened this issue Sep 27, 2019 · 23 comments

Comments

@demortx
Copy link

demortx commented Sep 27, 2019

Describe the bug
NOT FOUND (/var/www/www-root/data/www/site.com/system/Debug/Toolbar/Views/_database.tpl) -> real name _database.tpl.php

Fix
_database.tpl.php copy and rename _database.tpl and etc files

CodeIgniter 4 version
4.0.0-rc.2

Affected module(s)
Which package or class is the bug in, if known.

CRITICAL - 2019-09-27 11:33:34 --> Invalid file: {0}
#0 /var/www/www-root/data/www/site.com/system/View/Parser.php(156): CodeIgniter\Exceptions\FrameworkException::forInvalidFile('') -> NOT FOUND (/var/www/www-root/data/www/site.com/system/Debug/Toolbar/Views/_database.tpl) -> real name _database.tpl.php
#1 /var/www/www-root/data/www/site.com/system/Debug/Toolbar/Views/toolbar.tpl.php(139): CodeIgniter\View\Parser->render('_files.tpl')
#2 /var/www/www-root/data/www/site.com/system/Debug/Toolbar.php(495): include('/var/www/www-ro...')
#3 /var/www/www-root/data/www/site.com/system/Debug/Toolbar.php(453): CodeIgniter\Debug\Toolbar->format(Array, 'html')
#4 /var/www/www-root/data/www/site.com/app/Config/Events.php(41): CodeIgniter\Debug\Toolbar->respond()
#5 [internal function]: CodeIgniter\Events\Events::Config{closure}()
#6 /var/www/www-root/data/www/site.com/system/Events/Events.php(187): call_user_func(Object(Closure))
#7 /var/www/www-root/data/www/site.com/system/CodeIgniter.php(226): CodeIgniter\Events\Events::trigger('pre_system')
#8 /var/www/www-root/data/www/site.com/public_html/index.php(45): CodeIgniter\CodeIgniter->run()
#9 {main}

Context

  • OS: [e.g. Windows 99]
  • Web server [apache2handler]
  • PHP version [7.3.0]
@dayrui
Copy link

dayrui commented Sep 27, 2019

/system/View/Parser.php
error, bug

`$fileExt = pathinfo($view, PATHINFO_EXTENSION);
$view = empty($fileExt) ? $view . '.php' : $view; // allow Views as .html, .tpl, etc (from CI3)

	// Was it cached?
	if (isset($options['cache']))
	{
		$cacheName = $options['cache_name'] ?? str_replace('.php', '', $view);

		if ($output = cache($cacheName))
		{
			$this->logPerformance($start, microtime(true), $view);
			return $output;
		}
	}

	$file = $this->viewPath . $view;`

@dayrui
Copy link

dayrui commented Sep 27, 2019

fix
$view = $view . '.php'; $file = $this->viewPath . $view;

@MGatner
Copy link
Member

MGatner commented Sep 27, 2019

I noticed this today as well. @dayrui I'm not sure where your fix ix supposed to go - could you send over a PR? Or clarify? Did you mean to add before line 146:

$view = $view . '.php';

So the updated version is:

	$view = $view . '.php';
	$file = $this->viewPath . $view;

I think that doesn't fix the problem of allowing other extensions, just compound extensions.

@MGatner
Copy link
Member

MGatner commented Sep 27, 2019

I think changing the file extensions in system/Debug/Toolbar/Views from .tpl.php to just .tpl might be the way to go, now that we support non-PHP files.

@MGatner
Copy link
Member

MGatner commented Sep 27, 2019

That works for everything except toolbar.tpl.php which appears to be included separately and needs the compound extension.

@InsiteFX
Copy link
Contributor

I' am also getting an Error

@MGatner
Copy link
Member

MGatner commented Sep 28, 2019

@jim-parry We might need to consider a hotfix or the Toolbar isn’t going to work until RC.3. Could you or @lonnieezell check out #2280 and #2281 and decide on an approach, then we can at least get dev patched?

@jim-parry
Copy link
Contributor

My preference would be to have them have just a ".tpl" file extension. That would let us eliminate a phpunit filter and keep them out of the testing more cleanly.

However, I don't where these are all used. and I would be surprised if it's just a matter of renaming them. I did a search in my IDE, and found Commands/Sessions/CreateMigration and Debug/Toolbar. Is that all that uses them?

If we are thinking a hotfix, the cleanest would be to have all the changes in the one PR, which would then be based on master, not develop.

@jim-parry
Copy link
Contributor

Something niggly that might be impacting the toolbar: Debug/Toolbar/Views/toolbarloader.js.php has messed up quoting, lines 78-79

@MGatner
Copy link
Member

MGatner commented Sep 28, 2019

That's what #2280 is. It passed all tests, FWIW. The template in CreateMigration is unrelated to the toolbar, so I think changing it would be good but not as part of this potential hotfix. I can re-submit #2280 off master.

@michalsn
Copy link
Member

In CreateMigration we're using full file name with extension, so it's all good.

@jim-parry
Copy link
Contributor

The tests don't break, but there is no Debug/ToolbarTest, so we wouldn't know :(
CreateMigration references a different template, not the toolbar ones, so we are good there (for unit testing) I would think that renamikng that template would make sense later, for consistency.

toolbar.tpl.php is referenced in Toolbar::495. That would need to be changed as part of #2280.

@jim-parry
Copy link
Contributor

It sounds like we are on the same page, and online at the same time. Weird, but effective!

@MGatner
Copy link
Member

MGatner commented Sep 28, 2019

toolbar.tpl.php is actually a PHP file and is included directly rather than passed through the parser. I'm not sure why it has the .tpl extension but I'd feel more comfortable leaving it out of this for now.

Re: tests I have been using the toolbar with renamed templates in my dev environment and can confirm that all collectors work and the toolbar displays.

@MGatner
Copy link
Member

MGatner commented Sep 28, 2019

Okay #2283 sent - does nothing except rename the actual templates for collector views. PR is for master, not sure how you want to handle that and if to release.

@jim-parry
Copy link
Contributor

This would affect the codeigniter4, appstarter and framework distributions.
I will merge & update the releases.

@lonnieezell
Copy link
Member

lonnieezell commented Sep 28, 2019 via email

@jim-parry
Copy link
Contributor

@lonnieezell See #2283 - Matthew explained that in the "fix"

@MGatner
Copy link
Member

MGatner commented Sep 28, 2019

@michalsn has #2281 which took the approach of pointing the code back to the files. I went with renaming the files to match the code, mostly because they are actually template files and not PHP code.

@jim-parry
Copy link
Contributor

@lonnieezell I need to hotfix the framework distro as is. Does is make sense to include this change, and redo all three distros and the codebase repo? If so, is it ok to just re-release them, or should be do an "rc.2b"?

@lonnieezell
Copy link
Member

lonnieezell commented Sep 28, 2019 via email

@jim-parry
Copy link
Contributor

Agreed. I had to do that before too.
coedigniter4 and framework distros affected.
will post & tweet

@MGatner
Copy link
Member

MGatner commented Sep 28, 2019

Resolved by 8ba099e

@MGatner MGatner closed this as completed Sep 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants