Review Request 111401: implement ratings in the bodega-server

Giorgos Tsiapaliokas terietor at gmail.com
Thu Jul 25 07:53:38 UTC 2013



> On July 16, 2013, 3:39 p.m., Aaron J. Seigo wrote:
> > sql/ratings.sql, line 28
> > <http://git.reviewboard.kde.org/r/111401/diff/2/?file=169131#file169131line28>
> >
> >     create indexes here on ratings(asset) and assetRatingAverages(asset)

for the ratings(asset) there is already one in line 20 `create index ratings_asset on ratings(asset);`.


> On July 16, 2013, 3:39 p.m., Aaron J. Seigo wrote:
> > server/lib/db/ratings.js, lines 195-219
> > <http://git.reviewboard.kde.org/r/111401/diff/2/?file=169126#file169126line195>
> >
> >     please use async.queue instead of this. you can see an example of this in server/test/partners.js among other places
> >     
> >     this code also assumes that the input is well formed. sending a rating attribute twice in the same json will result in it being entered twice.
> >     
> >     also, there is no check to see if the user has already added a rating for this item?
> >     
> >     it should probably be done something like:
> >     
> >     
> >     * get the rating attributes for the asset
> >     * iterate over all ratings passed in and make sure there is only one of each, and that each attribute matches the ratings for that asset
> >     * delete the ratings for that user on that asset for the rating attributes passed iin
> >     * insert the new rating entries

> * and that each attribute matches the ratings for that asset

I have added a new function in the in pg in order to make sure that we won't
screw up the database if we try to modify it directly. The same is also being
called from node.js


- Giorgos


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111401/#review36051
-----------------------------------------------------------


On July 5, 2013, 1:34 p.m., Giorgos Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111401/
> -----------------------------------------------------------
> 
> (Updated July 5, 2013, 1:34 p.m.)
> 
> 
> Review request for Bodega.
> 
> 
> Description
> -------
> 
> This patch implements the ratings feature in the bodega-server.
> 
> I have also attached a screenshot of the apidocs.
> 
> 
> Diffs
> -----
> 
>   server/bodegaDbHelper ba740cb 
>   server/doc/bodega.json 59c14e3 
>   server/lib/bodegadb.js 3be8d81 
>   server/lib/db/ratings.js PRE-CREATION 
>   server/lib/errors.js 356d7c4 
>   server/routes.js 726fc80 
>   server/test/ratings.js PRE-CREATION 
>   sql/core.plsql 7b92784 
>   sql/ratings.sql PRE-CREATION 
>   sql/testdata.sql dd15d57 
> 
> Diff: http://git.reviewboard.kde.org/r/111401/diff/
> 
> 
> Testing
> -------
> 
> ./node_modules/.bin/mocha test/ratings.js --reporter spec
> WARNING: Setting up server with no ssl!
> Bodega server listening on localhost:3000 in devel mode
> 
> 
>   Ratings
>     needs to authorize first
>       ? authorize correctly. (107ms)
>     List Attributes
>       ? it should fail because the asset is invalid 
>       ? it should succeed 
>     Remove asset rate
>       ? it should fail because the asset is invalid 
>       ? it should succeed 
>     Asset Ratings
>       ? it should fail because the asset is invalid 
>       ? it should succeed 
>       ? it should be empty because there are no ratings for the asset 
>     Participant
>       ? it should succeed 
>       ? it should have no ratings 
>     Add asset rate
>       ? it should fail because the asset is invalid 
>       ? it should succeed 
> 
> 
>   12 passing (219 ms)
> 
> 
> File Attachments
> ----------------
> 
> 
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/07/05/bodega-ratings-apidocs.png
> 
> 
> Thanks,
> 
> Giorgos Tsiapaliokas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/active/attachments/20130725/21dcb736/attachment.html>


More information about the Active mailing list