You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by ra...@apache.org on 2011/08/16 20:14:22 UTC
Review Request: Review RAVE 148 : Allow any logged in user to change his/her
layout
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1528/
-----------------------------------------------------------
Review request for rave, raminder and Jasha Joachimsthal.
Summary
-------
Review RAVE 148 : Allow any logged in user to change his/her layout
This addresses bug RAVE-148.
https://issues.apache.org/jira/browse/RAVE-148
Diffs
-----
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/User.java 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/UserProfile.java PRE-CREATION
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/PageService.java 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/RegionService.java 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/RegionWidgetService.java 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/UserService.java 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultRegionService.java 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultRegionWidgetService.java 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultUserService.java 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/UserProfileController.java 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/validator/NewAccountValidator.java 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/validator/UserProfileValidator.java 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/resources/messages.properties 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/webapp/WEB-INF/views/userProfile.jsp 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/webapp/script/rave_forms.js 1154667
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/service/UserServiceTest.java 1154667
Diff: https://reviews.apache.org/r/1528/diff
Testing
-------
No
Thanks,
raminder
Re: Review Request: Review RAVE 148 : Allow any logged in user to change
his/her layout
Posted by Marlon Pierce <mp...@cs.indiana.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1528/#review1485
-----------------------------------------------------------
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/User.java
<https://reviews.apache.org/r/1528/#comment3396>
This may not be useful yet. We have not checked OpenID login with any accounts other than the openID test account.
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/User.java
<https://reviews.apache.org/r/1528/#comment3397>
Good to delete this method. Must test password reset with hashing and salting
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/UserProfile.java
<https://reviews.apache.org/r/1528/#comment3398>
Do we need this separate class, or can we just use User?
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/UserProfile.java
<https://reviews.apache.org/r/1528/#comment3399>
Need to confirm that this works with password hashing and salting.
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultUserService.java
<https://reviews.apache.org/r/1528/#comment3416>
User "profile" here may be confused with OS Person. Profile here is prescriptive metadata for the rave-portal DB only (username, password, layout, etc).
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultUserService.java
<https://reviews.apache.org/r/1528/#comment3415>
Widgets can be distributed other ways. This is the simplest but may not be best from the user's point of view.
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/UserProfileController.java
<https://reviews.apache.org/r/1528/#comment3417>
Resolve this with the tabbed view now in SVN.
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/UserProfileController.java
<https://reviews.apache.org/r/1528/#comment3418>
Verify this works with hashed passwords.
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/UserProfileController.java
<https://reviews.apache.org/r/1528/#comment3419>
Obsolete comment.
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/UserProfileController.java
<https://reviews.apache.org/r/1528/#comment3420>
This is not the correct exception to throw. It occurs if you try to create a new user with the same username as an existing user.
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/validator/NewAccountValidator.java
<https://reviews.apache.org/r/1528/#comment3421>
"4" shouldn't be hard-coded. This should be in a property file.
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/validator/UserProfileValidator.java
<https://reviews.apache.org/r/1528/#comment3422>
See recent updates to other validator classes. Make this validator consistent with the others.
https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/resources/messages.properties
<https://reviews.apache.org/r/1528/#comment3423>
Should be "at least"
- Marlon
On 2011-08-16 18:14:22, Raminder Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1528/
> -----------------------------------------------------------
>
> (Updated 2011-08-16 18:14:22)
>
>
> Review request for rave, Raminder Singh and Jasha Joachimsthal.
>
>
> Summary
> -------
>
> Review RAVE 148 : Allow any logged in user to change his/her layout
>
>
> This addresses bug RAVE-148.
> https://issues.apache.org/jira/browse/RAVE-148
>
>
> Diffs
> -----
>
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/User.java 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/model/UserProfile.java PRE-CREATION
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/PageService.java 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/RegionService.java 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/RegionWidgetService.java 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/UserService.java 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultRegionService.java 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultRegionWidgetService.java 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultUserService.java 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/UserProfileController.java 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/validator/NewAccountValidator.java 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/validator/UserProfileValidator.java 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/resources/messages.properties 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/webapp/WEB-INF/views/userProfile.jsp 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/webapp/script/rave_forms.js 1154667
> https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/test/java/org/apache/rave/portal/service/UserServiceTest.java 1154667
>
> Diff: https://reviews.apache.org/r/1528/diff
>
>
> Testing
> -------
>
> No
>
>
> Thanks,
>
> Raminder
>
>