Would it be possible to the ob_start etc the StaticGraph class eg in a private method strokeAsImageData? Would be good if it was possible. Otherwise we always need to make sure to apply this patch when updating the pchart library etc.

Hi Thomas, I am very unsure about this. Could you put some sample code and clarify me how we could get it done better without getting affected by an update from pchart's library?

I meant can you do something like

public function getPngImageData()
{
   return 'data:image/png;base64,'.base64_encode($this->strokeAsImageData());
}

private function strokeAsImageData() 
{
     ob_start();
     $this->pImage->stroke()
     $imagedata = ob_get_contents();
     ob_end_clean();
     return $imagedata;
}

or it might be possible to do but I am unsure about that

public function getPngImageData()
{
   return 'data:image/png;base64,'.base64_encode($this->getRenderedImage());
}

Would be no longer needed if doing Piwik::checkUserHasViewAccess($idSite) in the beginning

Let's consider we have

<?php
$idSiteArr = array(1,2,3,4,5);

Is it possible to pass an array of idSites like this outside the for loop?

for example,
Piwik::checkUserHasViewAccess($idSiteArr);

I had used like this "Piwik::checkUserHasViewAccess($idSite);" because I thought this function could check idSite one at a time. Could it take array as given above?

Plus what will happen if user doesn't have view access only for idSide=3. Will it output other 4 image graphs or just stop after outputting first 2?

"The diff is quite big indeed. I wonder if it was possible to make the code ..."

Hi Thomas, there're actually very few changes added to existing code. Not big as you said. Github diff tool may make it seem too big. What I did in ImageGraph.get function is just put a for loop around the statements inside the function and storing the image graph outputs to an array variable so that it could be used later. It's that simple. It doesn't alter the output behavior of any other function parameters supplied to this function. If you think that it really affects the existing behavior, then we may move it to new function. Otherwise, I don't find any reason to reuse the code and write the new functionality in a new function. Moreover, that would increase the program LOC drastically which is really not useful. Please apply this patch in you app and play around with this a few more times. You may find something more constructive. What do you think?

Update:
@mattab @tsteur
For your clarification, I used an excellent diff comparison tool like TextCompare to find the no. of lines of changes, it's just 50-55 new lines of code. Previous version is this: https://github.com/piwik/piwik/blob/2effc45935c6a0b4f5d3617d0557b0b6a026b96b/plugins/ImageGraph/API.php and mine is : https://github.com/piwik/piwik/blob/6767033cfe12d577e15203bdb6d10bf90bdfaa75/plugins/ImageGraph/API.php

Please view the changes minutely or use some other GUI tool which may give better picture of changes, it's nothing super special only an enhancement of functionality.

Piwik::checkUserHasViewAccess($idSite); you can do this at the very beginning, then you don't have to do it in a for loop. Followed by this you can do $idSiteArr = Site::getIdSitesFromIdSitesString($idSite) as this will exactly do what you want and you can remove all the custom logic re $idSiteArr. I think here https://github.com/piwik/piwik/pull/7660/files?w=1#diff-b96413000922b656fd09c8adc762c5a2R133 you can remove line 133-149 as this method will take care of this.

Yes the diff is smaller now

This should be a private method. Otherwise it will be added to the API documentation as an API