4 replies to this topic

[adamkiss]

Another error

Following code:

	$page->images->removeAll();
	$page->images->add($commitsGraphSrc);
	$page->images->add($languagesGraphSrc);
	$page->save();

	$page->images->eq(0)->rename('commits-graph-'.substr(date('now'),0,4).'.png');
	$page->images->eq(1)->rename('languages-graph-'.substr(date('now'),0,4).'.png');
	$page->save();

should delete all images if any, add two new images, save the page, and then remove the images to something sensible from the google chart URL (because $..GraphSrc is google chart api URL).

And the delete fails horribly with following:

[b]Fatal error[/b]:  Exception: Invalid type to Pageimages::remove(item) (in /Users/adam/Sites/this.is.secret/wire/core/Pagefiles.php line 182)


#0 /Users/adam/Sites/this.is.secret/wire/core/Array.php(653): Pagefiles->remove('commits-graph-1...')
#1 /Users/adam/Sites/this.is.secret/site/templates/github.php(103): WireArray->removeAll()
#2 /Users/adam/Sites/this.is.secret/wire/core/TemplateFile.php(92): require('/Users/adam/Sit...')
#3 [internal function]: TemplateFile->___render()
#4 /Users/adam/Sites/this.is.secret/wire/core/Wire.php(267): call_user_func_array(Array, Array)
#5 /Users/adam/Sites/this.is.secret/wire/core/Wire.php(229): Wire->runHooks('render', Array)
#6 /Users/adam/Sites/this.is.secret/wire/modules/PageRender.module(236): Wire->__call('render', Array)
#7 /Users/adam/Sites/this.is.secret/wire/modules/PageRender.module(236): TemplateFile->render()
#8 [internal function]: PageRender->___renderPage(Object(HookEvent))
#9 /Users/adam/Sites/this.is.secr... in <b>/Users/adam/Sites/this.is.secret/index.php on line <b>203</b>

Also, if it;s something, I'm not totally sure why calling $this->remove() in the Array does Pageimages::remove... or why the $item in chekcing isValidItem is string and not pageimage.

[adamkiss]

I replaced

  $page->images->removeAll();

with

foreach ($page->images as $pi){
  $page->images->remove($pi);
}

And it works. But shouldn't removeAll() work the same way? Did I fuck up something on my install?

[ryan]

And it works. But shouldn't removeAll() work the same way? Did I fuck up something on my install?

I'm not sure why that would work when removeAll doesn't because the foreach you posted is identical to the one used by removeAll. But that's not the issue here, instead it's this:

Files aren't actually removed or copied until you call $page->save(); that's what ultimately commits the change to the file system. So I think you'd solve this one by adding a $page->save(); immediately after your removeAll (before adding the other images).

The reason for this is that we don't want permanent changes (like deleting files) made to pages until we're certain those changes are actually going to be saved. Otherwise temporary runtime manipulations to something like $page->images wouldn't be possible, because you'd be copying or deleting files around without ever having that committed to the database (resulting in data corruption, and lots of untracked files).

[adamkiss]

But it isn't working?

This works:

foreach ($page->images as $pi){
  $page->images->remove($pi);
}
$page->images->add($commitsGraphSrc);
$page->images->add($languagesGraphSrc);
$page->save();

This doesn't (throws the aforementioned exception)

$page->images->removeAll();
$page->save();
$page->images->add($commitsGraphSrc);
$page->images->add($languagesGraphSrc);
$page->save();

Edit: I thought that loading up the generated images from google chart is the problem; I removed the gchart images, uploaded one other file via administration and fired up the code with removeAll(); - same thing happened. Exception.

Ot fails on 'argument instanceof Pageimage', when argument is 'string'. I don't know why.

[ryan]

Adam, you are right about this one. Tried it myself and got the same exception you did. Thanks for finding it -- It's a bug in the Pagesfiles::remove function that was preventing it from working with keys. Just fixed it in the latest commit.