You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by Stanton Sievers <si...@gmail.com> on 2014/02/18 17:24:18 UTC
Review Request 18216: Page clone API should return the cloned page
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18216/
-----------------------------------------------------------
Review request for rave.
Bugs: RAVE-1092
https://issues.apache.org/jira/browse/RAVE-1092
Repository: rave
Description
-------
When cloning a page, the PageApi and PageService will now return the Page object itself.
Diffs
-----
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/PageService.java 1561424
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1561424
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultPageServiceTest.java 1561424
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/PageApi.java 1561424
Diff: https://reviews.apache.org/r/18216/diff/
Testing
-------
Existing unit tests pass. Added a new unit test to sanity check the returned value.
Verified that page cloning works in the default portal and that the REST call returns the correct page object.
There's definitely more unit testing that needs to be done around the page cloning API.
Thanks,
Stanton Sievers
Re: Review Request 18216: Page clone API should return the cloned page
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18216/
-----------------------------------------------------------
(Updated Feb. 19, 2014, 5:11 p.m.)
Review request for rave.
Changes
-------
Avoiding infinite recursion when serializing a page with subpages. Ensuring a page's list of subpages is always non-null.
Bugs: RAVE-1092
https://issues.apache.org/jira/browse/RAVE-1092
Repository: rave
Description
-------
When cloning a page, the PageApi and PageService will now return the Page object itself.
Diffs (updated)
-----
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/impl/PageImpl.java 1569485
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/PageService.java 1569485
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1569485
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultPageServiceTest.java 1569485
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/PageApi.java 1569485
Diff: https://reviews.apache.org/r/18216/diff/
Testing
-------
Existing unit tests pass. Added a new unit test to sanity check the returned value.
Verified that page cloning works in the default portal and that the REST call returns the correct page object.
There's definitely more unit testing that needs to be done around the page cloning API.
Thanks,
Stanton Sievers
Re: Review Request 18216: Page clone API should return the cloned page
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18216/#review34863
-----------------------------------------------------------
I've found a problem with this patch that occurs when the cloned page has subpages. Infinite recursion occurs when serializing the Page through the page->subpages->parentPage reference. I'll need to determine how to fix that before this patch can be submitted.
I also think I found a problem when cloning the clone of a page with no subpages. I'll fix that one too.
- Stanton Sievers
On Feb. 19, 2014, 1:26 p.m., Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18216/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2014, 1:26 p.m.)
>
>
> Review request for rave.
>
>
> Bugs: RAVE-1092
> https://issues.apache.org/jira/browse/RAVE-1092
>
>
> Repository: rave
>
>
> Description
> -------
>
> When cloning a page, the PageApi and PageService will now return the Page object itself.
>
>
> Diffs
> -----
>
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/PageService.java 1569485
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1569485
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultPageServiceTest.java 1569485
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/PageApi.java 1569485
>
> Diff: https://reviews.apache.org/r/18216/diff/
>
>
> Testing
> -------
>
> Existing unit tests pass. Added a new unit test to sanity check the returned value.
>
> Verified that page cloning works in the default portal and that the REST call returns the correct page object.
>
> There's definitely more unit testing that needs to be done around the page cloning API.
>
>
> Thanks,
>
> Stanton Sievers
>
>
Re: Review Request 18216: Page clone API should return the cloned page
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18216/
-----------------------------------------------------------
(Updated Feb. 19, 2014, 1:26 p.m.)
Review request for rave.
Changes
-------
Updated patch against trunk. Test now passes.
Bugs: RAVE-1092
https://issues.apache.org/jira/browse/RAVE-1092
Repository: rave
Description
-------
When cloning a page, the PageApi and PageService will now return the Page object itself.
Diffs (updated)
-----
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/PageService.java 1569485
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1569485
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultPageServiceTest.java 1569485
https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/PageApi.java 1569485
Diff: https://reviews.apache.org/r/18216/diff/
Testing
-------
Existing unit tests pass. Added a new unit test to sanity check the returned value.
Verified that page cloning works in the default portal and that the REST call returns the correct page object.
There's definitely more unit testing that needs to be done around the page cloning API.
Thanks,
Stanton Sievers
Re: Review Request 18216: Page clone API should return the cloned page
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18216/#review34761
-----------------------------------------------------------
This patch is no longer valid against trunk (the test doesn't pass). I'll be posting an updated patch shortly.
- Stanton Sievers
On Feb. 18, 2014, 4:24 p.m., Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18216/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2014, 4:24 p.m.)
>
>
> Review request for rave.
>
>
> Bugs: RAVE-1092
> https://issues.apache.org/jira/browse/RAVE-1092
>
>
> Repository: rave
>
>
> Description
> -------
>
> When cloning a page, the PageApi and PageService will now return the Page object itself.
>
>
> Diffs
> -----
>
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/PageService.java 1561424
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1561424
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultPageServiceTest.java 1561424
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/PageApi.java 1561424
>
> Diff: https://reviews.apache.org/r/18216/diff/
>
>
> Testing
> -------
>
> Existing unit tests pass. Added a new unit test to sanity check the returned value.
>
> Verified that page cloning works in the default portal and that the REST call returns the correct page object.
>
> There's definitely more unit testing that needs to be done around the page cloning API.
>
>
> Thanks,
>
> Stanton Sievers
>
>
Re: Review Request 18216: Page clone API should return the cloned page
Posted by Matt Franklin <mf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18216/#review34755
-----------------------------------------------------------
Ship it!
Ship It!
- Matt Franklin
On Feb. 18, 2014, 4:24 p.m., Stanton Sievers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18216/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2014, 4:24 p.m.)
>
>
> Review request for rave.
>
>
> Bugs: RAVE-1092
> https://issues.apache.org/jira/browse/RAVE-1092
>
>
> Repository: rave
>
>
> Description
> -------
>
> When cloning a page, the PageApi and PageService will now return the Page object itself.
>
>
> Diffs
> -----
>
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/PageService.java 1561424
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1561424
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultPageServiceTest.java 1561424
> https://svn.apache.org/repos/asf/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/PageApi.java 1561424
>
> Diff: https://reviews.apache.org/r/18216/diff/
>
>
> Testing
> -------
>
> Existing unit tests pass. Added a new unit test to sanity check the returned value.
>
> Verified that page cloning works in the default portal and that the REST call returns the correct page object.
>
> There's definitely more unit testing that needs to be done around the page cloning API.
>
>
> Thanks,
>
> Stanton Sievers
>
>