Sonataadminbundle: Improve the way we override and translate export label

Created on 29 Jan 2020  路  7Comments  路  Source: sonata-project/SonataAdminBundle

Feature Request (or Bug ?)

When exporting field

/**
     * @return array
     */
    public function getExportFields()
    {
        return [
            'lead.usagename',
            'lead.firstname',
        ];
    }

The labels used are export.label_lead_usagename and export.label_lead_firstname.

I would like to use another label instead. This would be great to override it this way:

/**
     * @return array
     */
    public function getExportFields()
    {
        return [
            'export.label_usagename' => 'lead.usagename',
            'lead.firstname',
        ];
    }

I thought this was implemented, but it doesn't seemed to be. Or it doesn't work for me.

feature

Most helpful comment

I would like to work on this issue.

The implementation will lead on 4.0 to the following use:

public function getExportFields()
    {
        return [
            'special_label' => 'foo',
            'bar',
        ];
    }

I don't feel like mixing array entries with key specified and without are a great pattern.
I know some linters which are banning the syntax.

I was looking for a better syntax and I had an idea.

Using a configureExportFields with something like an ExportMapper.

public function configureExportFields(ExportMapper $exportMapper)
    {
        $exportMapper->add('foo');
        $exportMapper->add('bar', 'trans');
        $exportMapper->add('baz', null, ['label' => 'special_label']);
    }

This would be easier to add new options.

WDYT @greg0ire @core23 @phansys ?

All 7 comments

Can you provide a PR with this feature? @VincentLanglet

@core23 I just looked how to do it.

This is here, and already half-implemented.
I can provide a PR easily, but I have some questions. Seems like there is decisions to take.
(And this can be a BC depends on the point of view)

public function getDataSourceIterator()
    {
        $datagrid = $this->getDatagrid();
        $datagrid->buildPager();

        $fields = [];

        foreach ($this->getExportFields() as $key => $field) {
            $label = $this->getTranslationLabel($field, 'export', 'label');
            $transLabel = $this->trans($label);

            // NEXT_MAJOR: Remove this hack, because all field labels will be translated with the major release
            // No translation key exists
            if ($transLabel === $label) {
                $fields[$key] = $field;
            } else {
                $fields[$transLabel] = $field;
            }
        }

        return $this->getModelManager()->getDataSourceIterator($datagrid, $fields);
    }

After some tests 'foo' => 'bar' returns

  • trans('export.label_bar)` if the trans exists.
  • foo if the trans does not exists.

I was expecting trans('foo') every time since I want to override the label.

When someone write

->add('bar', TextType::class, [
    'label' => 'foo',
])

We get trans('foo') every time. Even if show.label_bar exists.

I made another test.
'bar' without key, use the label 'bar' if export.label_bar has no translation ; I don't know why.

I was expecting one of the following:

  • 0, as the key should be 0.
  • export.label_bar, this way it's easier to know which key you have to add in the translations.

cc @greg0ire @OskarStark @phansys
I'll be happy having you point of view :)

I was expecting trans('foo') every time since I want to override the label.

The reason why we check for a translated key is to keep BC. We introduced export label translation in a minor version. As the resulting key had changed, we had to add a BC layer, so it could work as before for legacy users.

IMHO if the user provides a stringable key, it should be used in any case.

I would like to work on this issue.

The implementation will lead on 4.0 to the following use:

public function getExportFields()
    {
        return [
            'special_label' => 'foo',
            'bar',
        ];
    }

I don't feel like mixing array entries with key specified and without are a great pattern.
I know some linters which are banning the syntax.

I was looking for a better syntax and I had an idea.

Using a configureExportFields with something like an ExportMapper.

public function configureExportFields(ExportMapper $exportMapper)
    {
        $exportMapper->add('foo');
        $exportMapper->add('bar', 'trans');
        $exportMapper->add('baz', null, ['label' => 'special_label']);
    }

This would be easier to add new options.

WDYT @greg0ire @core23 @phansys ?

@VincentLanglet Good IDEA!!! I have some question about Admin self-referencing and this idea.

  • It is possible to create multi level menu from Admins (order => order-element => element, order => biling address) (filters for order element more then 3 elements, and element cheper then 12$) ? Parent/child must be config.
  • user should pick which admin list want watch, (filters should be the same)
  • parents fileds should be displayed,
  • children should display children count,
  • parents active could be toggled (probable with reset filters)
  • parents in list can be optional show as modal (show page)

This scructure will be huge improve for AdminBundle and your idea. More of this things are easy to do.

Back to your idea, you want change generate source to exporter from array to ExporterMapper.

  • We can add user callback function to add own fields (like priceSum) or add own exporter/report type by extension.
  • Extensions should allow return multiple fields and have own options (simmilar to form type)
  • support for iterator and aggregated methods,
  • allow add own report structure (xls)

As recently discussed in some other PRs, we have another issue to fix with the export functionality.

Indeed the translator property of AbstractAdmin is currently deprecated and will probably be removed in the next major.
But there is no other solution implemented to translate the export labels export.label_foo.

Was this page helpful?
0 / 5 - 0 ratings