Intermesh / groupoffice

Group Office groupware and CRM
https://www.group-office.com
Other
175 stars 46 forks source link

Spreadsheet modifications #1155

Open michalcharvat opened 1 week ago

michalcharvat commented 1 week ago

1) AbstractConverter: public final function exportToBlob(Query $entities, array $params = []): Blob change to public function exportToBlob(Query $entities, array $params = []): Blob Reason is simple - if you have to use existing functionality except simple logic from exportToBlob method you have to copy paste almost whole logic from Spreadsheet / AbstractConverter. Another option could be new method called processExport which will be public but not final so method exportToBlob will stay as is 2) Spreadsheet: finishExport - I would like to split logic from this method to new method called colorizeHeaders. New method will be called in exportToBlob before finishExport and it gives you more flexibility such as create multisheet exports.

/**
 * @return void
 */
protected function colorizeHeaders(): void
{
    switch ($this->extension) {
        case 'html':
        case 'csv':
            break;
        default:
            $headerStyle = [
                'font' => [
                    'bold' => true,
                    'color' => ['argb' => 'ffffff']
                ],
                'fill' => [
                    // SOLID FILL
                    'fillType' => Fill::FILL_SOLID,
                    'color' => ['argb' => '0277bd']
                ]
            ];
            foreach ($this->spreadsheet->getActiveSheet()->getColumnIterator() as $col) {
                $style = $this->spreadsheet->getActiveSheet()->getStyle($col->getColumnIndex() . "1");
                $style->applyFromArray($headerStyle);

                $colDim = $this->spreadsheet->getActiveSheet()->getColumnDimension($col->getColumnIndex());
                $colDim->setAutoSize(true);
            }
            break;
    }
}

There will be new dummy colorizeHeaders method in AbstractConverter. FinishExport will just write export to file

protected function finishExport(): Blob
{
    switch ($this->extension) {
        case 'html':
            fwrite($this->fp, "</table></body></html>");
            break;
        case 'csv':

            break;
        default:
            $writer = new Xlsx($this->spreadsheet);
            $writer->setPreCalculateFormulas(false);
            $writer->save($this->tempFile->getPath());
            break;
    }

    $cls = $this->entityClass;
    $blob = Blob::fromTmp($this->tempFile);
    $blob->name = $cls::entityType()->getName() . "-" . date('Y-m-d-H:i:s') . '.' . $this->getFileExtension();
    if (!$blob->save()) {
        throw new SaveException($blob);
    }

    return $blob;
}

If you agree I will prepare PR

mschering commented 5 days ago
  1. Can't you put all the logic in initExport(), exportEntity() and finishExport()? Can you give me an example of what's not possible using these 3 functions?
  2. It's fine to put the colorize headers logic in a separate function. But I don't like it to be in AbstractConvertor. That file is also for other things then spreadsheets. Think Icalendar, vcard etc. colorizeheaders makes no sense there. Why is a multisheet export not possible if you leave it there?
michalcharvat commented 4 days ago

1) Nope you cant if you dont create standard export because you cant avoid use exportEntity method. I have to prepare data for this custom export before I process them and processing is a bit complex. Basically I skip exportEntity method. Currently I have to copy paste everything from the AbstractConverter and Spreadsheet just to use similar logic - it works well for HTML and XLSX (I didnt tested it with CSV). Sure it is probably much safer because class depends on core but if you add there conditiions to check if converter uses AbstractConverter I am in trouble. 2) My bad you can really have it in Spreadsheet class. You need call that method during the processing data, in other case only latest sheet will be colorised

mschering commented 4 days ago
  1. You just want to remove "final"? Go ahead I'm ok with that.
  2. If you can make it a protected method in Spreadsheet then go ahead.