Skip to content

Conversation

adou600
Copy link
Member

@adou600 adou600 commented Dec 21, 2012

After new content is created with CreateJS, a frontend script needs to take care of the routes creation, if the SimpleCmfBundle is not used.

This PR is linked to symfony-cmf/cmf-sandbox#143.

Do not merge this PR for now, it has been created to synchronize the work in progress.

Copy link
Member

Choose a reason for hiding this comment

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

maybe you should make this a list of rdf types for which to create routes, defaulting to empty array.

if i create a new simplecms document or a new block, i don't need a route for that...

@adou600
Copy link
Member Author

adou600 commented Jan 17, 2013

@dbu this PR is also ready for review.

Combined with the changes on the Sandbox and CreatePHP, it is now possible to automatically create the routes for new NewsArticle thanks to a JavaScript bound to the midgardstoragesavedentity event.

@lsmith77
Copy link
Member

@adou600 is everything merged in CreatePHP?

@dbu
Copy link
Member

dbu commented Jan 17, 2013

openpsa/createphp#46 is not merged yet. and @flack is away until the end of the month.

i wonder if we should merge all our fixes in the symfony-cmf fork of createphp and temporarily point the sandbox to that fork until its all sorted out?

@lsmith77
Copy link
Member

hmm tricky .. i worry that this will cause even more confusion for users. we already have issues here and there with BC breaks etc .. then again if you guys are 100% sure that this will go through without changes once he is back its a bit painful to have to wait again to finally get this in ..

@dbu
Copy link
Member

dbu commented Jan 18, 2013

i just discussed with adrien: this PR does not need any updates in createphp, so i will review and merge now.

however for the add to really work in the sandbox we need the createphp PR to be merged... i will comment some more there.

Copy link
Member

Choose a reason for hiding this comment

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

do we really need a new document here? this could become tricky.

Copy link
Member

Choose a reason for hiding this comment

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

i wonder if we can handle that generically. or otherwise move it into the RoutingExtraBundle as it is useful in general?

Copy link
Member

Choose a reason for hiding this comment

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

yes this seems like just adding 2 convenience methods, think it will however cause more trouble than its worth.

Copy link
Member

Choose a reason for hiding this comment

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

well the problem will be the mapper that is guessing a method name from local and has no way of knowing how to correctly map a locale otherwise. but maybe we should extend the mapper for the route and handle that?

Copy link
Member

Choose a reason for hiding this comment

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

i discussed with adou and we will make another PR on createphp and make it possible to configure specific mappers for some classes and provide a mapper that handles locale on the route. this solves a general shortcoming of createphp then and is more elegant.

Copy link
Member

Choose a reason for hiding this comment

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

i would prefer you inject the parameters explicitly .. if you want you can inject them all as an array

@dbu dbu removed this from the 1.1 milestone Apr 4, 2014
@lsmith77
Copy link
Member

this should now be handled via RoutingAutoBundle

1 similar comment
@lsmith77
Copy link
Member

this should now be handled via RoutingAutoBundle

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

Successfully merging this pull request may close these issues.

3 participants