Deprecate exporter class and service #4029

greg0ire · 2016-07-29T19:06:00Z

I am targetting this branch, because this is BC

Closes #3124 , #1550 (I think)

Changelog

### Changed
- The export and list actions now integrate the sonata exporter bundle

### Deprecated
- Exporter class and service : use equivalents from `sonata-project/exporter` instead.

To do

Wait for 1.6.0 to be released
Fix symfony 2.3 compatibility problem (the deprecated tag is not supported)
Wait for 1.7.0 to be released
Refactor CrudController::exportAction() to use a dedicated, unit-tested service when available
Create the sonata.admin.admin_exporter service when the exporter bundle is registered
Use getAvailableFormats() to display the formats list
Document all this

Subject

This deprecates the export class and service

greg0ire · 2016-07-30T16:36:08Z

Shit, I can't deprecate services in old versions of symfony…

greg0ire · 2016-08-01T08:16:48Z

Fixed. Please review @sonata-project/contributors

greg0ire · 2016-08-16T17:41:11Z

Hmmm… there is another problem : getExportFormats() I think this method should be taken into account only if it differs from the default formats, and I need sonata-project/exporter@0f642e1 to get them. @soullivaneuh , can you release exporter 1.7.0 ? ~~Also, can I require 1.7.0 without this being considered major?~~ Won't solve my problem since you have to register the bundle to get the service…
~~Otherwise the code will get really ugly, b/c I will have to check that the service exists AND that the getAvailableFormats() method exists.~~ I'll add a conflict in composer.json.

OskarStark · 2016-08-17T09:01:31Z

getAvailableFormats is now available in 1.7.0 @greg0ire

greg0ire · 2016-08-17T09:38:04Z

I know, but I realized that I took it into account when POSTing, but must also make sure the format list in the combo box is updated too.

greg0ire · 2016-08-17T22:01:41Z

Bridge/Exporter/AdminExporter.php

+use Exporter\Exporter;
+use Sonata\AdminBundle\Admin\AdminInterface;
+
+final class AdminExporter


@sonata-project/contributors : if you have a better name for this service I'm all ears. I extracted this from CrudController::exportAction so that it can be easily unit tested.

core23 · 2016-08-18T02:50:34Z

Admin/AbstractAdmin.php

@@ -66,6 +66,7 @@
        )\\\(.*)@x';

    const MOSAIC_ICON_CLASS = 'fa fa-th-large fa-fw';
+    const DEFAULT_EXPORT_FORMATS = array('json', 'xml', 'csv', 'xls');


Why? We can't override this constant and we only use it once.

Three times you mean. On the getter, in the new service, and in the test for the new service

Just read the diff...

You extracted a new constant which is only used once. So why did we need this?

Fixed it, it is actually used 3 times now.

Okay then 👍

SonataCI · 2017-01-13T15:51:56Z

Could you please rebase your PR and fix merge conflicts?

core23 · 2017-03-12T08:36:25Z

Please rebase, so we can merge this @greg0ire

The class was already marked as deprecated, but only in its phpdoc. Let's schedule its deletion.

greg0ire · 2017-03-12T09:41:59Z

@core23 done

greg0ire · 2017-03-12T10:04:01Z

@sonata-project/contributors please merge

jordisala1991 · 2017-03-12T10:07:44Z

Resources/views/CRUD/base_list.html.twig

@@ -176,16 +176,19 @@ file that was distributed with this source code.
                                </div>


+                                {# NEXT_MAJOR : remove this assignment #}
+                                {% set export_formats = export_formats|default(admin.getExportFormats) %}


Just to make sure, admin.getExportFormats will work the same as admin.getExportFormats()?

AFAIK you can do something like admin.exportFormats at twig will pick the getter.

If this works also, lets merge this

It works also, but I can change it to your solution, which is more graceful.

Your decision, tell me when you want me to merge

Please merge when the build is green.

jordisala1991 · 2017-03-12T11:09:32Z

Thanks @greg0ire !!

greg0ire · 2017-03-12T11:13:36Z

Finally! Thanks everyone!

OskarStark · 2017-03-13T08:26:31Z

@greg0ire it would be great if you could remove the deprecated code afterwards on master

greg0ire · 2017-03-13T09:35:41Z

@OskarStark will do!

soullivaneuh · 2017-03-16T09:13:51Z

UPGRADE-3.x.md

@@ -65,6 +65,12 @@ All files under the ``Tests`` directory are now correctly handled as internal te
 You can't extend them anymore, because they are only loaded when running internal tests.
 More information can be found in the [composer docs](https://getcomposer.org/doc/04-schema.md#autoload-dev).

+### Exporter service and class


Again on the wrong place. Please be careful.

A check bot is very necessary for this IMO.

Argh! Sorry…

SonataCI added the review required label Jul 29, 2016

greg0ire force-pushed the deprecate_exporter branch 4 times, most recently from 1d2a922 to 1d06d9b Compare July 30, 2016 11:44

greg0ire force-pushed the deprecate_exporter branch 2 times, most recently from c903a46 to b51fb4b Compare July 30, 2016 16:57

greg0ire force-pushed the deprecate_exporter branch 2 times, most recently from 53feda0 to b51fb4b Compare August 3, 2016 21:14

greg0ire mentioned this pull request Aug 5, 2016

Add missing deprecation message and warning for Exporter #3960

Closed

1 task

greg0ire force-pushed the deprecate_exporter branch from b51fb4b to b56a426 Compare August 16, 2016 18:08

greg0ire added the in progress label Aug 16, 2016

greg0ire reviewed Aug 17, 2016
View reviewed changes

greg0ire force-pushed the deprecate_exporter branch from 206bf9d to f98932b Compare August 17, 2016 22:06

core23 reviewed Aug 18, 2016
View reviewed changes

greg0ire force-pushed the deprecate_exporter branch from f98932b to 96b340a Compare August 18, 2016 06:45

greg0ire added the minor label Aug 18, 2016

greg0ire force-pushed the deprecate_exporter branch from 96b340a to 1245bda Compare August 18, 2016 18:04

greg0ire removed the in progress label Aug 18, 2016

greg0ire force-pushed the deprecate_exporter branch 2 times, most recently from 3f378a1 to a1dad64 Compare August 18, 2016 21:55

greg0ire force-pushed the deprecate_exporter branch from 8261504 to 76b8c56 Compare November 30, 2016 18:42

SonataCI added the pending author label Jan 13, 2017

greg0ire mentioned this pull request Jan 27, 2017

add customizing solutions to export action docs #4294

Merged

greg0ire added 5 commits March 12, 2017 09:38

Deprecate exporter class and service

52eab4b

The class was already marked as deprecated, but only in its phpdoc. Let's schedule its deletion.

Get default formats from the exporter

f31dea1

Create admin exporter class

f4388b6

Use admin exporter service when available

9c1f2c2

Load admin exporter service conditionally

c41978a

greg0ire force-pushed the deprecate_exporter branch from 76b8c56 to f6ab219 Compare March 12, 2017 08:46

SonataCI removed the pending author label Mar 12, 2017

greg0ire force-pushed the deprecate_exporter branch from f6ab219 to dd37721 Compare March 12, 2017 09:17

core23 approved these changes Mar 12, 2017

View reviewed changes

jordisala1991 reviewed Mar 12, 2017

View reviewed changes

jordisala1991 approved these changes Mar 12, 2017

View reviewed changes

greg0ire added 3 commits March 12, 2017 11:14

Ask admin exporter service for available formats

b80ab52

Fix broken tests

84b82f8

Document the export action

9b0829d

greg0ire force-pushed the deprecate_exporter branch from dd37721 to 9b0829d Compare March 12, 2017 10:14

jordisala1991 merged commit f880155 into sonata-project:3.x Mar 12, 2017

greg0ire deleted the deprecate_exporter branch March 12, 2017 11:12

OskarStark removed the review required label Mar 13, 2017

soullivaneuh reviewed Mar 16, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate exporter class and service #4029

Deprecate exporter class and service #4029

greg0ire commented Jul 29, 2016 •

edited

Loading

greg0ire commented Jul 30, 2016

greg0ire commented Aug 1, 2016

greg0ire commented Aug 16, 2016 •

edited

Loading

OskarStark commented Aug 17, 2016 •

edited

Loading

greg0ire commented Aug 17, 2016

greg0ire Aug 17, 2016

core23 Aug 18, 2016

greg0ire Aug 18, 2016

core23 Aug 18, 2016

greg0ire Aug 18, 2016

core23 Aug 19, 2016

SonataCI commented Jan 13, 2017

core23 commented Mar 12, 2017

greg0ire commented Mar 12, 2017

greg0ire commented Mar 12, 2017

jordisala1991 Mar 12, 2017

greg0ire Mar 12, 2017

jordisala1991 Mar 12, 2017

greg0ire Mar 12, 2017

jordisala1991 commented Mar 12, 2017

greg0ire commented Mar 12, 2017

OskarStark commented Mar 13, 2017

greg0ire commented Mar 13, 2017

soullivaneuh Mar 16, 2017

greg0ire Mar 16, 2017

Deprecate exporter class and service #4029

Deprecate exporter class and service #4029

Conversation

greg0ire commented Jul 29, 2016 • edited Loading

Changelog

To do

Subject

greg0ire commented Jul 30, 2016

greg0ire commented Aug 1, 2016

greg0ire commented Aug 16, 2016 • edited Loading

OskarStark commented Aug 17, 2016 • edited Loading

greg0ire commented Aug 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SonataCI commented Jan 13, 2017

core23 commented Mar 12, 2017

greg0ire commented Mar 12, 2017

greg0ire commented Mar 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jordisala1991 commented Mar 12, 2017

greg0ire commented Mar 12, 2017

OskarStark commented Mar 13, 2017

greg0ire commented Mar 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greg0ire commented Jul 29, 2016 •

edited

Loading

greg0ire commented Aug 16, 2016 •

edited

Loading

OskarStark commented Aug 17, 2016 •

edited

Loading