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

Add button not showing for collection #113

Open
tom-s opened this issue Jul 30, 2014 · 13 comments
Open

Add button not showing for collection #113

tom-s opened this issue Jul 30, 2014 · 13 comments

Comments

@tom-s
Copy link

tom-s commented Jul 30, 2014

Hello,

First, thanks for your amazing bunddle.
I'm having issues with using collections : they get displayed correctly and are editable, but they don't show any "Add" button.

I've got the following RDF mappings

<!-- MyApp.Common.ContentBundle.Entity.ActualitesPage.xml-->
<type
    xmlns:sioc="http://rdfs.org/sioc/ns#"
    xmlns:dcterms="http://purl.org/dc/terms/"
    typeof="sioc:ActualitesPage"
>
    <children>
        <collection rel="dcterms:hasPart" rev="dcterms:partOf" identifier="sections"/>
    </children>
</type>

<!-- MyApp.Common.ContentBundle.Entity.ActualitesPage.xml-->
<type
    xmlns:sioc="http://rdfs.org/sioc/ns#"
    xmlns:dcterms="http://purl.org/dc/terms/"
    typeof="sioc:ActualitesPageSection"
>
    <children>
        <collection rel="dcterms:hasPart" rev="dcterms:partOf" identifier="sections"/>
    </children>
</type>

<!-- MyApp.Common.ContentBundle.Entity.ActualitesPageSection.xml-->
<type
    xmlns:sioc="http://rdfs.org/sioc/ns#"
    xmlns:dcterms="http://purl.org/dc/terms/"
    typeof="sioc:ActualitesPageSection"
>
    <rev>dcterms:partOf</rev>
    <children>
        <property property="dcterms:title" identifier="title"/>
        <collection rel="dcterms:hasPart" rev="dcterms:partOf" identifier="sectionLines"/>
    </children>
</type>

<!-- MyApp.Common.ContentBundle.Entity.ActualitesPageSectionLine.xml-->
<type
    xmlns:sioc="http://rdfs.org/sioc/ns#"
    xmlns:dcterms="http://purl.org/dc/terms/"
    typeof="sioc:ActualitesPageSectionLine"
>
    <rev>dcterms:partOf</rev>
    <children>
        <property property="dcterms:contentLeft" identifier="contentLeft"/>
        <property property="dcterms:contentRight" identifier="contentRight"/>
    </children>
</type>

which I'm displaying with the following template :

{% for section in sections %}
        {% createphp section as="rdf" noautotag %}
        <section class="actualites content" {{ createphp_attributes(rdf) }}>
            <h1 {{ createphp_attributes( rdf.title ) }}>{{ createphp_content( rdf.title ) }}</h1>
            {% for line in section.getSectionLines() %}
                {% createphp line as="linerdf" noautotag %}
                 <ul class="floated-list clearfix" {{ createphp_attributes(linerdf) }}>
                    <li class="actualite-image">
                        <div {{ createphp_attributes( linerdf.contentLeft ) }}>{{ createphp_content( linerdf.contentLeft ) }}</div>
                    </li>
                    <li class="actualite-text">
                        <div {{ createphp_attributes( linerdf.contentRight ) }}>{{ createphp_content( linerdf.contentRight ) }}</div>
                    </li>
                </ul>
                {% endcreatephp %}
            {% endfor %}
        </section>
        {% endcreatephp %}
    {% endfor %}

It seems my collections are not recognized as such, (no rev rel tag in the html), am I doing something wrong ?

Many thanks,

Thomas

@dbu
Copy link
Member

dbu commented Jul 30, 2014

there was somebody trying to implement the add button, see #24. but at the time there was no RoutingAutoBundle and he wanted to create documents that have routes, so everything got quite complicated, and in the end was never finished.

if you can help debug general collection support (without the problems we had in #24) that would be awesome. can you try to manually add the html attributes to see if its createphp not outputting needed information, or a create.js issue? i am not sure if createphp renders all the needed html when the mapping is there. there is an array mapping format too - maybe by digging into https://github.com/flack/createphp you can find what is going on?

@tom-s
Copy link
Author

tom-s commented Jul 30, 2014

I've made some progress with this. If I manually add rel="dcterms:hasPart" on my section, the add button show (meaning that createphp does not render the rel attribute).

{% for section in sections %}
        {% createphp section as="rdf" noautotag %}
        <section class="actualites content" {{ createphp_attributes(rdf) }} rel="dcterms:hasPart">>
            <h1 {{ createphp_attributes( rdf.title ) }}>{{ createphp_content( rdf.title ) }}</h1>
            {% for line in section.getSectionLines() %}
                {% createphp line as="linerdf" noautotag %}
                 <ul class="floated-list clearfix" {{ createphp_attributes(linerdf) }}>
                    <li class="actualite-image">
                        <div {{ createphp_attributes( linerdf.contentLeft ) }}>{{ createphp_content( linerdf.contentLeft ) }}</div>
                    </li>
                    <li class="actualite-text">
                        <div {{ createphp_attributes( linerdf.contentRight ) }}>{{ createphp_content( linerdf.contentRight ) }}</div>
                    </li>
                </ul>
                {% endcreatephp %}
            {% endfor %}
        </section>
        {% endcreatephp %}
    {% endfor %}

I know have an issue with the sectionline not being added correctly (it apears on the screen and I can edit it but when I click save I get the following error)
{"code":500,"message":"Notice: Undefined index: in /home/thomas/workspace/bo_symfony/vendor/midgard/createphp/src/Midgard/CreatePHP/RestService.php line 171"}

@tom-s
Copy link
Author

tom-s commented Jul 30, 2014

I've came up with a solution. This is probably not ideal, but it works for me and hopefully will help someone else.

In my template I actually need to add both rel="dcterms:hasPart" and rev="dcterms:partOf on my section. This will allow the parent ID to be posted when I save a child element. It gives:

{% for section in sections %}
        {% createphp section as="rdf" noautotag %}
        <section class="actualites content" {{ createphp_attributes(rdf) }}  rel="dcterms:hasPart" rev="dcterms:partOf">
            <h1 {{ createphp_attributes( rdf.title ) }}>{{ createphp_content( rdf.title ) }}</h1>
            {% for line in section.getSectionLines() %}
                {% createphp line as="linerdf" noautotag %}
                 <ul class="floated-list clearfix" {{ createphp_attributes(linerdf) }}>
                    <li class="actualite-image">
                        <div {{ createphp_attributes( linerdf.contentLeft ) }}>{{ createphp_content( linerdf.contentLeft ) }}</div>
                    </li>
                    <li class="actualite-text">
                        <div {{ createphp_attributes( linerdf.contentRight ) }}>{{ createphp_content( linerdf.contentRight ) }}</div>
                    </li>
                </ul>
                {% endcreatephp %}
            {% endfor %}
        </section>
        {% endcreatephp %}

In the file init-create-editor.js I add to activate the midgardCollectionWidget which wasn't by default. To do so I replace line 31

 collectionWidgets: {
            'default': null,
            'feature': 'midgardCollectionAdd'
        },

with

 collectionWidgets: {
            'default': 'midgardCollectionAdd',
            'feature': 'midgardCollectionAdd'
        },

In my config file I added

map:
        "http://rdfs.org/sioc/ns#ActualitesPageSectionLine": "MyApp\Common\ContentBundle\Entity\ActualitesPageSectionLine"
        "http://rdfs.org/sioc/ns#ActualitesPageSection": "MyApp\Common\ContentBundle\Entity\ActualitesPageSection"

As you noticed, this is different from what the documentation says. Following the (wrong) documentation it should be something like

map:
        "<http://rdfs.org/sioc/ns#ActualitesPageSectionLine>": "MyApp\Common\ContentBundle\Entity\ActualitesPageSectionLine"
        "<http://rdfs.org/sioc/ns#ActualitesPageSection>": "MyApp\Common\ContentBundle\Entity\ActualitesPageSection"

which generated an exception in createPHP. I think this needs to be updated.

These manipulations allowed my new objects to be stored in the database. Unfortunately they were not linked to their parent. To fix this issue I've created my own implementation of DoctrineOrmMapper which contains the following method :

public function prepareObject(TypeInterface $type, $parent = null)
    {
        if (null == $parent) {
            throw new RuntimeException('You need a parent to create new objects');
        }

        // Create object
        list($prefix, $shortname) = explode(':', $type->getRdfType());
        $ns = $type->getVocabularies();
        $ns = $ns[$prefix];
        $name = $ns.$shortname;
        if (isset($this->typeMap[$name])) {
            $class = $this->typeMap[$name];
            $instance = new $class;

            // Retrieve ID
            $meta = $this->om->getClassMetaData(get_class($parent));
            $ids = $meta->getIdentifierValues($parent);
            $id= $ids['id'];

            // Get parent
            $repository = $this->om->getRepository(get_class($parent));
            $parentInstance = $repository->find(array('id' => $id));

            // Do manipulation on object according to its class
            switch($class) {
                case 'MyApp\Common\ContentBundle\Entity\ActualitesPageSectionLine' : $instance->addActualitesPageSection($parentInstance); break;
            }

            return $instance;
        }
    }

As you can see I'm setting the parent of my child, using the method appropriate for my class (see switch case). I hope this helps, do no hesitate to comment my discoveries.
I'll be happy to help if I can.

On a different matter, I need to add delete button. I'm guessing there is currently no delete mechanism in createjs ?

Cheers,

@dbu
Copy link
Member

dbu commented Aug 4, 2014

very interesting, glad you got so far. the parent handling is probably tricky with the orm. with phpcr-odm, its part of the very design principle that data is in a hierarchy... i am surprised what you need to do for the parent: is it a new object that is not loaded from the orm? that feels strange - it should be loaded by RestService at $parent = $this->_mapper->getBySubject($this->jsonldDecode(current($about))); and that mapper is the orm mapper...
i think what we could do for the orm mapper is configuring it with a map of classname => parent setter method and then dynamically call that method. if the getBySubject works as expected, that should be all the code that is missing to support the orm.

for the rel/rev part: we should make sure to validate this is properly specified in the mapping and then automatically update it in the attributes so it works as expected. and at the same time validate the input in this RestHandler file where you had undefined index to report that the frontend sent an invalid request.

do you want to try a pull request to add collection support for the orm and to cleanup the mapping?

about delete: @uwej711 implemented that, but for now its missing documentation: symfony-cmf/symfony-cmf-docs#441 - the code was added in #91. if you can figure out from that how it works and document it, that would be great!

@n0-m4d
Copy link

n0-m4d commented Apr 15, 2015

hi!

i was having the same troubles tom-s had and followed his steps to get things running.

when it came to extending the DoctrineOrmMapper I wrote a more generic solution. surely not perfect, feel free to improve and harden

namespace Bwlp\Core\CreateBundle\Mapper;

use Midgard\CreatePHP\Mapper\DoctrineOrmMapper;
use Midgard\CreatePHP\Type\TypeInterface;
use \RuntimeException;

/**
 * Mapper to handle Doctrine ORM.
 */
class BwlpDoctrineOrmMapper extends DoctrineOrmMapper
{
    public function prepareObject(TypeInterface $type, $parent = null)
    {
        $object = parent::prepareObject($type);

        if (null == $parent)
        {
            throw new RuntimeException('You need a parent to create new objects');
        }

        /** @var \Doctrine\ORM\Mapping\ClassMetadata $meta */
        $meta = $this->om->getClassMetaData(get_class($object));

        $parentSet = false;
        foreach ($meta->associationMappings as $mapping)
        {
            if ($mapping['targetEntity'] == get_class($parent))
            {
                $parentSet = true;
                $meta->setFieldValue($object, $mapping['fieldName'], $parent);
            }
        }

        if (!$parentSet)
        {
            throw new RuntimeException('parent could not be found/set for '
                . get_class($object));
        }

        return $object;
    }
}

@dbu
Copy link
Member

dbu commented Apr 24, 2015

very interesting idea! do you want to contribute this to https://github.com/flack/createphp ?

instead of the string comparison to see if the target could be the parent, you might want to use is_subclass_of so that more complex schemes work as well. if you don't want that, you should use the get real classname functionality of doctrine to not stumble when $parent is a proxy of the real parent class - and you could use the method ClassMetadata::getAssociationsByTargetClass and pick the first in that list.

instead of having $parentSet = true you could just return $object; if you find the matching field. that would simplify the code a bit.

i think i would move the logic to find the parent into a protected method, so that users that have more complicated cases can extend the DoctrineOrmMapper and overwrite that logic (e.g. to hardcode special cases where the logic does not work because there are other references to the same class.)

@n0-m4d
Copy link

n0-m4d commented Apr 24, 2015

using is_subclass_of might not be a good idea because $parent is not a parent (class) in terms of inheritance. $parent is the object which holds a collection of other objects - these are related but there is no inheritance. but maybe i got you wrong... did i?

@n0-m4d
Copy link

n0-m4d commented Apr 24, 2015

and yes, i would like to contribute but before i have to check out how to accomplish that... never done it before.

@dbu
Copy link
Member

dbu commented Apr 24, 2015

i meant is_subclass_of(get_class($parent), $mapping['targetEntity'])) - the method is instanceof for class names rather than class instances.

to create the pull request, you click the fork button in the project https://github.com/flack/createphp and fork it to your account. then you clone that repository locally and create a new branch (something like mapper-doctrine-orm-parent) and edit the file until it does what you want. then you commit and push and when you go to the github page of your fork, you should be offered to create a pull request. there you describe what the change wants to achieve. don't hesistate to ask if you need further help.

@n0-m4d
Copy link

n0-m4d commented Apr 24, 2015

thanks. meanwhile i went through the steps you desribed...

i modified my implementation to not stumble upon proxies

    public function prepareObject(TypeInterface $type, $parent = null)
    {
        $object = parent::prepareObject($type);

        if (null == $parent)
        {
            throw new RuntimeException('You need a parent to create new objects');
        }

        /** @var \Doctrine\ORM\Mapping\ClassMetadata $metaData */
        $metaData = $this->om->getClassMetaData(get_class($object));
        $metaDataParent = $this->om->getClassMetaData(get_class($parent));

        foreach ($metaData->associationMappings as $mapping)
        {
            if ($mapping['targetEntity'] == $metaDataParent->getName())
            {
                $metaData->setFieldValue($object, $mapping['fieldName'], $parent);
                return $object;
            }
        }

        throw new RuntimeException('parent could not be found/set for '
                . get_class($object));
    }

on my pull request page (n0-m4d/createphp#1) i have a button "merge pull request"... can you explain what it does?

@dbu
Copy link
Member

dbu commented Apr 24, 2015

ups, almost :-) you want to create the pull request against flack/createphp, not against your own fork. you can create the pull request at this url: openpsa/createphp@master...bitladen-nw:bitladen-nw-patch-1

i added you one line of feedback to your pull request, maybe you can adjust that before creating the other PR. once it exists, you can still add more commits to your branch when you get feedback.

@n0-m4d
Copy link

n0-m4d commented Apr 27, 2015

ha, next try for a pull request... did i do it right this time? ;)

@dbu
Copy link
Member

dbu commented Apr 27, 2015 via email

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

No branches or pull requests

3 participants