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

5.x #680

Closed
wants to merge 7 commits into from
Closed

5.x #680

wants to merge 7 commits into from

Conversation

t-works
Copy link

@t-works t-works commented Jun 13, 2022

Subject

I am targeting this branch, because this fixes issues in current branch 5.x and because it is supposed to work && tested with symfony 6.1+.

This is rebased version.

I fixed issues preventing CKEditor upload and browsing
Closes #669

Changelog

FIXED: CKEditor upload and browse files with symfony 6.1 and doctrine-orm-admin-bundle

### Added
- Added service configuration for Sonata\FormatterBundle\Controller\CkeditorAdminController

### Fixed
- browser.html.twig (compatibility with doctrine-orm-admin-bundle)
- upload.html.twig 

## To do
- [ ] Fix label translation in file browser filters;
- [ ] Update the documentation;
- [ ] Allow using custom contexts. (currently using "context:  my-context" in fos_ck_editor.configs.default of tha application does not work with file browser)

@jordisala1991
Copy link
Member

Now the bundle tests browser and upload routes, with functional tests using doctrine orm admin bundle, from 4.4 up to 6.1 versions of Symfony.

Can you check the 5.x-dev (without this PR) on your project to see if everything works?

@VincentLanglet
Copy link
Member

@jordisala1991 Now your PR is merged, is there additional fixes that we have to take from this one ?

@jordisala1991
Copy link
Member

@jordisala1991 Now your PR is merged, is there additional fixes that we have to take from this one ?

I don't know yet, that's why I have asked @t-works to tests the 5.x-dev branch.

@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

@t-works
Copy link
Author

t-works commented Jun 16, 2022

Now your PR is merged, is there additional fixes that we have to take from this one ?

The browse server action in FCKEditor triggers this errror:

Neither the property "sortparameters" nor one of the methods "sortparameters()", "getsortparameters()"/"issortparameters()"/"hassortparameters()" or "__call()" exist and have public access in class
"Sonata\DoctrineORMAdminBundle\Model\ModelManager".

Twig\Error
RuntimeError
in C:\var\projects\stronaklubu2\vendor\sonata-project\formatter-bundle\src\Resources\views\Ckeditor\browser.html.twig (line 97)

{% set sort_parameters = datagrid.sortparameters(field_description, datagrid)|merge(ckParameters) %}

I described reason in previous discussion - it's got to be:
{% set sort_parameters = datagrid.sortparameters(field_description, datagrid)|merge(ckParameters) %}

The upload action works fine.

BTW please note I had to use a fork of the
https://github.com/coopTilleuls/CoopTilleulsCKEditorSonataMediaBundle
to use it with symfony6
the fork with dependencies pushed up and changed name to avoid namespace clash is here:
https://github.com/t-works/CKEditorSonataMediaBundleFork

Could you please rebase your PR and fix merge conflicts?

I fixed merge conflicts, removed xml config since you rewrote it to PHP.

A note on tests:

public function uploadAction(Request $request, MediaManagerInterface $mediaManager): Response
....
media = $mediaManager->create();

2 arguments required here.

@VincentLanglet
Copy link
Member

Now your PR is merged, is there additional fixes that we have to take from this one ?

The browse server action in FCKEditor triggers this errror:

Neither the property "sortparameters" nor one of the methods "sortparameters()", "getsortparameters()"/"issortparameters()"/"hassortparameters()" or "__call()" exist and have public access in class "Sonata\DoctrineORMAdminBundle\Model\ModelManager".

Twig\Error RuntimeError in C:\var\projects\stronaklubu2\vendor\sonata-project\formatter-bundle\src\Resources\views\Ckeditor\browser.html.twig (line 97)

{% set sort_parameters = datagrid.sortparameters(field_description, datagrid)|merge(ckParameters) %}

I described reason in previous discussion - it's got to be: {% set sort_parameters = datagrid.sortparameters(field_description, datagrid)|merge(ckParameters) %}

Thanks, we will add more functionnals tests and fix this

@jordisala1991
Copy link
Member

This should not be needed anymore. Closing for now. Have a look at 5.x branch now.

@VincentLanglet
Copy link
Member

This should now be covered by #693

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants