You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by hi...@gmail.com on 2012/02/14 21:17:48 UTC
Review Request: RAVE-260 Person profile controller
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3896/
-----------------------------------------------------------
Review request for rave.
Summary
-------
Controller for the person profile
Diffs
-----
trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1244037
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1244037
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/personProfile.jsp PRE-CREATION
trunk/rave-portal-resources/src/main/resources/messages.properties 1244037
trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1244037
trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/ProfileControllerTest.java 1244037
trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java 1244037
Diff: https://reviews.apache.org/r/3896/diff
Testing
-------
Thanks,
hilker.j
Re: Review Request: RAVE-457 Person profile controller
Posted by mf...@apache.org.
> On 2012-02-15 13:30:49, mfranklin wrote:
> > trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java, line 73
> > <https://reviews.apache.org/r/3896/diff/2/?file=74931#file74931line73>
> >
> > Why not use addAttributesToModel helper?
>
> hilker.j wrote:
> The helper additionally contains the referring_page_id. I felt for querying people, the entry point would either be manually modifying the URL, a link, or a search. In some of these cases you would not have a referring page id to pass, and not including a referring page id appears to 404 the page in the current set up. Perhaps we need to look into a new way to handle saving the tab a person was using while entering menus (The current phonebook does it well)
I think we should either re-examine the current approach for tracking a users visited page or make it so this method follows the current convention. If you want to make it an optional parameter use @RequestParameter(optional=true)
- mfranklin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3896/#review5121
-----------------------------------------------------------
On 2012-02-14 20:41:20, hilker.j wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3896/
> -----------------------------------------------------------
>
> (Updated 2012-02-14 20:41:20)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> Controller for the person profile
>
>
> Diffs
> -----
>
> trunk/rave-portal-resources/src/main/resources/messages.properties 1244037
> trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1244037
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1244037
> trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/ProfileControllerTest.java 1244037
> trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1244037
> trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java 1244037
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/personProfile.jsp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3896/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> hilker.j
>
>
Re: Review Request: RAVE-457 Person profile controller
Posted by hi...@gmail.com.
> On 2012-02-15 13:30:49, mfranklin wrote:
> > trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java, line 73
> > <https://reviews.apache.org/r/3896/diff/2/?file=74931#file74931line73>
> >
> > Why not use addAttributesToModel helper?
The helper additionally contains the referring_page_id. I felt for querying people, the entry point would either be manually modifying the URL, a link, or a search. In some of these cases you would not have a referring page id to pass, and not including a referring page id appears to 404 the page in the current set up. Perhaps we need to look into a new way to handle saving the tab a person was using while entering menus (The current phonebook does it well)
> On 2012-02-15 13:30:49, mfranklin wrote:
> > trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java, line 67
> > <https://reviews.apache.org/r/3896/diff/2/?file=74931#file74931line67>
> >
> > small nit, but are you really setting up a form or are you rendering a profile?
The previous profile auto made a form, the function name lingered through, I've now modified to setUpPersonProfile
> On 2012-02-15 13:30:49, mfranklin wrote:
> > trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp, line 44
> > <https://reviews.apache.org/r/3896/diff/2/?file=74936#file74936line44>
> >
> > Why do we have to similar tests that output the same result?
The idea was to include the profile page view of any users along side the exists self view page, at least for the time being. This led me to create a second set of constants in the message.properties, requiring the second (nearly identical) test. We could choose use the same constants for both pages. But the initial plan was do do it entirely along side until the point where we can phase out the current self-only profile page in favor of one that can view any user
- hilker.j
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3896/#review5121
-----------------------------------------------------------
On 2012-02-14 20:41:20, hilker.j wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3896/
> -----------------------------------------------------------
>
> (Updated 2012-02-14 20:41:20)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> Controller for the person profile
>
>
> Diffs
> -----
>
> trunk/rave-portal-resources/src/main/resources/messages.properties 1244037
> trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1244037
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1244037
> trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/ProfileControllerTest.java 1244037
> trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1244037
> trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java 1244037
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/personProfile.jsp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3896/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> hilker.j
>
>
Re: Review Request: RAVE-457 Person profile controller
Posted by mf...@apache.org.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3896/#review5121
-----------------------------------------------------------
trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java
<https://reviews.apache.org/r/3896/#comment11220>
small nit, but are you really setting up a form or are you rendering a profile?
trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java
<https://reviews.apache.org/r/3896/#comment11221>
Why not use addAttributesToModel helper?
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp
<https://reviews.apache.org/r/3896/#comment11222>
Why do we have to similar tests that output the same result?
- mfranklin
On 2012-02-14 20:41:20, hilker.j wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3896/
> -----------------------------------------------------------
>
> (Updated 2012-02-14 20:41:20)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> Controller for the person profile
>
>
> Diffs
> -----
>
> trunk/rave-portal-resources/src/main/resources/messages.properties 1244037
> trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1244037
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1244037
> trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/ProfileControllerTest.java 1244037
> trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1244037
> trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java 1244037
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/personProfile.jsp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3896/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> hilker.j
>
>
Re: Review Request: RAVE-457 Person profile controller
Posted by mf...@apache.org.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3896/#review5171
-----------------------------------------------------------
Ship it!
Committed
- mfranklin
On 2012-02-16 19:51:57, hilker.j wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3896/
> -----------------------------------------------------------
>
> (Updated 2012-02-16 19:51:57)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> Controller for the person profile
>
>
> Diffs
> -----
>
> trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java 1244973
> trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1244973
> trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/ProfileControllerTest.java 1244973
> trunk/rave-portal-resources/src/main/resources/messages.properties 1244973
> trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1244973
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1244973
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/personProfile.jsp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3896/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> hilker.j
>
>
Re: Review Request: RAVE-457 Person profile controller
Posted by hi...@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3896/
-----------------------------------------------------------
(Updated 2012-02-16 19:51:57.499890)
Review request for rave.
Changes
-------
Updated to handle periods in usernames
Summary
-------
Controller for the person profile
Diffs (updated)
-----
trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java 1244973
trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1244973
trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/ProfileControllerTest.java 1244973
trunk/rave-portal-resources/src/main/resources/messages.properties 1244973
trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1244973
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1244973
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/personProfile.jsp PRE-CREATION
Diff: https://reviews.apache.org/r/3896/diff
Testing
-------
Thanks,
hilker.j
Re: Review Request: RAVE-457 Person profile controller
Posted by mf...@apache.org.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3896/#review5163
-----------------------------------------------------------
Ship it!
I will apply this today
- mfranklin
On 2012-02-15 18:58:14, hilker.j wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3896/
> -----------------------------------------------------------
>
> (Updated 2012-02-15 18:58:14)
>
>
> Review request for rave.
>
>
> Summary
> -------
>
> Controller for the person profile
>
>
> Diffs
> -----
>
> trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java 1244628
> trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1244628
> trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/ProfileControllerTest.java 1244628
> trunk/rave-portal-resources/src/main/resources/messages.properties 1244628
> trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1244628
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1244628
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/personProfile.jsp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/3896/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> hilker.j
>
>
Re: Review Request: RAVE-457 Person profile controller
Posted by hi...@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3896/
-----------------------------------------------------------
(Updated 2012-02-15 18:58:14.110423)
Review request for rave.
Summary
-------
Controller for the person profile
Diffs (updated)
-----
trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java 1244628
trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1244628
trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/ProfileControllerTest.java 1244628
trunk/rave-portal-resources/src/main/resources/messages.properties 1244628
trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1244628
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1244628
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/personProfile.jsp PRE-CREATION
Diff: https://reviews.apache.org/r/3896/diff
Testing
-------
Thanks,
hilker.j
Re: Review Request: RAVE-457 Person profile controller
Posted by hi...@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3896/
-----------------------------------------------------------
(Updated 2012-02-15 16:48:01.119328)
Review request for rave.
Summary
-------
Controller for the person profile
Diffs (updated)
-----
trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java 1244037
trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1244037
trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/ProfileControllerTest.java 1244037
trunk/rave-portal-resources/src/main/resources/messages.properties 1244037
trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1244037
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1244037
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/personProfile.jsp PRE-CREATION
Diff: https://reviews.apache.org/r/3896/diff
Testing
-------
Thanks,
hilker.j
Re: Review Request: RAVE-457 Person profile controller
Posted by hi...@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3896/
-----------------------------------------------------------
(Updated 2012-02-14 20:41:20.022140)
Review request for rave.
Changes
-------
Added a stub of a user page to follow description of task
Summary (updated)
-------
Controller for the person profile
Diffs (updated)
-----
trunk/rave-portal-resources/src/main/resources/messages.properties 1244037
trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1244037
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/templates/base_layout.jsp 1244037
trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/ProfileControllerTest.java 1244037
trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1244037
trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ProfileController.java 1244037
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/personProfile.jsp PRE-CREATION
Diff: https://reviews.apache.org/r/3896/diff
Testing
-------
Thanks,
hilker.j