[
https://issues.apache.org/jira/browse/RAVE-71?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13125696#comment-13125696
]
[email protected] commented on RAVE-71:
---------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2338/#review2529
-----------------------------------------------------------
Thanks for the submitted patch. I have a few remarks.
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/Widget.java
<https://reviews.apache.org/r/2338/#comment5698>
Please add tests in JpaWidgetRepositoryTest that proves the correct
behaviour in CRUD operations instead of only mocking the result in the Service.
You probably need to set the Widget (this) to the new WidgetRating) to let
the OneToMany relation work properly. Add a method addRating(WidgetRating
rating) to add a single WidgetRating.
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/WidgetService.java
<https://reviews.apache.org/r/2338/#comment5699>
shouldn't the save and delete be "void"?
why are the input variables different between these methods?
/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rest/WidgetApi.java
<https://reviews.apache.org/r/2338/#comment5700>
This makes it possible to do a request for any userid. Shouldn't the user
id be retrieved from the request (current authenticated user)?
Why is widgetId a long primitive and userId a Long object?
/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rest/WidgetApi.java
<https://reviews.apache.org/r/2338/#comment5701>
This makes it possible to do a request for any userid. Shouldn't the user
id be retrieved from the request (current authenticated user)?
/trunk/rave-portal-resources/src/main/webapp/WEB-INF/views/widget.jsp
<https://reviews.apache.org/r/2338/#comment5702>
Please use message keys instead of hard coded text
/trunk/rave-portal-resources/src/main/webapp/WEB-INF/views/widget.jsp
<https://reviews.apache.org/r/2338/#comment5703>
Please move this inline script to an (existing) external scriptfile
- Jasha
On 2011-10-11 15:32:31, Sean Cooper wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2338/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-11 15:32:31)
bq.
bq.
bq. Review request for rave.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Partial Patch which allows users to rate widgets from the detailed widget
view.
bq.
bq. This patch does NOT include a UI to view the user's current rating for a
widget, nor does it include the UI for showing the rating totals for a given
widget.
bq.
bq. Questions for the team
bq. 1) Do we want to add the ability to rate widgets from the widget store
(non-detailed page)?
bq. 2) Do we want to add the ability to rate widgets from the widget menu on a
user's page?
bq. 3) Do we want to include totals of the ratings? (e.g. 314 Liked, 2014
Disliked)
bq. 3) How should we handle the display of the rating for the user?
bq. a) Iterate over object model to determine user's current rating and
totals - This will result in performance issues when a lot of users have
provided ratings for widgets.
bq. b) Create a statistics service to query the database directly and attach
this statistical information to the object model - This will result in a hard
coded SQL query to gather this information and requires attaching more
information to the web model, as well as requiring mapping the various widget
ratings to the widgets on the UI side.
bq. c) Suggest another approach
bq.
bq.
bq. This addresses bug RAVE-71.
bq. https://issues.apache.org/jira/browse/RAVE-71
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/Widget.java
1181810
bq.
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/WidgetService.java
1181810
bq.
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultWidgetService.java
1181810
bq.
/trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/model/WidgetTest.java
1181810
bq.
/trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/WidgetServiceTest.java
1181810
bq.
/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/api/rest/WidgetApi.java
1181810
bq.
/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreController.java
1181810
bq.
/trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/api/rest/WidgetApiTest.java
1181810
bq.
/trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/WidgetStoreControllerTest.java
1181810
bq. /trunk/rave-portal-resources/src/main/webapp/WEB-INF/views/widget.jsp
1181810
bq. /trunk/rave-portal-resources/src/main/webapp/script/rave_api.js 1181810
bq.
bq. Diff: https://reviews.apache.org/r/2338/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Sean
bq.
bq.
> Users can review and rate Widgets in the Widget Repository
> ----------------------------------------------------------
>
> Key: RAVE-71
> URL: https://issues.apache.org/jira/browse/RAVE-71
> Project: Rave
> Issue Type: Story
> Reporter: Scott Wilson
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
