Merge branch 'feature/79', closes #79

pull/80/merge
Martin Zimmermann 10 years ago
commit 26cc7eb634

@ -2,8 +2,11 @@
import sqlite3 import sqlite3
import logging import logging
import operator
import os.path import os.path
from collections import defaultdict
logger = logging.getLogger("isso") logger = logging.getLogger("isso")
from isso.db.comments import Comments from isso.db.comments import Comments
@ -19,7 +22,7 @@ class SQLite3:
a trigger for automated orphan removal. a trigger for automated orphan removal.
""" """
MAX_VERSION = 2 MAX_VERSION = 3
def __init__(self, path, conf): def __init__(self, path, conf):
@ -89,3 +92,28 @@ class SQLite3:
con.execute('PRAGMA user_version = 2') con.execute('PRAGMA user_version = 2')
logger.info("%i rows changed", con.total_changes) logger.info("%i rows changed", con.total_changes)
# limit max. nesting level to 1
if self.version == 2:
first = lambda rv: list(map(operator.itemgetter(0), rv))
with sqlite3.connect(self.path) as con:
top = first(con.execute("SELECT id FROM comments WHERE parent IS NULL").fetchall())
flattened = defaultdict(set)
for id in top:
ids = [id, ]
while ids:
rv = first(con.execute("SELECT id FROM comments WHERE parent=?", (ids.pop(), )))
ids.extend(rv)
flattened[id].update(set(rv))
for id in flattened.keys():
for n in flattened[id]:
con.execute("UPDATE comments SET parent=? WHERE id=?", (id, n))
con.execute('PRAGMA user_version = 3')
logger.info("%i rows changed", con.total_changes)

@ -37,6 +37,12 @@ class Comments:
Add new comment to DB and return a mapping of :attribute:`fields` and Add new comment to DB and return a mapping of :attribute:`fields` and
database values. database values.
""" """
if c.get("parent") is not None:
ref = self.get(c["parent"])
if ref.get("parent") is not None:
c["parent"] = ref["parent"]
self.db.execute([ self.db.execute([
'INSERT INTO comments (', 'INSERT INTO comments (',
' tid, parent,' ' tid, parent,'

@ -75,7 +75,6 @@ define(["app/text/html", "app/dom", "app/utils", "app/config", "app/api", "app/m
if (parent !== null) { if (parent !== null) {
el.onsuccess(); el.onsuccess();
el.remove();
} }
}); });
}); });
@ -85,16 +84,8 @@ define(["app/text/html", "app/dom", "app/utils", "app/config", "app/api", "app/m
return el; return el;
}; };
// lookup table for responses (to link to the parent)
var map = {id: {}, name: {}};
var insert = function(comment, scrollIntoView) { var insert = function(comment, scrollIntoView) {
map.name[comment.id] = comment.author;
if (comment.parent) {
comment["replyto"] = map.name[comment.parent];
}
var el = $.htmlify(Mark.up(templates["comment"], comment)); var el = $.htmlify(Mark.up(templates["comment"], comment));
// update datetime every 60 seconds // update datetime every 60 seconds
@ -113,12 +104,7 @@ define(["app/text/html", "app/dom", "app/utils", "app/config", "app/api", "app/m
if (comment.parent === null) { if (comment.parent === null) {
entrypoint = $("#isso-root"); entrypoint = $("#isso-root");
} else { } else {
var key = comment.parent; entrypoint = $("#isso-" + comment.parent + " > .text-wrapper > .isso-follow-up");
while (key in map.id) {
key = map.id[key];
}
map.id[comment.id] = comment.parent;
entrypoint = $("#isso-" + key + " > .text-wrapper > .isso-follow-up");
} }
entrypoint.append(el); entrypoint.append(el);
@ -134,26 +120,17 @@ define(["app/text/html", "app/dom", "app/utils", "app/config", "app/api", "app/m
var form = null; // XXX: probably a good place for a closure var form = null; // XXX: probably a good place for a closure
$("a.reply", footer).toggle("click", $("a.reply", footer).toggle("click",
function(toggler) { function(toggler) {
form = footer.insertAfter(new Postbox(comment.id)); form = footer.insertAfter(new Postbox(comment.parent === null ? comment.id : comment.parent));
form.onsuccess = function() { toggler.next(); }; form.onsuccess = function() { toggler.next(); };
$(".textarea", form).focus(); $(".textarea", form).focus();
$("a.reply", footer).textContent = msgs["comment-close"]; $("a.reply", footer).textContent = msgs["comment-close"];
}, },
function() { function() {
form.remove(); form.remove()
$("a.reply", footer).textContent = msgs["comment-reply"]; $("a.reply", footer).textContent = msgs["comment-reply"];
} }
); );
if (comment.parent !== null) {
$("a.parent", header).on("mouseover", function() {
$("#isso-" + comment.parent).classList.add("parent-highlight");
});
$("a.parent", header).on("mouseout", function() {
$("#isso-" + comment.parent).classList.remove("parent-highlight");
});
}
// update vote counter, but hide if votes sum to 0 // update vote counter, but hide if votes sum to 0
var votes = function(value) { var votes = function(value) {
var span = $("span.votes", footer); var span = $("span.votes", footer);

@ -14,13 +14,6 @@
</span> </span>
{{ /if }} {{ /if }}
{{ if parent }}
<span class="spacer"></span>
<a class="parent" href="#isso-{{ parent }}">
<i>{{ svg-forward }}</i>{{ replyto | blank: `i18n-comment-anonymous` }}
</a>
{{ /if }}
<span class="spacer"></span> <span class="spacer"></span>
<a class="permalink" href="#isso-{{ id }}"> <a class="permalink" href="#isso-{{ id }}">

@ -1,6 +0,0 @@
<!-- Generator: IcoMoon.io --><svg width="10" height="10" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" fill="gray">
<g>
<path d="M 17.961,11.954L 17.961,2 L 32,16L 17.961,30L 17.961,19.958 C 10.826,19.958, 3.784,21.2,0,27.094 C 0.394,16.353, 8.43,13.796, 17.961,11.954z">
</path>
</g>
</svg>

Before

Width:  |  Height:  |  Size: 357 B

@ -1,6 +1,5 @@
define(["text!./forward.svg", "text!./arrow-down.svg", "text!./arrow-up.svg"], function (forward, arrdown, arrup) { define(["text!./arrow-down.svg", "text!./arrow-up.svg"], function (arrdown, arrup) {
return { return {
"forward": forward,
"arrow-down": arrdown, "arrow-down": arrdown,
"arrow-up": arrup "arrow-up": arrup
}; };

@ -105,6 +105,14 @@ class TestComments(unittest.TestCase):
rv = loads(r.data) rv = loads(r.data)
self.assertEqual(len(rv), 20) self.assertEqual(len(rv), 20)
def testCreateInvalidParent(self):
self.post('/new?uri=test', data=json.dumps({'text': '...'}))
self.post('/new?uri=test', data=json.dumps({'text': '...', 'parent': 1}))
invalid = self.post('/new?uri=test', data=json.dumps({'text': '...', 'parent': 2}))
self.assertEqual(loads(invalid.data)["parent"], 1)
def testVerifyFields(self): def testVerifyFields(self):
verify = lambda comment: comments.API.verify(comment)[0] verify = lambda comment: comments.API.verify(comment)[0]
@ -176,19 +184,16 @@ class TestComments(unittest.TestCase):
[ comment 1 ] [ comment 1 ]
| |
--- [ comment 2, ref 1 ] --- [ comment 2, ref 1 ]
| |
--- [ comment 3, ref 2 ] --- [ comment 3, ref 1 ]
| [ comment 4 ]
--- [ comment 4, ref 2 ]
[ comment 5 ]
""" """
client = JSONClient(self.app, Response) client = JSONClient(self.app, Response)
client.post('/new?uri=%2Fpath%2F', data=json.dumps({'text': 'First'})) client.post('/new?uri=%2Fpath%2F', data=json.dumps({'text': 'First'}))
client.post('/new?uri=%2Fpath%2F', data=json.dumps({'text': 'Second', 'parent': 1})) client.post('/new?uri=%2Fpath%2F', data=json.dumps({'text': 'Second', 'parent': 1}))
client.post('/new?uri=%2Fpath%2F', data=json.dumps({'text': 'Third 1', 'parent': 2})) client.post('/new?uri=%2Fpath%2F', data=json.dumps({'text': 'Third', 'parent': 1}))
client.post('/new?uri=%2Fpath%2F', data=json.dumps({'text': 'Third 2', 'parent': 2})) client.post('/new?uri=%2Fpath%2F', data=json.dumps({'text': 'Last'}))
client.post('/new?uri=%2Fpath%2F', data=json.dumps({'text': '...'}))
client.delete('/id/1') client.delete('/id/1')
self.assertEqual(self.get('/?uri=%2Fpath%2F').status_code, 200) self.assertEqual(self.get('/?uri=%2Fpath%2F').status_code, 200)
@ -197,8 +202,6 @@ class TestComments(unittest.TestCase):
client.delete('/id/3') client.delete('/id/3')
self.assertEqual(self.get('/?uri=%2Fpath%2F').status_code, 200) self.assertEqual(self.get('/?uri=%2Fpath%2F').status_code, 200)
client.delete('/id/4') client.delete('/id/4')
self.assertEqual(self.get('/?uri=%2Fpath%2F').status_code, 200)
client.delete('/id/5')
self.assertEqual(self.get('/?uri=%2Fpath%2F').status_code, 404) self.assertEqual(self.get('/?uri=%2Fpath%2F').status_code, 404)
def testPathVariations(self): def testPathVariations(self):

@ -11,6 +11,8 @@ import tempfile
from isso.db import SQLite3 from isso.db import SQLite3
from isso.core import Config from isso.core import Config
from isso.compat import iteritems
class TestDBMigration(unittest.TestCase): class TestDBMigration(unittest.TestCase):
@ -49,3 +51,56 @@ class TestDBMigration(unittest.TestCase):
self.assertEqual(db.version, SQLite3.MAX_VERSION) self.assertEqual(db.version, SQLite3.MAX_VERSION)
self.assertEqual(db.preferences.get("session-key"), self.assertEqual(db.preferences.get("session-key"),
"supersecretkey") "supersecretkey")
def test_limit_nested_comments(self):
tree = {
1: None,
2: None,
3: 2,
4: 3,
7: 3,
5: 2,
6: None
}
with sqlite3.connect(self.path) as con:
con.execute("PRAGMA user_version = 2")
con.execute("CREATE TABLE threads ("
" id INTEGER PRIMARY KEY,"
" uri VARCHAR UNIQUE,"
" title VARCHAR)")
con.execute("CREATE TABLE comments ("
" tid REFERENCES threads(id),"
" id INTEGER PRIMARY KEY,"
" parent INTEGER,"
" created FLOAT NOT NULL, modified FLOAT,"
" text VARCHAR, email VARCHAR, website VARCHAR,"
" mode INTEGER,"
" remote_addr VARCHAR,"
" likes INTEGER DEFAULT 0,"
" dislikes INTEGER DEFAULT 0,"
" voters BLOB)")
con.execute("INSERT INTO threads (uri, title) VALUES (?, ?)", ("/", "Test"))
for (id, parent) in iteritems(tree):
con.execute("INSERT INTO comments ("
" tid, parent, created)"
"VALUEs (?, ?, ?)", (id, parent, id))
conf = Config.load(None)
db = SQLite3(self.path, conf)
flattened = [
(1, None),
(2, None),
(3, 2),
(4, 2),
(5, 2),
(6, None),
(7, 2)
]
with sqlite3.connect(self.path) as con:
rv = con.execute("SELECT id, parent FROM comments ORDER BY created").fetchall()
self.assertEqual(flattened, rv)

Loading…
Cancel
Save