[WIP] Support for move and remove #23

dantleech · 2016-06-02T09:49:29Z

Takes over from #17

Adds move and remove operations.

It builds on the PR of @ElectricMaxxx and:

Removes the add method (we can perhaps add it later if it makes sense).
Allows multiple nodes to be moved / deleted.

dantleech · 2016-06-02T09:51:24Z

Repository/PhpcrRepository.php

@@ -102,6 +107,33 @@ public function getTags()
    /**
     * {@inheritdoc}
     */
+    public function add($path, $resource)


dantleech · 2016-06-02T12:28:49Z

Repository/AbstractPhpcrRepository.php

+        // if the query does not contain a glob pattern, then it applies to an
+        // explicit node and we should throw an exception if it is not found.
+        if (false === $this->globHelper->isGlobbed($query) && 0 === count($nodes)) {
+            throw new ResourceNotFoundException($query);


Not sure if we should throw an exception if an explicit node was specified but not found (as here) or always throw an exception if zero nodes are found. /cc @wouterj @webmozart

Actually meant this as a coment on move -- I see that for remove the number of items is returned. We can simply return the number of nodes found by the finder.

dantleech · 2016-06-02T12:50:42Z

Repository/PhpcrRepository.php

+        $this->session->save();
+    }
+
+    private function doMoveNodes($nodes, $sourceQuery, $targetPath)


Should be safe to type-hint on array here as the finder seems to only return arrays of things.

dantleech · 2016-06-02T13:30:53Z

@wouterj @ElectricMaxxx I am reasonably happy with this, so feel free to review. I will move onto the ResourceBundle now.

ElectricMaxxx · 2016-06-02T14:56:46Z

Repository/Api/EditableRepository.php

+ *
+ * @author Maximilian Berghoff <[email protected]>
+ */
+interface EditableRepository extends EditableRepository


I think you should alias the second one.

Hmm, yeah. Somehow strange that that even works.

On Thu, Jun 02, 2016 at 07:56:46AM -0700, Maximilian Berghoff wrote:

In [1]Repository/Api/EditableRepository.php:

* file that was distributed with this source code.

/
+
+namespace Symfony\Cmf\Component\Resource\Repository\Api;
+
+use InvalidArgumentException;
+use Puli\Repository\Api\EditableRepository;
+use Puli\Repository\Api\UnsupportedLanguageException;
+
+/*

* Extends the Puli editable repository to implement the as-of-yet not

* implemented features.

* @author Maximilian Berghoff [email protected]

*/
+interface EditableRepository extends EditableRepository

I think you should alias the second one.

—
You are receiving this because you authored the thread.
Reply to this email directly, [2]view it on GitHub, or [3]mute the thread.

Reverse link: [4]unknown

References

Visible links

[WIP] Support for move and remove #23 (comment)

https://github.com/symfony-cmf/resource/pull/23/files/c12c5cf7b1de0cbf2bcb789fffc4610038aa5567#r65555686

https://github.com/notifications/unsubscribe/AAgZccdL1mCPtwD8BOrwyN4p1roz7Bxmks5qHu8ugaJpZM4IsYhH

https://github.com/symfony-cmf/resource/pull/23/files/c12c5cf7b1de0cbf2bcb789fffc4610038aa5567#r65555686

travis say it does not:

PHP Fatal error: Cannot declare class Symfony\Cmf\Component\Resource\Repository\Api\EditableRepository because the name is already in use in /home/travis/build/symfony-cmf/resource/Repository/Api/EditableRepository.php on line 24

It seems that PHP7 doesn't care about that. Fixed now for PHP5.

wouterj · 2016-06-03T08:54:34Z

Repository/PhpcrRepository.php

+
+    private function doMoveNodes(array $nodes, $sourceQuery, $targetPath)
+    {
+        if (false === $this->isGlobbed($sourceQuery)) {


~~if it's not globbed, wouldn't it not always result in $nodes containing just one element? In other words, why can't we simply loop through $nodes like done in removeNodes()?~~

Nvm, I spotted the difference now.

wouterj · 2016-06-03T10:22:42Z

How are we going to handle supported moves? E.g. some documents cannot have children or a Menu document cannot not Content childs, etc.

This has to be implemented both on the client side (this bundle?) and the TreeBrowserBundle (to indicate to the user if a drop is allowed)

dantleech · 2016-06-03T10:51:37Z

How are we going to handle supported moves?

We are working on this by introducing child restrictions to the PHPCR-ODM.

This would allow use to add descriptors to describe valid children types and also links to create new children.

We may later want to add more control at the tree-browser level, but for now I think that would be OK?

wouterj · 2016-06-06T22:28:07Z

The move target has to exists. This means that we can't move resources to a new node. Is this expected behaviour? I kinda think it is.

Implement move and remove

a938192

dantleech added the wip/poc label Jun 2, 2016

dantleech force-pushed the write_move_delete branch from d3353e3 to 2c7a994 Compare June 2, 2016 09:50

dantleech reviewed Jun 2, 2016
View reviewed changes

dantleech force-pushed the write_move_delete branch from 2c7a994 to 5dae52e Compare June 2, 2016 09:52

dantleech changed the title ~~Write move delete~~ [WIP Jun 2, 2016

dantleech changed the title ~~[WIP~~ [WIP] Support for move and remove Jun 2, 2016

dantleech mentioned this pull request Jun 2, 2016

Add, remove and move #17

Closed

4 tasks

dantleech force-pushed the write_move_delete branch from 5dae52e to 8a295bd Compare June 2, 2016 09:58

Refactored to support multiple nodes

10f0b7e

dantleech force-pushed the write_move_delete branch from 8a295bd to 10f0b7e Compare June 2, 2016 09:59

dantleech reviewed Jun 2, 2016
View reviewed changes

dantleech force-pushed the write_move_delete branch 3 times, most recently from a56e0db to eeff6f3 Compare June 2, 2016 12:46

dantleech reviewed Jun 2, 2016
View reviewed changes

dantleech force-pushed the write_move_delete branch 4 times, most recently from cbbbc78 to 202bdfd Compare June 2, 2016 13:29

dantleech force-pushed the write_move_delete branch from 202bdfd to 318a42e Compare June 2, 2016 13:31

Refactored tests

c12c5cf

dantleech force-pushed the write_move_delete branch from 318a42e to c12c5cf Compare June 2, 2016 14:50

ElectricMaxxx reviewed Jun 2, 2016
View reviewed changes

dantleech added 2 commits June 2, 2016 16:29

Take into account base path

05be4f2

Fixed tests for PHP5

6425f09

dantleech force-pushed the write_move_delete branch from b580149 to 6425f09 Compare June 2, 2016 16:06

wouterj reviewed Jun 3, 2016
View reviewed changes

wouterj mentioned this pull request Jun 5, 2016

Reintroduce Drag 'n Drop symfony-cmf/tree-browser-bundle#99

Merged

4 tasks

wouterj mentioned this pull request Jun 6, 2016

move, rename delete resource of editable repository symfony-cmf/resource-rest-bundle#36

Merged

2 tasks

dantleech merged commit 0265227 into master Jun 18, 2016

dantleech removed the wip/poc label Jun 18, 2016

dantleech deleted the write_move_delete branch June 18, 2016 10:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Support for move and remove #23

[WIP] Support for move and remove #23

dantleech commented Jun 2, 2016 •

edited

Loading

dantleech Jun 2, 2016

dantleech Jun 2, 2016

dantleech Jun 2, 2016

dantleech Jun 2, 2016

dantleech commented Jun 2, 2016

ElectricMaxxx Jun 2, 2016

dantleech Jun 2, 2016

ElectricMaxxx Jun 2, 2016 •

edited

Loading

dantleech Jun 2, 2016

wouterj Jun 3, 2016 •

edited

Loading

wouterj commented Jun 3, 2016 •

edited

Loading

dantleech commented Jun 3, 2016 •

edited

Loading

wouterj commented Jun 6, 2016

[WIP] Support for move and remove #23

[WIP] Support for move and remove #23

Conversation

dantleech commented Jun 2, 2016 • edited Loading

dantleech Jun 2, 2016

Choose a reason for hiding this comment

dantleech Jun 2, 2016

Choose a reason for hiding this comment

dantleech Jun 2, 2016

Choose a reason for hiding this comment

dantleech Jun 2, 2016

Choose a reason for hiding this comment

dantleech commented Jun 2, 2016

ElectricMaxxx Jun 2, 2016

Choose a reason for hiding this comment

dantleech Jun 2, 2016

Choose a reason for hiding this comment

ElectricMaxxx Jun 2, 2016 • edited Loading

Choose a reason for hiding this comment

dantleech Jun 2, 2016

Choose a reason for hiding this comment

wouterj Jun 3, 2016 • edited Loading

Choose a reason for hiding this comment

wouterj commented Jun 3, 2016 • edited Loading

dantleech commented Jun 3, 2016 • edited Loading

wouterj commented Jun 6, 2016

dantleech commented Jun 2, 2016 •

edited

Loading

ElectricMaxxx Jun 2, 2016 •

edited

Loading

wouterj Jun 3, 2016 •

edited

Loading

wouterj commented Jun 3, 2016 •

edited

Loading

dantleech commented Jun 3, 2016 •

edited

Loading