Sonataadminbundle: Audit entity compare not working

[cassianotartari]

Environment

Sonata packages

$ composer show sonata-project/*
sonata-project/admin-bundle              3.3.2 The missing Symfony Admin Generator
sonata-project/block-bundle              3.0.1 Symfony SonataBlockBundle
sonata-project/cache                     1.0.7 Cache library
sonata-project/core-bundle               3.0.3 Symfony SonataCoreBundle
sonata-project/doctrine-orm-admin-bundle 3.0.4 Symfony Sonata / Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/exporter                  1.5.0 Lightweight Exporter library

Symfony packages

$ composer show symfony/*
symfony/assetic-bundle     v2.8.0  Integrates Assetic into Symfony2
symfony/monolog-bundle     2.11.1  Symfony MonologBundle
symfony/phpunit-bridge     v3.1.1  Symfony PHPUnit Bridge
symfony/polyfill-apcu      v1.2.0  Symfony polyfill backporting apcu_* functions to lower PHP versions
symfony/polyfill-intl-icu  v1.2.0  Symfony polyfill for intl's ICU-related data and classes
symfony/polyfill-mbstring  v1.2.0  Symfony polyfill for the Mbstring extension
symfony/polyfill-php56     v1.2.0  Symfony polyfill backporting some PHP 5.6+ features to lower PHP versions
symfony/polyfill-php70     v1.2.0  Symfony polyfill backporting some PHP 7.0+ features to lower PHP versions
symfony/polyfill-util      v1.2.0  Symfony utilities for portability of PHP codes
symfony/security-acl       v3.0.0  Symfony Security Component - ACL (Access Control List)
symfony/swiftmailer-bundle v2.3.11 Symfony SwiftmailerBundle
symfony/symfony            v3.1.1  The Symfony PHP framework

PHP version

$ php -v
PHP 7.0.8-3+deb.sury.org~trusty+1 (cli) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies
    with Xdebug v2.4.0, Copyright (c) 2002-2016, by Derick Rethans
    with blackfire v1.10.4, https://blackfire.io, by Blackfireio Inc.

Subject

I've enable orm entity audit following docs but when I try to compare revisions it shows only the entity without the compared entity and highlighted fields.

Digging into why I've found that it's no possible to extend a block provided from a macro. Look at this issue at Stackoverflow.

CRUDController:historyCompareRevisionsAction renders SonataAdminBundle:CRUD:base_show_compare.html that extends SonataAdminBundle:CRUD:base_show.html.twig and this imports macro SonataAdminBundle:CRUD:base_show_macro.html.twig with blocks show_field and show_title thats base_show_compare try to override but nothing happens. So compare action only shows the entity.

To test this I've pasted code from base_show_macro into base_show making necessary adjusts and compare action works great, highlighting with read modified fields.

I hope I made the flow clear.

Steps to reproduce

Enable audit and try to compare revisions. It will show only the entity without compared entity.

Expected results

Show 2 entities and highlighted fields.

Actual results

Show only the revision.

[greg0ire]

Digging into why I've found that it's no possible to extend a block provided from a macro.

That's the precise moment where I don't understand your issue anymore. What does this have to do with the issue at hand? Also, I looked at the history for the authors of this, sadly they are not on github. But their email is, maybe you should shoot them an email, asking them to comment on this?

[cassianotartari]

That's the precise moment where I don't understand your issue anymore. What does this have to do with the issue at hand? Also, I looked at the history for the authors of this, sadly they are not on github. But their email is, maybe you should shoot them an email, asking them to comment on this?

As said in the stackoverflow issue you can't override a block provided from a macro and it's what SonataAdminBundle:CRUD:base_show_compare.html tries to do.

@greg0ire sorry but I don't know where to look to find authors emails, I looked and did not find.

[greg0ire]

Here is one (hover the question mark) : https://github.com/sonata-project/SonataAdminBundle/commit/56331388135a2fed37bb2e962a7e875d7fa9c954

[cassianotartari]

Here is one (hover the question mark) : 5633138

Sorry, I am totally inexperienced with the Github/Git, shame on me. Still not find, I didn't see any question mark to over it. You may have lost patience...

[greg0ire]

You may have lost patience...

Not yet ;)

1. click on the link
2. press Ctrl+F
3. Type "Niels"
4. Question mark will be on the right

[cassianotartari]

I was looking to this before. Maybe it's not shown to me. I'm logged in.

I found before you paste the link Niels V/NLZ13 and tried to google it but no luck...

[greg0ire]

Indeed, it's not shown for you :(

[greg0ire]

Just clone the whole repo and type git show 5633138 then…

[cassianotartari]

I'll try it right now

[cassianotartari]

As I added a reaction, I found it and emailed him. Thanks for the patient @greg0ire

[nlzet]

@cassianotartari @greg0ire thanks for mailing me, I am the one who made that commit. A long time ago I renamed my github account which had some consequences :/.

It is a while ago I made this, and I think back at the time there was no "base_show_macro", all the blocks where in "base_show", and for that reason it was easy to create a "base_show_compare" which extends "base_show" and only overrides some blocks.

This doesn't seem to work anymore since there is a macro. What @cassianotartari mentioned above should be true, this should work by removing the macro and just putting the blocks inside the "base_show" view, because the only things that are broken, are the blocks "show_title" and "show_field".

Finally, if you inspect "base_show" you can see that the macro has been made to remove some code duplication, so if you want to revert this, this will restore the duplication.

This looks like the breaking commit: https://github.com/sonata-project/SonataAdminBundle/commit/fd1a1596f86bfb4afdafe9767efaa4c787bba6a5

[greg0ire]

thanks for mailing me, I am the one who made that commit. A long time ago I renamed my github account which had some consequences :/.

I think you can still associate the email with your account, so that you regain the authorship of these commits on github ;)

This looks like the breaking commit: fd1a159

Aaaaand same here! The author is unrecognized… guess you should mail him too @cassianotartari …

[cassianotartari]

@nlzet thanks for answering.

Aaaaand same here! The author is unrecognized… guess you should mail him too @cassianotartari …

Done.

[cassianotartari]

This macro issue has impacts in other uses than entity compare such as I faced now. I configured a custom show template just to override show_field block with a if inside to catch just one specific field_name and it is not working. Older versions of sonata admin it was working.

[soullivaneuh]

@cassianotartari I reproduced the bug. Are you working for a PR?

Show only the revision.

Not totally true. ATM, this is showing always the last revision of the entity.

[cassianotartari]

No, I've just override the issue template in my project.

Maybe the solution is revert @nlzet commit to remove macro

[soullivaneuh]

@cassianotartari Maybe, but as @nlzet said:

Finally, if you inspect "base_show" you can see that the macro has been made to remove some code duplication, so if you want to revert this, this will restore the duplication.

I would like to avoid this and find an alternative.

Could you please post your code of your solution? This could be helpful finding a final solution.

[cassianotartari]

I was looking to my code right now, I did the only way I found that time, duplicate blocks. But as I'm using tabs didn't check the elseif of has_tab. The show_helper macro can't be used here...

My show_compare template is extending my own base_show. And I'm using tabs, so I've commented:

{{ show_helper.render_groups(admin, object, elements, show_tab.groups, has_tab) }}

And added in same place:

<div class="row">
    {% for code in show_tab.groups %}
        {% set show_group = admin.showgroups[code] %}

        <div class="{{ show_group.class|default('col-md-12') }} {{ false ? 'nopadding' }}">
            <div class="{{ show_group.box_class }}">
                <div class="box-header">
                    <h4 class="box-title">
                        {% block show_title %}
                            {{ admin.trans(show_group.name, {}, show_group.translation_domain) }}
                        {% endblock %}
                    </h4>
                </div>
                <div class="box-body table-responsive no-padding">
                    <table class="table">
                        <tbody>
                        {% for field_name in show_group.fields %}
                            {% block show_field %}
                                    <tr class="sonata-ba-view-container">
                                        {% if elements[field_name] is defined %}
                                            {{ elements[field_name]|render_view_element(object)}}
                                        {% endif %}
                                    </tr>
                            {% endblock %}
                        {% endfor %}
                        </tbody>
                    </table>
                </div>
            </div>
        </div>
    {% endfor %}
</div>

[hoffereka]

Is there any official fix for this? It'd be nice to use compare functionality out of the box. Or could someone describe in a couple of steps how to get working it? What to override, what to remove or something? Thanks in advance!