octobercms / october

Self-hosted CMS platform based on the Laravel PHP Framework.
https://octobercms.com/
Other
11.01k stars 2.22k forks source link

Dynamically extending ImportExport column list #2029

Closed Flynsarmy closed 8 years ago

Flynsarmy commented 8 years ago

The issue

I am using the ImportExportController behavior in my plugin however the columns to import and export must be dynamically generated at runtime based on the current backend URL. There doesn't appear to be a way to extend the list columns at runtime. See the current implementation here.

Proposal

I'd like to add a importExportExtendListColumnConfig controller method to handle this. The method name is ugly and can be changed. Here is my proposed implementation:

protected function makeListColumns($config)
{
    $config = $this->controller->importExportExtendListColumnConfig($config);
    $config = $this->makeConfig($config);

    if (!isset($config->columns) || !is_array($config->columns)) {
        return null;
    }
    ...
}

/**
 * Called after the list columns are defined.
 * @param mixed $config The current import/export columns
 * @return mixed
 */
public function importExportExtendListColumnConfig($config)
{
    return $config;
}

The makeListColumns method is used by both getImportDbColumns and getExportColumns so perhaps adding a $context argument to makeListColumns which is then passed to importExportExtendListColumnConfig would be helpful too.

Example Usage

In a controller implementing ImportExportController behavior:

public function importExportExtendListColumnConfig($config)
{
    $config = Yaml::parse(File::get($config));
    $config['columns']['my_new-column'] = ['label' => 'My New Column'];

    return $config;
}

If you're happy with this I can write up a pull request.

daftspunk commented 8 years ago

This sounds right, except maybe the method should probably be called importExportExtendColumns. We need to understand more of the why instead of the how. Perhaps there is a more elegant way to solve the problem, but without knowing too much about the problem it is difficult to know.

Can you submit a PR to the test plugin that outlines the problem? Test plugin for investigation

EdOzolins commented 6 years ago

Not sure if this solves the problem completely, but I think it is starting point... Extend your controller

    public function import()
    {

        if (Request::url() == 'example.oi') {
            $this->importColumns = [
                'column_name' => 'Column Label'
            ];            
        }

        $this->asExtension('ImportExportController')->import();
    }
jeremymouton commented 2 years ago

This is an old topic, but I was able to extend a plugin's export columns by overwriting the config file entirely.

\Acme\MyPlugin\Controllers\MyController::extend(function($controller) {
  $controller->importExportConfig = '$/acme2/mypluginextension/controllers/mycontroller/config_import_export.yaml';
});
daftspunk commented 2 years ago

Thanks @jeremymouton

There isn't still a clear use case for this, however, we've added some basic overrides in v2.1.11

/**
 * importExportExtendExportColumns
 */
public function importExportExtendExportColumns($columns)
{
    return $columns;
}

/**
 * importExportExtendImportColumns
 */
public function importExportExtendImportColumns($columns)
{
    return $columns;
}
SebastiaanKloos commented 2 years ago

Thanks @jeremymouton

There isn't still a clear use case for this, however, we've added some basic overrides in v2.1.11

/**
 * importExportExtendExportColumns
 */
public function importExportExtendExportColumns($columns)
{
    return $columns;
}

/**
 * importExportExtendImportColumns
 */
public function importExportExtendImportColumns($columns)
{
    return $columns;
}

Hi @daftspunk, There is currently no way to use the function. You can't override the function in the controller consuming the behavior.

Maybe this can be implemented in the behavior.

$columns = $this->controller->importExportExtendImportColumns($columns);

What do you think?

daftspunk commented 2 years ago

Good catch @SebastiaanKloos - fixed in v2.1.12

Thank you!