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.

Sign up for GitHub

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

Jump to bottom

updated docs for child admins #5115

Merged
merged 2 commits into from
Jun 7, 2018
Merged

updated docs for child admins #5115

merged 2 commits into from
Jun 7, 2018

Conversation

OskarStark
Copy link
Member

Refs #5109

  • I added yaml configuration
  • I renamed everything to app.admin..... instead of sonata.admin......

@OskarStark OskarStark added the docs label Jun 4, 2018
OskarStark
OskarStark commented Jun 4, 2018
View reviewed changes
docs/reference/child_admin.rst Outdated
<argument type="service" id="sonata.admin.video" />
<argument>playlist</argument>
<argument type="service" id="app.admin.video" />
<argument>video</argument>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure, but I guess it should be the name of the child like this? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nop, you need to provide parent field on your child entity. So if Video is the entity it should have playlist field that points to Playlist Entity.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I am calling addChild on the Playlist, and add 'Video' to it.

So the second Parameter is Name of the Parent instead of Name of the Child? (@see #5109 )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, my bad there, I rechecked the code a few minutes ago and then I see it is the name of the parent or in this case the value you had in your $parentAssociationMapping

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok 👍

greg0ire
greg0ire reviewed Jun 4, 2018
View reviewed changes
docs/reference/child_admin.rst Outdated
.. code-block:: yaml

# app/config/services.yml
app.admin.playlist:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using the FQCN of the class here?

greg0ire
greg0ire reviewed Jun 4, 2018
View reviewed changes
docs/reference/child_admin.rst Outdated
<!-- app/config/config.xml -->
<service id="sonata.admin.playlist" class="App\Admin\PlaylistAdmin">
<!-- app/config/services.xml -->
<service id="App\Admin\PlaylistAdmin" class="App\Admin\PlaylistAdmin">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we still need the class attribute?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure for XML 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we keep them in all our config files, so the docs should be the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OskarStark here such services are described with id attribute only (see XML tab)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OskarStark looks like we made a mistake then 😅 please remove them

@OskarStark OskarStark mentioned this pull request Jun 4, 2018
Closed
@OskarStark
Copy link
Member Author

all comments applied 👍

@greg0ire
Copy link
Contributor

greg0ire commented Jun 6, 2018

For the FQCN, see symfony/symfony-docs#9886

@OskarStark
Copy link
Member Author

@greg0ire please merge 😄

@greg0ire
Copy link
Contributor

greg0ire commented Jun 7, 2018

For the FQCN, see symfony/symfony-docs#9886

I forgot this is about docs, so yeah, very mergeable, no need to wait for this.

@greg0ire greg0ire merged commit 0a010ba into sonata-project:3.x Jun 7, 2018
@OskarStark OskarStark deleted the docs-update branch June 7, 2018 07:08
@OskarStark
Copy link
Member Author

@greg0ire the build failed on 3.x

@kunicmarko20
Copy link
Contributor

Didn't we have that on Media also? because of new phpcs fixer

@greg0ire
Copy link
Contributor

greg0ire commented Jun 7, 2018

Highly probable.

pestaa pushed a commit to Crosssec/SonataAdminBundle that referenced this pull request Aug 6, 2018
@OskarStark @pestaa
Updated docs for children admins (sonata-project#5115)
b8f70b0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greg0ire greg0ire greg0ire left review comments

@kunicmarko20 kunicmarko20 kunicmarko20 approved these changes

@core23 core23 core23 approved these changes

@covex-nn covex-nn covex-nn approved these changes

Assignees
No one assigned
Labels
docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@OskarStark @greg0ire @kunicmarko20 @covex-nn @core23