Skip to content
This repository has been archived by the owner on Sep 30, 2021. It is now read-only.

[WIP] Fixes for PHPCR-ODM #82

Closed

Conversation

dantleech
Copy link

This PR will hopefully succeed in getting the PHPCR-ODM implemntation working.

Todo:

  • The EasyExtendsBundle creates invalid mappings
  • Paths (e.g. `/cms/media/cms/media/53d00ace7b3a6' are not permitted IDs

@@ -32,7 +32,9 @@ public function process(ContainerBuilder $container)
$definition = $container->getDefinition('sonata.core.model.adapter.chain');

if ($container->has('doctrine')) {
$definition->addMethodCall('addAdapter', array(new Reference('sonata.core.model.adapter.doctrine_orm')));
if (class_exists('\Doctrine\ORM\Version')) {
Copy link
Author

Choose a reason for hiding this comment

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

the doctrine service is always present when the DoctrinePhpcrBundle is activated - but this does not mean the ORM is installed.

@dantleech
Copy link
Author

@rande Can you suggest any solutions for making the PHPCR-ODM "id's" work?

i.e. last time I seem to recall that we made the routing requirements less strict to allow slashes in the ID. What do you think?

@benglass
Copy link

@dantleech the issue with ids not working is in a PR sonata-project/SonataDoctrinePhpcrAdminBundle#284

@dantleech
Copy link
Author

I thought the problem here was with the Route requirements (at least thats the error I was confronting - maybe problem you mention is the one I would encounter next ....).

I also wonder if we shouldn't just make the ID the UUID - that would make alot more sense and avoid potential future problem with same-name-siblings if that ever gets implemented in Jackalope. /cc @dbu

@dbu
Copy link

dbu commented Jul 24, 2014

the official ID for a phpcr-odm document is the path, not the uuid. so we should keep using that. same-name siblings would have a [2] and so on at the end. more important, not every document has a uuid, only when it is made referenceable.

@dantleech
Copy link
Author

but we could encforce the UUID mapping on Sonata Media documents -- also UUIDs are not mutable and they don't contain slashes :)

@dbu
Copy link

dbu commented Jul 24, 2014

i merged sonata-project/SonataDoctrinePhpcrAdminBundle#284 - does this fix the issue here?

@dantleech
Copy link
Author

dantleech commented Jul 24, 2014

@dbu: no, the error is:

An exception has been thrown during the rendering of a template ("Parameter "id" for route "sonata_media_download" must match "[^/]++" ("cms/media/53d16b4dc08e0" given) to generate a corresponding URL.") in SonataMediaBundle:MediaAdmin:edit.html.twig at line 73.

I remember that when we had this before it was necessary to relax the requirement on the route object. Looking into it.

@SonataCI
Copy link
Collaborator

SonataCI commented Aug 5, 2015

Can one of the admins verify this patch?

@core23
Copy link
Member

core23 commented Feb 11, 2016

@dantleech Please rebase

}

$res = $this->modelAdapter->getUrlsafeIdentifier($model);
return $res;
Copy link
Member

Choose a reason for hiding this comment

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

Why this?

@dbu
Copy link

dbu commented Apr 25, 2016

@dantleech is any of this still relevant? sonata admin does work these days without doctrine orm installed afaik.

@soullivaneuh
Copy link
Member

According to the new Sonata version management and next major release plan, this project has been refactored regarding branching and versioning.

If you see this message, your PR concerns a patch or a minor release and is not targeting the right branch.

So I'm closing this one, but don't see it as a refusal. If you think your work is still relevant and want to continue, feel free to reopen it on the right branch (e.g. the default one).

Regards.

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

Successfully merging this pull request may close these issues.

6 participants