You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by Paul Sharples <p....@bolton.ac.uk> on 2012/05/02 19:41:05 UTC
Re: Review Request: RAVE-103. Support shared spaces.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4843/
-----------------------------------------------------------
(Updated 2012-05-02 17:41:05.131682)
Review request for rave.
Changes
-------
Updated to reflect Jashas & Matts additions/comments. Reworked the page and pageUser models so that the pageRepository now contains/handles requests for both types(rather than have a separate pageUserRepository). The cascade rules have been set on page, so that it persists/removes pageUser objects. Finally added another css for tabs in the UI which are shared by you (i.e. the logged in user).
Summary
-------
RAVE-103. support shared spaces. I've submitted this patch here rather than commit the code directly, as the changes affect the UI and I wanted a request for comments type approach first. This is a page sharing patch which allows a user to share his/her page with other rave users, as well as also allowing the user to revoke page shares. A user who receives a shared page can opt to confirm the share (meaning the page will always appear in his/her tabbed page list or decline it (i.e I don't want this shared page). There's still more to improve on this, but the basic functionality is there. Note I have removed the render sequencing away from the page object into the new pageUser object. This is because with the possibility of having several users of a page, they all need to have their own page render sequencing. See RAVE-103.
Diffs (updated)
-----
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/PageRepository.java 1308947
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/Page.java 1306906
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/PageInvitationStatus.java PRE-CREATION
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/PageUser.java PRE-CREATION
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/impl/JpaPageRepository.java 1308947
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/security/impl/DefaultPagePermissionEvaluator.java 1306906
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/PageService.java 1306906
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1310534
/trunk/rave-components/rave-core/src/main/resources/META-INF/persistence.xml 1306906
/trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/model/PageTest.java 1306906
/trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/repository/impl/JpaPageRepositoryTest.java 1308947
/trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultPageServiceTest.java 1327947
/trunk/rave-components/rave-core/src/test/resources/test_data.sql 1308947
/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/PageApi.java 1306906
/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/UserApi.java PRE-CREATION
/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/PageController.java 1306906
/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1331453
/trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/PageControllerTest.java 1330044
/trunk/rave-portal-resources/src/main/resources/messages.properties 1330724
/trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1327926
/trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1327941
/trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp 1330737
/trunk/rave-portal-resources/src/main/webapp/css/default.css 1330727
/trunk/rave-portal-resources/src/main/webapp/script/rave_api.js 1306906
/trunk/rave-portal-resources/src/main/webapp/script/rave_layout.js 1330737
/trunk/rave-portal/src/test/resources/test-data.sql 1306906
Diff: https://reviews.apache.org/r/4843/diff
Testing
-------
Thanks,
Paul
Re: Review Request: RAVE-103. Support shared spaces.
Posted by Anthony Carlucci <ac...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4843/#review7642
-----------------------------------------------------------
Ship it!
Paul, see my comment re: Sub Page Rendering...other than the minor changes I'd say this feature can be added to Rave!
- Anthony
On 2012-05-02 17:41:05, Paul Sharples wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4843/
> -----------------------------------------------------------
>
> (Updated 2012-05-02 17:41:05)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> RAVE-103. support shared spaces. I've submitted this patch here rather than commit the code directly, as the changes affect the UI and I wanted a request for comments type approach first. This is a page sharing patch which allows a user to share his/her page with other rave users, as well as also allowing the user to revoke page shares. A user who receives a shared page can opt to confirm the share (meaning the page will always appear in his/her tabbed page list or decline it (i.e I don't want this shared page). There's still more to improve on this, but the basic functionality is there. Note I have removed the render sequencing away from the page object into the new pageUser object. This is because with the possibility of having several users of a page, they all need to have their own page render sequencing. See RAVE-103.
>
>
> Diffs
> -----
>
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/PageRepository.java 1308947
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/Page.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/PageInvitationStatus.java PRE-CREATION
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/PageUser.java PRE-CREATION
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/impl/JpaPageRepository.java 1308947
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/security/impl/DefaultPagePermissionEvaluator.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/PageService.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1310534
> /trunk/rave-components/rave-core/src/main/resources/META-INF/persistence.xml 1306906
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/model/PageTest.java 1306906
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/repository/impl/JpaPageRepositoryTest.java 1308947
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultPageServiceTest.java 1327947
> /trunk/rave-components/rave-core/src/test/resources/test_data.sql 1308947
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/PageApi.java 1306906
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/UserApi.java PRE-CREATION
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/PageController.java 1306906
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1331453
> /trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/PageControllerTest.java 1330044
> /trunk/rave-portal-resources/src/main/resources/messages.properties 1330724
> /trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1327926
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1327941
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp 1330737
> /trunk/rave-portal-resources/src/main/webapp/css/default.css 1330727
> /trunk/rave-portal-resources/src/main/webapp/script/rave_api.js 1306906
> /trunk/rave-portal-resources/src/main/webapp/script/rave_layout.js 1330737
> /trunk/rave-portal/src/test/resources/test-data.sql 1306906
>
> Diff: https://reviews.apache.org/r/4843/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Paul
>
>
Re: Review Request: RAVE-103. Support shared spaces.
Posted by Anthony Carlucci <ac...@mitre.org>.
> On 2012-05-04 17:41:36, Anthony Carlucci wrote:
> > /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/impl/JpaPageRepository.java, line 161
> > <https://reviews.apache.org/r/4843/diff/3/?file=105971#file105971line161>
> >
> > The renderSequence for the sub-pages are getting lost (they are all getting set to 0). The sub-page render sequences should be set to the values from the page template.
>
> Paul Sharples wrote:
> When I query the Database I see that in the page_template table the 'person_profile' page has a render sequnce of 0, followed by the first sub-page 'About' (render sequence of 0) and followed by the 'My activity' sub-page (with a render sequence of 1). When I look in the page_user table, the created records have the same sequencing - person_profile(0), followed by 'About' (0), followed by 'My activity' (1). The renderSequcing is added to the pageUser object at line 170.
>
> PageUser pageUser = new PageUser(lPage.getOwner(), lPage, pt.getRenderSequence());
>
> Is this not the expected behaviour, or have I missed something?
>
> Paul Sharples wrote:
> Can you clarify if this is the expected behaviour or not?
>
> thanks
Sorry Paul, apparently I never committed my comment for this line - yes, in short this is the expected behaviour. I went back and re-applied the patch again and the render sequencing worked for the sub pages. I must have messed up some of the merging when I applied the patch the first time around. This looks good to go!
- Anthony
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4843/#review7575
-----------------------------------------------------------
On 2012-05-02 17:41:05, Paul Sharples wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4843/
> -----------------------------------------------------------
>
> (Updated 2012-05-02 17:41:05)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> RAVE-103. support shared spaces. I've submitted this patch here rather than commit the code directly, as the changes affect the UI and I wanted a request for comments type approach first. This is a page sharing patch which allows a user to share his/her page with other rave users, as well as also allowing the user to revoke page shares. A user who receives a shared page can opt to confirm the share (meaning the page will always appear in his/her tabbed page list or decline it (i.e I don't want this shared page). There's still more to improve on this, but the basic functionality is there. Note I have removed the render sequencing away from the page object into the new pageUser object. This is because with the possibility of having several users of a page, they all need to have their own page render sequencing. See RAVE-103.
>
>
> Diffs
> -----
>
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/PageRepository.java 1308947
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/Page.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/PageInvitationStatus.java PRE-CREATION
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/PageUser.java PRE-CREATION
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/impl/JpaPageRepository.java 1308947
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/security/impl/DefaultPagePermissionEvaluator.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/PageService.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1310534
> /trunk/rave-components/rave-core/src/main/resources/META-INF/persistence.xml 1306906
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/model/PageTest.java 1306906
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/repository/impl/JpaPageRepositoryTest.java 1308947
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultPageServiceTest.java 1327947
> /trunk/rave-components/rave-core/src/test/resources/test_data.sql 1308947
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/PageApi.java 1306906
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/UserApi.java PRE-CREATION
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/PageController.java 1306906
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1331453
> /trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/PageControllerTest.java 1330044
> /trunk/rave-portal-resources/src/main/resources/messages.properties 1330724
> /trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1327926
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1327941
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp 1330737
> /trunk/rave-portal-resources/src/main/webapp/css/default.css 1330727
> /trunk/rave-portal-resources/src/main/webapp/script/rave_api.js 1306906
> /trunk/rave-portal-resources/src/main/webapp/script/rave_layout.js 1330737
> /trunk/rave-portal/src/test/resources/test-data.sql 1306906
>
> Diff: https://reviews.apache.org/r/4843/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Paul
>
>
Re: Review Request: RAVE-103. Support shared spaces.
Posted by Paul Sharples <p....@bolton.ac.uk>.
> On 2012-05-04 17:41:36, Anthony Carlucci wrote:
> > /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/PageController.java, line 77
> > <https://reviews.apache.org/r/4843/diff/3/?file=105982#file105982line77>
> >
> > For efficiency the userService.getAuthenticatedUser() method only needs to be called once before the loop
Agreed, easy fix
> On 2012-05-04 17:41:36, Anthony Carlucci wrote:
> > /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/PageController.java, line 95
> > <https://reviews.apache.org/r/4843/diff/3/?file=105982#file105982line95>
> >
> > For efficiency the userService.getAuthenticatedUser() method only needs to be called once before the loop
Agreed, easy fix
> On 2012-05-04 17:41:36, Anthony Carlucci wrote:
> > /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java, line 31
> > <https://reviews.apache.org/r/4843/diff/3/?file=105983#file105983line31>
> >
> > minor nit - if this represents a list please make both the constant and value plural (PAGE_USERS / pageUsers)
The comment wasn't quite correct (my bad), it is the current pageUser, so I'll change the comment and leave the values as is.
> On 2012-05-04 17:41:36, Anthony Carlucci wrote:
> > /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/impl/JpaPageRepository.java, line 161
> > <https://reviews.apache.org/r/4843/diff/3/?file=105971#file105971line161>
> >
> > The renderSequence for the sub-pages are getting lost (they are all getting set to 0). The sub-page render sequences should be set to the values from the page template.
When I query the Database I see that in the page_template table the 'person_profile' page has a render sequnce of 0, followed by the first sub-page 'About' (render sequence of 0) and followed by the 'My activity' sub-page (with a render sequence of 1). When I look in the page_user table, the created records have the same sequencing - person_profile(0), followed by 'About' (0), followed by 'My activity' (1). The renderSequcing is added to the pageUser object at line 170.
PageUser pageUser = new PageUser(lPage.getOwner(), lPage, pt.getRenderSequence());
Is this not the expected behaviour, or have I missed something?
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4843/#review7575
-----------------------------------------------------------
On 2012-05-02 17:41:05, Paul Sharples wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4843/
> -----------------------------------------------------------
>
> (Updated 2012-05-02 17:41:05)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> RAVE-103. support shared spaces. I've submitted this patch here rather than commit the code directly, as the changes affect the UI and I wanted a request for comments type approach first. This is a page sharing patch which allows a user to share his/her page with other rave users, as well as also allowing the user to revoke page shares. A user who receives a shared page can opt to confirm the share (meaning the page will always appear in his/her tabbed page list or decline it (i.e I don't want this shared page). There's still more to improve on this, but the basic functionality is there. Note I have removed the render sequencing away from the page object into the new pageUser object. This is because with the possibility of having several users of a page, they all need to have their own page render sequencing. See RAVE-103.
>
>
> Diffs
> -----
>
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/PageRepository.java 1308947
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/Page.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/PageInvitationStatus.java PRE-CREATION
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/PageUser.java PRE-CREATION
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/impl/JpaPageRepository.java 1308947
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/security/impl/DefaultPagePermissionEvaluator.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/PageService.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1310534
> /trunk/rave-components/rave-core/src/main/resources/META-INF/persistence.xml 1306906
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/model/PageTest.java 1306906
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/repository/impl/JpaPageRepositoryTest.java 1308947
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultPageServiceTest.java 1327947
> /trunk/rave-components/rave-core/src/test/resources/test_data.sql 1308947
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/PageApi.java 1306906
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/UserApi.java PRE-CREATION
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/PageController.java 1306906
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1331453
> /trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/PageControllerTest.java 1330044
> /trunk/rave-portal-resources/src/main/resources/messages.properties 1330724
> /trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1327926
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1327941
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp 1330737
> /trunk/rave-portal-resources/src/main/webapp/css/default.css 1330727
> /trunk/rave-portal-resources/src/main/webapp/script/rave_api.js 1306906
> /trunk/rave-portal-resources/src/main/webapp/script/rave_layout.js 1330737
> /trunk/rave-portal/src/test/resources/test-data.sql 1306906
>
> Diff: https://reviews.apache.org/r/4843/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Paul
>
>
Re: Review Request: RAVE-103. Support shared spaces.
Posted by Paul Sharples <p....@bolton.ac.uk>.
> On 2012-05-04 17:41:36, Anthony Carlucci wrote:
> > /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/impl/JpaPageRepository.java, line 161
> > <https://reviews.apache.org/r/4843/diff/3/?file=105971#file105971line161>
> >
> > The renderSequence for the sub-pages are getting lost (they are all getting set to 0). The sub-page render sequences should be set to the values from the page template.
>
> Paul Sharples wrote:
> When I query the Database I see that in the page_template table the 'person_profile' page has a render sequnce of 0, followed by the first sub-page 'About' (render sequence of 0) and followed by the 'My activity' sub-page (with a render sequence of 1). When I look in the page_user table, the created records have the same sequencing - person_profile(0), followed by 'About' (0), followed by 'My activity' (1). The renderSequcing is added to the pageUser object at line 170.
>
> PageUser pageUser = new PageUser(lPage.getOwner(), lPage, pt.getRenderSequence());
>
> Is this not the expected behaviour, or have I missed something?
Can you clarify if this is the expected behaviour or not?
thanks
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4843/#review7575
-----------------------------------------------------------
On 2012-05-02 17:41:05, Paul Sharples wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4843/
> -----------------------------------------------------------
>
> (Updated 2012-05-02 17:41:05)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> RAVE-103. support shared spaces. I've submitted this patch here rather than commit the code directly, as the changes affect the UI and I wanted a request for comments type approach first. This is a page sharing patch which allows a user to share his/her page with other rave users, as well as also allowing the user to revoke page shares. A user who receives a shared page can opt to confirm the share (meaning the page will always appear in his/her tabbed page list or decline it (i.e I don't want this shared page). There's still more to improve on this, but the basic functionality is there. Note I have removed the render sequencing away from the page object into the new pageUser object. This is because with the possibility of having several users of a page, they all need to have their own page render sequencing. See RAVE-103.
>
>
> Diffs
> -----
>
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/PageRepository.java 1308947
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/Page.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/PageInvitationStatus.java PRE-CREATION
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/PageUser.java PRE-CREATION
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/impl/JpaPageRepository.java 1308947
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/security/impl/DefaultPagePermissionEvaluator.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/PageService.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1310534
> /trunk/rave-components/rave-core/src/main/resources/META-INF/persistence.xml 1306906
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/model/PageTest.java 1306906
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/repository/impl/JpaPageRepositoryTest.java 1308947
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultPageServiceTest.java 1327947
> /trunk/rave-components/rave-core/src/test/resources/test_data.sql 1308947
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/PageApi.java 1306906
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/UserApi.java PRE-CREATION
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/PageController.java 1306906
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1331453
> /trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/PageControllerTest.java 1330044
> /trunk/rave-portal-resources/src/main/resources/messages.properties 1330724
> /trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1327926
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1327941
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp 1330737
> /trunk/rave-portal-resources/src/main/webapp/css/default.css 1330727
> /trunk/rave-portal-resources/src/main/webapp/script/rave_api.js 1306906
> /trunk/rave-portal-resources/src/main/webapp/script/rave_layout.js 1330737
> /trunk/rave-portal/src/test/resources/test-data.sql 1306906
>
> Diff: https://reviews.apache.org/r/4843/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Paul
>
>
Re: Review Request: RAVE-103. Support shared spaces.
Posted by Anthony Carlucci <ac...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4843/#review7575
-----------------------------------------------------------
Paul, I finally got around to reviewing this as well. Overall this is a great step forward for the shared spaces, however please see my comments below regarding a few things.
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/impl/JpaPageRepository.java
<https://reviews.apache.org/r/4843/#comment16760>
The renderSequence for the sub-pages are getting lost (they are all getting set to 0). The sub-page render sequences should be set to the values from the page template.
/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/PageController.java
<https://reviews.apache.org/r/4843/#comment16761>
For efficiency the userService.getAuthenticatedUser() method only needs to be called once before the loop
/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/PageController.java
<https://reviews.apache.org/r/4843/#comment16762>
For efficiency the userService.getAuthenticatedUser() method only needs to be called once before the loop
/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java
<https://reviews.apache.org/r/4843/#comment16763>
minor nit - if this represents a list please make both the constant and value plural (PAGE_USERS / pageUsers)
- Anthony
On 2012-05-02 17:41:05, Paul Sharples wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4843/
> -----------------------------------------------------------
>
> (Updated 2012-05-02 17:41:05)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> RAVE-103. support shared spaces. I've submitted this patch here rather than commit the code directly, as the changes affect the UI and I wanted a request for comments type approach first. This is a page sharing patch which allows a user to share his/her page with other rave users, as well as also allowing the user to revoke page shares. A user who receives a shared page can opt to confirm the share (meaning the page will always appear in his/her tabbed page list or decline it (i.e I don't want this shared page). There's still more to improve on this, but the basic functionality is there. Note I have removed the render sequencing away from the page object into the new pageUser object. This is because with the possibility of having several users of a page, they all need to have their own page render sequencing. See RAVE-103.
>
>
> Diffs
> -----
>
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/PageRepository.java 1308947
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/Page.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/PageInvitationStatus.java PRE-CREATION
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/PageUser.java PRE-CREATION
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/impl/JpaPageRepository.java 1308947
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/security/impl/DefaultPagePermissionEvaluator.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/PageService.java 1306906
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1310534
> /trunk/rave-components/rave-core/src/main/resources/META-INF/persistence.xml 1306906
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/model/PageTest.java 1306906
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/repository/impl/JpaPageRepositoryTest.java 1308947
> /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultPageServiceTest.java 1327947
> /trunk/rave-components/rave-core/src/test/resources/test_data.sql 1308947
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/PageApi.java 1306906
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rpc/UserApi.java PRE-CREATION
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/PageController.java 1306906
> /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1331453
> /trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/PageControllerTest.java 1330044
> /trunk/rave-portal-resources/src/main/resources/messages.properties 1330724
> /trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1327926
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/db/initial_data.sql 1327941
> /trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/page.jsp 1330737
> /trunk/rave-portal-resources/src/main/webapp/css/default.css 1330727
> /trunk/rave-portal-resources/src/main/webapp/script/rave_api.js 1306906
> /trunk/rave-portal-resources/src/main/webapp/script/rave_layout.js 1330737
> /trunk/rave-portal/src/test/resources/test-data.sql 1306906
>
> Diff: https://reviews.apache.org/r/4843/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Paul
>
>