You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by Aaron Gooch <eo...@gmail.com> on 2012/01/26 19:17:21 UTC

Review Request: Rave-433 implementation of controller and jsps

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3641/
-----------------------------------------------------------

Review request for rave.


Summary
-------

Rave-433 implementation of controller and jsps


Diffs
-----

  trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/CategoryService.java 1236296 
  trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultCategoryService.java 1236296 
  trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultCategoryServiceTest.java 1236296 
  trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/AdminControllerUtil.java 1236296 
  trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/CategoryController.java PRE-CREATION 
  trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1236296 
  trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1236296 
  trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/admin/CategoryControllerTest.java PRE-CREATION 
  trunk/rave-portal-resources/src/main/resources/messages.properties 1236296 
  trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1236296 
  trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/category.jsp PRE-CREATION 
  trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categoryDetail.jsp PRE-CREATION 

Diff: https://reviews.apache.org/r/3641/diff


Testing
-------


Thanks,

Aaron

Re: Review Request: Rave-433 implementation of controller and jsps

Posted by Aaron Gooch <eo...@gmail.com>.
I have resubmitted the patch with the updates.
https://reviews.apache.org/r/3641/

Aaron

On Thu, Jan 26, 2012 at 4:52 PM, Marlon Pierce <mp...@cs.indiana.edu>wrote:

>    This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3641/
>
> On January 26th, 2012, 9:48 p.m., *Anthony Carlucci* wrote:
>
>
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/category.jsp<https://reviews.apache.org/r/3641/diff/1/?file=70854#file70854line56> (Diff
> revision 1)
>
>    56
>
>                             <input type="submit" value="Create the category"/>
>
>   Use a message bundle item here for the value instead of hard-coded text
>
>  For internationalization also.
>
>
> - Marlon
>
> On January 26th, 2012, 6:17 p.m., Aaron Gooch wrote:
>   Review request for rave.
> By Aaron Gooch.
>
> *Updated 2012-01-26 18:17:21*
> Description
>
> Rave-433 implementation of controller and jsps
>
>   Diffs
>
>    - trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/CategoryService.java
>    (1236296)
>    - trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultCategoryService.java
>    (1236296)
>    - trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultCategoryServiceTest.java
>    (1236296)
>    - trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/AdminControllerUtil.java
>    (1236296)
>    - trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/CategoryController.java
>    (PRE-CREATION)
>    - trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java
>    (1236296)
>    - trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java
>    (1236296)
>    - trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/admin/CategoryControllerTest.java
>    (PRE-CREATION)
>    - trunk/rave-portal-resources/src/main/resources/messages.properties
>    (1236296)
>    - trunk/rave-portal-resources/src/main/resources/messages_nl.properties
>    (1236296)
>    - trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/category.jsp
>    (PRE-CREATION)
>    - trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categoryDetail.jsp
>    (PRE-CREATION)
>
> View Diff <https://reviews.apache.org/r/3641/diff/>
>

Re: Review Request: Rave-433 implementation of controller and jsps

Posted by Marlon Pierce <mp...@cs.indiana.edu>.

> On 2012-01-26 21:48:31, Anthony Carlucci wrote:
> > trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/category.jsp, line 56
> > <https://reviews.apache.org/r/3641/diff/1/?file=70854#file70854line56>
> >
> >     Use a message bundle item here for the value instead of hard-coded text

For internationalization also.  


- Marlon


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3641/#review4632
-----------------------------------------------------------


On 2012-01-26 18:17:21, Aaron Gooch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3641/
> -----------------------------------------------------------
> 
> (Updated 2012-01-26 18:17:21)
> 
> 
> Review request for rave.
> 
> 
> Summary
> -------
> 
> Rave-433 implementation of controller and jsps
> 
> 
> Diffs
> -----
> 
>   trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/CategoryService.java 1236296 
>   trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultCategoryService.java 1236296 
>   trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultCategoryServiceTest.java 1236296 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/AdminControllerUtil.java 1236296 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/CategoryController.java PRE-CREATION 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1236296 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1236296 
>   trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/admin/CategoryControllerTest.java PRE-CREATION 
>   trunk/rave-portal-resources/src/main/resources/messages.properties 1236296 
>   trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1236296 
>   trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/category.jsp PRE-CREATION 
>   trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categoryDetail.jsp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3641/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Aaron
> 
>

Re: Review Request: Rave-433 implementation of controller and jsps

Posted by Anthony Carlucci <ac...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3641/#review4632
-----------------------------------------------------------


Hi Aaron, overall this looks good however there are a few minor changes I recommend...


trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java
<https://reviews.apache.org/r/3641/#comment10294>

    Refactor constant to ADMIN_CATEGORIES to keep naming convention the same



trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/admin/CategoryControllerTest.java
<https://reviews.apache.org/r/3641/#comment10295>

    Needs Apache license here



trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/category.jsp
<https://reviews.apache.org/r/3641/#comment10291>

    Use a message bundle item here for the value instead of hard-coded text



trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categoryDetail.jsp
<https://reviews.apache.org/r/3641/#comment10292>

    Use a message bundle property here instead of hard coded text



trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categoryDetail.jsp
<https://reviews.apache.org/r/3641/#comment10293>

    Use a message bundle property here instead of hard coded text


- Anthony


On 2012-01-26 18:17:21, Aaron Gooch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3641/
> -----------------------------------------------------------
> 
> (Updated 2012-01-26 18:17:21)
> 
> 
> Review request for rave.
> 
> 
> Summary
> -------
> 
> Rave-433 implementation of controller and jsps
> 
> 
> Diffs
> -----
> 
>   trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/CategoryService.java 1236296 
>   trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultCategoryService.java 1236296 
>   trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultCategoryServiceTest.java 1236296 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/AdminControllerUtil.java 1236296 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/CategoryController.java PRE-CREATION 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1236296 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1236296 
>   trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/admin/CategoryControllerTest.java PRE-CREATION 
>   trunk/rave-portal-resources/src/main/resources/messages.properties 1236296 
>   trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1236296 
>   trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/category.jsp PRE-CREATION 
>   trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categoryDetail.jsp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3641/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Aaron
> 
>

Re: Review Request: Rave-433 implementation of controller and jsps

Posted by Anthony Carlucci <ac...@mitre.org>.

> On 2012-01-27 16:40:42, Anthony Carlucci wrote:
> >

Aaron - ignore my last comment about category.jsp.  You changed it...it was diff reviewer user error :)


- Anthony


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3641/#review4651
-----------------------------------------------------------


On 2012-01-27 16:22:13, Aaron Gooch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3641/
> -----------------------------------------------------------
> 
> (Updated 2012-01-27 16:22:13)
> 
> 
> Review request for rave.
> 
> 
> Summary
> -------
> 
> Rave-433 implementation of controller and jsps
> 
> 
> Diffs
> -----
> 
>   trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/CategoryService.java 1236692 
>   trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultCategoryService.java 1236692 
>   trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultCategoryServiceTest.java 1236692 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/AdminControllerUtil.java 1236692 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/CategoryController.java PRE-CREATION 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1236721 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1236721 
>   trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/admin/CategoryControllerTest.java PRE-CREATION 
>   trunk/rave-portal-resources/src/main/resources/messages.properties 1236692 
>   trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1236692 
>   trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categories.jsp PRE-CREATION 
>   trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categoryDetail.jsp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3641/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Aaron
> 
>

Re: Review Request: Rave-433 implementation of controller and jsps

Posted by Anthony Carlucci <ac...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3641/#review4651
-----------------------------------------------------------

Ship it!


- Anthony


On 2012-01-27 16:22:13, Aaron Gooch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3641/
> -----------------------------------------------------------
> 
> (Updated 2012-01-27 16:22:13)
> 
> 
> Review request for rave.
> 
> 
> Summary
> -------
> 
> Rave-433 implementation of controller and jsps
> 
> 
> Diffs
> -----
> 
>   trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/CategoryService.java 1236692 
>   trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultCategoryService.java 1236692 
>   trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultCategoryServiceTest.java 1236692 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/AdminControllerUtil.java 1236692 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/CategoryController.java PRE-CREATION 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1236721 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1236721 
>   trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/admin/CategoryControllerTest.java PRE-CREATION 
>   trunk/rave-portal-resources/src/main/resources/messages.properties 1236692 
>   trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1236692 
>   trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categories.jsp PRE-CREATION 
>   trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categoryDetail.jsp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3641/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Aaron
> 
>

Re: Review Request: Rave-433 implementation of controller and jsps

Posted by Anthony Carlucci <ac...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3641/#review4650
-----------------------------------------------------------



trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/category.jsp
<https://reviews.apache.org/r/3641/#comment10373>

    This is still hardcode, but all others have been fixed.  I will change this to the message key when I apply the patch


- Anthony


On 2012-01-27 16:22:13, Aaron Gooch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3641/
> -----------------------------------------------------------
> 
> (Updated 2012-01-27 16:22:13)
> 
> 
> Review request for rave.
> 
> 
> Summary
> -------
> 
> Rave-433 implementation of controller and jsps
> 
> 
> Diffs
> -----
> 
>   trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/CategoryService.java 1236692 
>   trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultCategoryService.java 1236692 
>   trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultCategoryServiceTest.java 1236692 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/AdminControllerUtil.java 1236692 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/CategoryController.java PRE-CREATION 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1236721 
>   trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1236721 
>   trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/admin/CategoryControllerTest.java PRE-CREATION 
>   trunk/rave-portal-resources/src/main/resources/messages.properties 1236692 
>   trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1236692 
>   trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categories.jsp PRE-CREATION 
>   trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categoryDetail.jsp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3641/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Aaron
> 
>

Re: Review Request: Rave-433 implementation of controller and jsps

Posted by Aaron Gooch <eo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3641/
-----------------------------------------------------------

(Updated 2012-01-27 16:22:13.487352)


Review request for rave.


Changes
-------

Updated to include Tony's review findings and also added to international (Dutch) message bundle per Marlon. 

fixed naming convention in ViewNames
updated Controller and tests to reflect new naming convention
category.jsp is now categories.jsp
added apache license to test
updated categories.jsp and categoryDetail.jsp to use message bundle on the button clicks.


Summary
-------

Rave-433 implementation of controller and jsps


Diffs (updated)
-----

  trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/CategoryService.java 1236692 
  trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultCategoryService.java 1236692 
  trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultCategoryServiceTest.java 1236692 
  trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/AdminControllerUtil.java 1236692 
  trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/admin/CategoryController.java PRE-CREATION 
  trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java 1236721 
  trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java 1236721 
  trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/admin/CategoryControllerTest.java PRE-CREATION 
  trunk/rave-portal-resources/src/main/resources/messages.properties 1236692 
  trunk/rave-portal-resources/src/main/resources/messages_nl.properties 1236692 
  trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categories.jsp PRE-CREATION 
  trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/admin/categoryDetail.jsp PRE-CREATION 

Diff: https://reviews.apache.org/r/3641/diff


Testing
-------


Thanks,

Aaron