Review Request 111654: add support for the discourse

[Aaron J. Seigo]

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


why a clone of the discourse repo on github? was this to ensure that changes that happen in upstream discourse do not cause unexpected problems? or do you have actual changes in your github clone?

the installation procedure needs to be recorded in the README, but i suppose i can do that once this is in.

one this patch is in, there will need to modifications made to the asset info API in bodega-server to include a link to the discussion forum. that implies adding a new service block to the config.json for things like the url to the discourse server. 


sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26869>

    this line should go into the sql/superuserCommand.sql file. this can happen post-merge, however.



sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26862>

    the name of the database needs to come from configuration somewhere; as we can't use the config.json from the nodejs app, it will need to be stored in the database.
    
    i need this for other things as well, so will take care of this once it is committed.



sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26863>

    there should be an EXCEPTION block here to catch the case where the dblink fails.
    
    that way the exception can be dealt with and it won't disrupt any transactions that might be happening otherwise.
    
    right now, if the dblink fails to the discourse system, then updating the partner data will likely just fail.



sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26864>

    all new partners added will start at 1000, as the seq_partnerId sequence is set to 1000 in defaultData.sql. so this is not an issue, and this block should be commented back in.
    
    this check should also be moved to the very start of the method since none of this should run for such "system" partners.



sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26870>

    i believe there is a race condition here due to dblink being async between 2 databases (so the usual "a function is automatically run in a transaction" safety does not apply here).
    
    instead, add a "RETURNING id" to the "INSERT INTO categories.." query and fetch it from there.
    
    same thing with the topic insert below.



sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26871>

    the names are not guaranteed to be unique between assets.
    
    if two people have an asset named "Awesome" this will cause problems.
    
    i would recommend storing the category and topic id's from the discourse db in bodega db or alternatively (and this would probably be nicer) add a reliable id to the topics/categories in the the discourse db. (e.g. based on the asset's ID, which never changes)
    
    this could be done non-intrusively by adding another table to the discourse db where these "bodega" ids are stored, perhaps?



sql/discourse.plsql
<http://git.reviewboard.kde.org/r/111654/#comment26865>

    an exception block is also required here for the same reason.


- Aaron J. Seigo


On July 23, 2013, 10:20 a.m., Giorgos Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111654/
> -----------------------------------------------------------
> 
> (Updated July 23, 2013, 10:20 a.m.)
> 
> 
> Review request for Bodega.
> 
> 
> Description
> -------
> 
> This patch keeps in sync the DBs of the bodega-server and discourse.
> 
> It syncs,
> 
> a. the user accounts*
> b. for each asset that has a partner with id > 1000 it create a new forum which has a topic and a post
> 
> * there is a default account for the discourse
> username: forumadmin
> password: password
> 
> 
> Diffs
> -----
> 
>   server/bodegaDbHelper 273e34f 
>   sql/discourse.plsql PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/111654/diff/
> 
> 
> Testing
> -------
> 
> How to test it,
> 
> * git clone https://github.com/terietor/discourse.git
> * cd discourse
> * git checkout bodega
> * https://github.com/terietor/discourse/blob/master/docs/DEVELOPER-ADVANCED.md
> * cd bodega-server/server
> * make sql && make test-sql (your bodega user must be a superuser in order to enable the dblink extention)
> * then go back to discourse and do `bundle exec rails s`
> * go to localhost:3000
> 
> 
> Thanks,
> 
> Giorgos Tsiapaliokas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/active/attachments/20130723/069a1374/attachment.html>