Skip to content
This repository was archived by the owner on Apr 17, 2018. It is now read-only.

Postmovement #80

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
03bae0d
Moderators can now move comment threads between posts.
LucasSloan Jul 23, 2013
a95fa1a
Children of original movers now hidden for non-admins.
LucasSloan Jul 23, 2013
c7fc246
Made it impossible to vote on moved items.
LucasSloan Jul 23, 2013
2739bef
Status message removed in the event of failed call.
LucasSloan Jul 23, 2013
eefd50e
Entering an properly formatted URL that doesn't lead to a post sends …
LucasSloan Jul 23, 2013
f3721e5
Changed movebox doc string, renamed Link._byURL to Link._move_url.
LucasSloan Jul 24, 2013
afe23b1
Added a lock to movement.
LucasSloan Jul 24, 2013
582d9fd
Changed move proceedure to no longer create new comments. All calls …
LucasSloan Jul 27, 2013
ac9ed72
Removed debugging statements, moved boolean.
LucasSloan Jul 27, 2013
fa13a33
Safed movement against editor trying to move children of already move…
LucasSloan Jul 27, 2013
b7e7355
Merge branch 'interesting_comments' of https://github.com/PotatoDumpl…
LucasSloan Sep 21, 2013
91d0c4e
Descendant karma attribute no longer disappears off python objects.
LucasSloan Oct 1, 2013
bedd944
Merge branch 'interesting_comments' of https://github.com/PotatoDumpl…
LucasSloan Oct 1, 2013
cf10a31
Thread movement now properly increments descendant karma.
LucasSloan Oct 1, 2013
2edef55
Descendant karma attribute no longer disappears off python objects.
LucasSloan Oct 1, 2013
33a3850
Merge branch 'interesting_comments' of https://github.com/PotatoDumpl…
LucasSloan Oct 1, 2013
2124bd7
Changed testing syntax.
LucasSloan Dec 9, 2013
9bdf974
Merge branch 'postmovement' of https://github.com/PotatoDumplings/les…
LucasSloan Dec 9, 2013
ce3d6ff
Merge branch 'master' of https://github.com/tricycle/lesswrong into p…
LucasSloan Dec 9, 2013
5e5f092
Variety of small changes for readability.
LucasSloan Dec 9, 2013
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions r2/r2/controllers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,40 @@ def POST_verifyemail(self, res, code):
c.user._commit()
res._success()

@Json
@validate(VModhash(),
VSrCanBan('id'),
thing = VByName('id'),
destination = VMoveURL('destination'),
reason = VComment('comment'))
def POST_move(self, res, thing, destination, reason):
res._update('status_' + thing._fullname, innerHTML = '')
if res._chk_errors((errors.NO_URL, errors.BAD_URL),
thing._fullname):
res._focus("destination_url_" + thing._fullname)
return
if res._chk_error(errors.COMMENT_TOO_LONG,
thing._fullname):
res._focus("comment_replacement_" + thing._fullname)
return

comment, inbox_rel = Comment._new(Account._byID(thing.author_id),
destination, None, thing.body,
thing.ip)
children = list(Comment._query(Comment.c.parent_id == thing._id))
for child in children:
child.recursive_move(destination, comment)

thing.moved = True
thing.set_body('This comment was moved by ' + c.user.name +
' to [here]({0}).\n\n'.format(comment.make_anchored_permalink(destination)) +
(reason if reason else ''))

if g.write_query_queue:
queries.new_comment(comment, None)

res._send_things(thing)

@Json
@validate(VCaptcha(),
VUser(),
Expand Down
14 changes: 14 additions & 0 deletions r2/r2/controllers/validator/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,20 @@ def run(self, name):
except NotFound:
return name

class VMoveURL(VRequired):
def __init__(self, item, *a, **kw):
VRequired.__init__(self, item, errors.NO_URL, *a, **kw)

def run(self, url):
if not url:
return self.error()
else:
link = Link._byURL(url)
if not link:
return self.error(errors.BAD_URL)
else:
return link

class VSubredditTitle(Validator):
def run(self, title):
if not title:
Expand Down
12 changes: 11 additions & 1 deletion r2/r2/lib/pages/pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,7 @@ def __init__(self, link = None, comment = None,

# link is a wrapped Link object
self.link = self.link_listing.things[0]
self.movebox = MoveBox()

link_title = ((self.link.title) if hasattr(self.link, 'title') else '')
if comment:
Expand All @@ -665,7 +666,7 @@ def __init__(self, link = None, comment = None,
Reddit.__init__(self, title = title, body_class = 'post', robots = self.robots, *a, **kw)

def content(self):
return self.content_stack(self.infobar, self.link_listing, self._content)
return self.content_stack(self.infobar, self.link_listing, self.movebox, self._content)

def build_toolbars(self):
return []
Expand Down Expand Up @@ -989,6 +990,15 @@ def __init__(self, link_name='', captcha=None, action = 'comment'):
Wrapped.__init__(self, link_name = link_name, captcha = captcha,
action = action)

class MoveBox(Wrapped):
"""Used on LinkInfoPage to render the comment reply form at the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update docstring

top of the comment listing as well as the template for the forms
which are JS inserted when clicking on 'reply' in either a comment
or message listing."""
def __init__(self, link_name='', captcha=None, action = 'comment'):
Wrapped.__init__(self, link_name = link_name, captcha = captcha,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the default action for a move thread form "comment"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason. Holdover from the initialization for what the movebox was created from. I'll remove it.

action = action)

class CommentListing(Wrapped):
"""Comment heading and sort, limit options"""
def __init__(self, content, num_comments, nav_menus = []):
Expand Down
43 changes: 41 additions & 2 deletions r2/r2/models/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,20 @@ def __init__(self, *a, **kw):
def by_url_key(cls, url):
return base_url(url.lower()).encode('utf8')

@classmethod
def _byURL(cls, url):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's already a _by_url method (below). Instead of having two very similarly named methods can see if the other one is used and remove if possible.

url_re = re.compile("(?:http://)?.*?(/r/.*?)?(/lw/.*?/.*)")
id_re = re.compile("/lw/(\w*)/.*")
matcher = url_re.match(url)
if not matcher:
return False
matcher = id_re.match(matcher.group(2))
try:
link = Link._byID(int(matcher.group(1), 36))
except NotFound: return None
if not link._loaded: link._load()
return link

@classmethod
def _by_url(cls, url, sr):
from subreddit import Default
Expand Down Expand Up @@ -861,7 +875,8 @@ class Comment(Thing, Printable):
banned_before_moderator = False,
is_html = False,
retracted = False,
show_response_to = False)
show_response_to = False,
moved = False)

def _markdown(self):
pass
Expand Down Expand Up @@ -941,6 +956,30 @@ def has_children(self):
child = list(q)
return len(child)>0

def recursive_move(self, destination, parent):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes me nervous as it will be potentially modifying a bunch of records without any transactional safety. As far as I know the ORM doesn't offer transactions but locks are available. Alternatively could the relevant records all be updated at once through SQL?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you're afraid of happening here.

On Tue, Jul 23, 2013 at 10:05 PM, Wesley Moore [email protected]:

In r2/r2/models/link.py:

@@ -941,6 +956,30 @@ def has_children(self):
child = list(q)
return len(child)>0

  • def recursive_move(self, destination, parent):

This makes me nervous as it will be potentially modifying a bunch of
records without any transactional safety. As far as I know the ORM doesn't
offer transactions but locks are available. Alternatively could the
relevant records all be updated at once through SQL?


Reply to this email directly or view it on GitHubhttps://github.com//pull/80/files#r5363906
.

"And someday when the descendants of humanity have spread from star to
star, they won't tell the children about the history of Ancient Earth until
they're old enough to bear it; and when they learn they'll weep to hear
that such a thing as Death had ever once existed!
"

-Eliezer Yudkowsky

"Trust me" means "I love you" http://project-apollo.net/mos/mos114.html

"This isn't even my final form!"

-Tim Hausler

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, thinking about it more. I think the main thing is comments added whilst the move is taking place. I think you should wrap the move process in a lock that prevents posting while it is taking place. There are already locks present when posting a comment so adding a new one should be too difficult: https://github.com/tricycle/lesswrong/blob/master/r2/r2/lib/comment_tree.py#L34

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, done.

On Tue, Jul 23, 2013 at 10:56 PM, Wesley Moore [email protected]:

In r2/r2/models/link.py:

@@ -941,6 +956,30 @@ def has_children(self):
child = list(q)
return len(child)>0

  • def recursive_move(self, destination, parent):

Hmm, thinking about it more. I think the main thing is comments added
whilst the move is taking place. I think you should wrap the move process
in a lock that prevents posting while it is taking place. There are already
locks present when posting a comment so adding a new one should be too
difficult:
https://github.com/tricycle/lesswrong/blob/master/r2/r2/lib/comment_tree.py#L34


Reply to this email directly or view it on GitHubhttps://github.com//pull/80/files#r5364322
.

"And someday when the descendants of humanity have spread from star to
star, they won't tell the children about the history of Ancient Earth until
they're old enough to bear it; and when they learn they'll weep to hear
that such a thing as Death had ever once existed!
"

-Eliezer Yudkowsky

"Trust me" means "I love you" http://project-apollo.net/mos/mos114.html

"This isn't even my final form!"

-Tim Hausler

q = Comment._query(Comment.c.parent_id == self._id)
children = list(q)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Query defines an __iter__ method, so you don't actually need to listify the result before iterating over it. I think you could do this:

children = Comment._query(...)
.
.
.
for child in children:
   ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed.

comment, inbox_rel = Comment._new(Account._byID(self.author_id),
destination, parent, self.body,
self.ip)
if not children:
pass
else:
for child in children:
child.recursive_move(destination, comment)


self.moderator_banned = not c.user_is_admin
self.banner = c.user.name
#self.moved = True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be commented out?

self._commit()
# NB: change table updated by reporting
from r2.models.report import unreport
unreport(self, correct=True, auto=False)

if g.write_query_queue:
queries.new_comment(comment, None)

def can_delete(self):
if not self._loaded:
self._load()
Expand Down Expand Up @@ -1082,7 +1121,7 @@ def add_props(cls, user, wrapped):
item.can_reply = (item.sr_id in can_reply_srs)

# Don't allow users to vote on their own comments
item.votable = bool(c.user != item.author and not item.retracted)
item.votable = bool(c.user != item.author and not item.retracted and not item.moved)
if item.votable and c.profilepage:
# Can only vote on profile page under certain conditions
item.votable = bool((c.user.safe_karma > g.karma_to_vote_in_overview) and (g.karma_percentage_to_be_voted > item.author.percent_up()))
Expand Down
89 changes: 89 additions & 0 deletions r2/r2/public/static/comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,29 @@ Comment.prototype.show_editor = function(listing, where, text) {
return edit_box;
};

Comment.prototype.show_move = function(listing, where, text) {
var edit_box = this.cloneAndReIDNodeOnce("samplemove");
if (edit_box.parentNode != listing.listing) {
if (edit_box.parentNode) {
edit_box.parentNode.removeChild(edit_box);
}
listing.insert_node_before(edit_box, where);
}
else if (edit_box.parentNode.firstChild != edit_box) {
var p = edit_box.parentNode;
p.removeChild(edit_box);
p.insertBefore(edit_box, p.firstChild);
}
var box = this.$parent("comment_replacement");
clearTitle(box);
box.value = text;
box.setAttribute("data-orig-value", text);
show(edit_box);
BeforeUnload.bind(Comment.checkModified, this._id);
return edit_box;
};


Comment.prototype.edit = function() {
this.show_editor(this.parent_listing(), this.row, this.text);
this.$parent("commentform").replace.value = "yes";
Expand Down Expand Up @@ -145,17 +168,34 @@ Comment.prototype.reply = function (showFlamebaitOverlay) {
this.$parent("comment_reply").focus();
};


Comment.prototype.move = function() {
this.show_move(this.parent_listing(), this.row, '');
this.$parent("moveform").replace.value = "";
this.$parent("destination_url").focus();
this.hide();
};

Comment.prototype.cancel = function() {
var edit_box = this.cloneAndReIDNodeOnce("samplecomment");
hide(edit_box);
BeforeUnload.unbind(Comment.checkModified, this._id);
this.show();
};

Comment.prototype.cancelmove = function() {
var edit_box = this.cloneAndReIDNodeOnce("samplemove");
hide(edit_box);
BeforeUnload.unbind(Comment.checkModified, this._id);
this.show();
};


Comment.comment = function(r, context) {
var id = r.id;
var parent_id = r.parent;
new Comment(parent_id, context).cancel();
new Comment(parent_id, context).cancelmove();
new Listing(parent_id, context).push(unsafe(r.content));
new Comment(r.id, context).show();
vl[id] = r.vl;
Expand Down Expand Up @@ -191,6 +231,14 @@ Comment.editcomment = function(r, context) {
com.show();
};

Comment.move = function(r, context) {
var com = new Comment(r.id, context);
com.get('body').innerHTML = unsafe(r.contentHTML);
com.get('edit_body').innerHTML = unsafe(r.contentTxt);
com.cancelmove();
com.show();
};

Comment.submitballot = function(r) {
var com = new Comment(r.id);
com.get('body').innerHTML = unsafe(r.contentHTML);
Expand Down Expand Up @@ -276,6 +324,9 @@ function cancelReply(canceler) {
new Comment(_id(canceler), Thing.getThingRow(canceler)).cancel();
};

function cancelMove(canceler) {
new Comment(_id(canceler), Thing.getThingRow(canceler)).cancelmove();
};

function reply(id, link, showFlamebaitOverlay) {
if (logged) {
Expand All @@ -286,6 +337,16 @@ function reply(id, link, showFlamebaitOverlay) {
}
};

function move(id, link) {
if (logged) {
var com = new Comment(id, Thing.getThingRow(link)).move();
}
else {
showcover(true, 'reply_' + id);
}
};


function chkcomment(form) {
if(checkInProgress(form)) {
var r = confirm("Request still in progress\n(click Cancel to attempt to stop the request)");
Expand Down Expand Up @@ -314,6 +375,34 @@ function chkcomment(form) {
});
};

function chkmove(form) {
if(checkInProgress(form)) {
var r = confirm("Request still in progress\n(click Cancel to attempt to stop the request)");
if (r==false)
tagInProgress(form, false);
return false;
}

var action = 'move';
var context = Thing.getThingRow(form);

tagInProgress(form, true);

function cleanup_func(res_obj) {
tagInProgress(form, false);

var obj = res_obj && res_obj.response && res_obj.response.object;
if (obj && obj.length)
for (var o = 0, ol = obj.length; o < ol; ++o)
Comment[action](obj[o].data, context);
}

return post_form(form, action, null, null, true, null, {
handle_obj: false,
cleanup_func: cleanup_func
});
};

function tagInProgress(form, inProgress) {
if (inProgress)
form.addClassName("inprogress");
Expand Down
56 changes: 56 additions & 0 deletions r2/r2/public/static/lesswrong.css
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,62 @@ div.inline-listing a:hover {
text-decoration: none;
}

.commentreplacement {
clear: both;
margin: 10px 15px 30px 0;
}
.commentreplacement textarea, .commentreplacement input {
border-color: #e9e9e9;
-webkit-box-shadow: inset 1px 1px 5px #dbdbdb;
-moz-box-shadow: inset 1px 1px 5px #dbdbdb;
box-shadow: inset 1px 1px 5px #dbdbdb;
margin: 0 0 10px;
width: 100%;
}
.commentreplacement .buttons {
float: left;
}
.commentreplacement .buttons button, .commentreplacement .help-toggle button, .poll-voting-area button {
background: #f0eeee; /* Old browsers */
background: -moz-linear-gradient(top, #f0eeee 0%, #c1c1c1 100%); /* FF3.6+ */
background: -webkit-gradient(linear, left top, left bottom, color-stop(0%,#f0eeee), color-stop(100%,#c1c1c1)); /* Chrome,Safari4+ */
background: -webkit-linear-gradient(top, #f0eeee 0%,#c1c1c1 100%); /* Chrome10+,Safari5.1+ */
background: -o-linear-gradient(top, #f0eeee 0%,#c1c1c1 100%); /* Opera11.10+ */
background: -ms-linear-gradient(top, #f0eeee 0%,#c1c1c1 100%); /* IE10+ */
filter: progid:DXImageTransform.Microsoft.gradient( startColorstr='#f0eeee', endColorstr='#c1c1c1',GradientType=0 ); /* IE6-9 */
background: linear-gradient(top, #f0eeee 0%,#c1c1c1 100%); /* W3C */
border-color: #c6c6c6;
color: #666666;
cursor: pointer;
font-weight: bold;
padding: 5px 30px;
}
.commentreplacement .help-toggle button {
margin-right: -6px;
}

.commentreplacement table.help {
clear: both;
margin: 5px 0 0 1px;
width: 480px;
border-collapse: collapse;
}
.commentreplacement .help,
.commentreplacement .help td,
.commentreplacement .help tr {
border: 1px solid #C0C0C0;
padding: 4px;
margin: 0px;
}
.commentreplacement .help-toggle {
float: right;
}
.commentreplacement .help-toggle a {
color: #538D4D;
font-weight: bold;
text-decoration: none;
}

#comment-controls label {
padding-right: 5px;
margin-left: 1em;
Expand Down
3 changes: 2 additions & 1 deletion r2/r2/public/static/main.css
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,8 @@ div.comment-links ul li.retract a.yes, div.comment-links ul li.retract a.no {

/* Temporary styling for text buttons until we have icons */
div.comment-links ul li.delete a, div.comment-links ul li.unban a,
div.comment-links ul li.ban a, div.comment-links ul li.ignore a {
div.comment-links ul li.ban a, div.comment-links ul li.ignore a,
div.comment-links ul li.move a {
height: 20px;
padding-top: 4px;
}
Expand Down
Loading