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

Exporting Data #8

Open
willishq opened this issue Oct 15, 2018 · 4 comments
Open

Exporting Data #8

willishq opened this issue Oct 15, 2018 · 4 comments
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects

Comments

@willishq
Copy link
Collaborator

We should have the ability to export data. This seems pretty trivial to implement in reality. All we are doing is taking the existing grid and row, and instead of passing the data to be transformed into HTML, we're passing it to an exporter which can convert it to any format.

We should think about creating an interface for this, and providing different export options in a separate repo, while adding a grid method with available export options.

$grid->addExporter('csv', CsvExporter::class)
$grid->addExporter('pdf', PdfExporter::class)
$grid->addExporter('xls', XlsExporter::class)

etc.

@willishq willishq added enhancement New feature or request help wanted Extra attention is needed question Further information is requested labels Oct 15, 2018
@willishq willishq added this to To do in Query Grid via automation Oct 15, 2018
@maxalmonte14
Copy link
Member

maxalmonte14 commented Oct 24, 2018

I've been working the whole morning trying to make an implementation of this feature, so far I have a functional CSVExporter, I'll show you my implementation and you can add your own ideas to make a clear API.

Grid class

 /**
  * Holds a list of Exporter objects.
  *
  * @var Exporter[]
  */
  private $exporters;

 /**
  * Adds a new Exporter class to
  * the exporters array property.
  *
  * @param string $key
  * @param string $classname
  *
  * @return void
  */
  public function addExporter($key, $classname)
  {
      $this->exporters[$key] = $classname;
  }

 /**
  * Returns an Exporter instance.
  *
  * @param string $key
  *
  * @return \QueryGrid\QueryGrid\Contracts\Exporter
  */
  public function getExporter($key)
  {
      return new $this->exporters[$key]($this->getResult());
  }

Exporter interface

namespace QueryGrid\QueryGrid\Contracts;

interface Exporter
{
    /**
     * Converts a set of data to an
     * exportable format.
     *
     * @return mixed
     */
    public function export();
}

CSVExporter class

<?php

namespace QueryGrid\QueryGrid\Exporters;

use QueryGrid\QueryGrid\Contracts\Exporter;
use QueryGrid\QueryGrid\GridResult;

class CSVExporter implements Exporter
{
    /**
     * The set of data to be converted.
     *
     * @var \QueryGrid\QueryGrid\GridResult
     */
    private $gridResult;

    /**
     * Creates a new CSVExporter instance.
     *
     * @param \QueryGrid\QueryGrid\GridResult $gridResult
     */
    public function __construct(GridResult $gridResult)
    {
        $this->gridResult = $gridResult;
    }

    /**
     * Converts a GridResult object to an
     * exportable CSV set of data.
     *
     * @return string
     */
    public function export()
    {
        $header = implode(',', array_map(function ($column) {
            return $column['label'];
        }, $this->gridResult->getColumns()->toArray()));

        $body = implode("\n", array_map(function($row) {
            return implode(',', $row);
        }, $this->gridResult->getRows()->toArray()));

        return sprintf("%s\n%s", $header, $body);
    }
}

CSVExporterTest class

<?php

namespace Tests;

use Tests\Grid\TestCase;
use QueryGrid\QueryGrid\Exporters\CSVExporter;

class CSVExporterTest extends TestCase
{
    /** @test */
    public function gridCanBeExportedToCSV()
    {
        $grid = $this->itHasAGridInstance();

        $grid->addColumn('id', 'Id');

        $grid->addColumn('email', 'Email Address');

        $grid->addColumn('role', 'Role');

        $dummyData = [
            ['id' => 1, 'email' => 'andrew@example.com', 'role' => 'admin'],
            ['id' => 2, 'email' => 'maxalmonte14@example.com', 'role' => 'editor']
        ];

        $this->dataProvider->setValues($dummyData);

        $grid->addExporter('csv', CSVExporter::class);

        $results = $grid->getExporter('csv')->export();

        $expectedResults = <<<'EOD'
Id,Email Address,Role
1,andrew@example.com,admin
2,maxalmonte14@example.com,editor
EOD;

        $this->assertEquals($expectedResults, $results);
    }
}

Note that in the export method, in the CSVExporter class I'm not really operating over the file system, but formatting the GridResult, I don't know if the feature is supposed to be implemented this way.

There is a lot of things that can be done to enhance this implementation, at the moment I can think about the following:

1 - Use an ExporterCollection instead of a plain array in Grid class ($exporters property)
2 - Add more scenarios to the CSVExporterTest like failing parsings, etc.
3 - Split the responsibility of CSVExporter class (and other possible exporters) into two methods, parse and export, the export method wouldn't be supposed to parse data, only return it (maybe we need a Parser interface for this matter)
4 - Create an AbstractExporter class to wrap common behavior. At this point this is not really necessary but, could be if we change the implementation.

I can make a PR so you can test the implementation, but I think is better to discuss how it looks so far 😅

@willishq
Copy link
Collaborator Author

Hey, good work man, it all looks great.

I have a concern about keeping the exporters in the repo with the core query-grid code, I think each exporter should have its own repo, seperate, otherwise we have the unenviable task of having to do a release every time we create a new export class, which would be annoying.

However, the core grid class would still need the export methods, and it’d need testing with a dummy exporter.

@willishq
Copy link
Collaborator Author

Actually, I think the export method should accept the grid, not the constructor.

or if we go the parse / export route, the parse method should accept the grid.

Also... the standard way of dealing with the data is an export, maybe we should have a default implementation of the export called ArrayExporter or JsonExporter.

Could we use the collection?

    public function export()
    {

        $header = implode(',', $this->gridResult->getColumns()->map(function (Column $column) {
            return $column['label'];
        })->toArray());

        $body = implode("\n", $this->gridResult->getRows()->map(function($row) {
            return implode(',', $row);
        })->toArray());

        return sprintf("%s\n%s", $header, $body);
    }

For simplicity it might even be worth adding an implode method to the base collection so we can do something like this

$header = $this->gridResult->getColumns()->implode(',', function (Column $column) {
    return $column->getLabel();
});

I'm just thinking out loud here, I'd be interested to hear your thoughts!

@maxalmonte14
Copy link
Member

I like the idea of creating a separate repo for all the exporters, that would be the best way to go in my opinion.

Having an ArrayExporter and JsonExporter would be a good start, I think we should define them along with the CSVExporter 👌

I don't see the point of passing the Grid object to the Exporter::export method, actually that was my initial idea but I couldn't figure out a way to do it without duplication and redundancy, like having an export method into the Grid class and then calling the Exporter::export method from inside it, like this:

Grid class

public function export($key)
{
    return (new $this->exporters[$key]())->export($this);
}

$grid->export('csv');

About the implode method in the Grid class I love the idea, at first I thought it wasn't necessary but now I see is a great feature.

How do you propose to pass the Grid object to the export method? I would like to see a sample implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
Query Grid
  
To do
Development

No branches or pull requests

2 participants