-
Notifications
You must be signed in to change notification settings - Fork 24
Postmovement #80
base: master
Are you sure you want to change the base?
Postmovement #80
Changes from 1 commit
03bae0d
a95fa1a
c7fc246
2739bef
eefd50e
f3721e5
afe23b1
582d9fd
ac9ed72
fa13a33
b7e7355
91d0c4e
bedd944
cf10a31
2edef55
33a3850
2124bd7
9bdf974
ce3d6ff
5e5f092
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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: | ||
|
|
@@ -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 [] | ||
|
|
@@ -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 | ||
| 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, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the default action for a move thread form "comment"?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 = []): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,6 +82,18 @@ def __init__(self, *a, **kw): | |
| def by_url_key(cls, url): | ||
| return base_url(url.lower()).encode('utf8') | ||
|
|
||
| @classmethod | ||
| def _byURL(cls, url): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)) | ||
| link = Link._byID(int(matcher.group(1), 36)) | ||
| if not link._loaded: link._load() | ||
| return link | ||
|
|
||
| @classmethod | ||
| def _by_url(cls, url, sr): | ||
| from subreddit import Default | ||
|
|
@@ -174,6 +186,7 @@ def _submit(cls, title, article, author, sr, ip, tags, spam = False, date = None | |
| # Parse and create polls in the article | ||
| l.set_article(article) | ||
|
|
||
| print l.url | ||
| l.set_url_cache() | ||
|
|
||
| # Add tags | ||
|
|
@@ -941,6 +954,28 @@ def has_children(self): | |
| child = list(q) | ||
| return len(child)>0 | ||
|
|
||
| def recursive_move(self, destination, parent): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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]:
"And someday when the descendants of humanity have spread from star to -Eliezer Yudkowsky "Trust me" means "I love you" http://project-apollo.net/mos/mos114.html "This isn't even my final form!" -Tim Hausler
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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]:
"And someday when the descendants of humanity have spread from star to -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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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._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() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,6 +164,11 @@ | |
| <% allow_delete = thing.can_be_deleted %> | ||
| ${parent.delete_or_report_buttons(allow_delete,False,True)} | ||
| ${parent.buttons()} | ||
| %if thing.can_ban: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this shown if
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can_ban already is that flag. I'm against renaming it, it seems pretty self explanatory. |
||
| <li class="move"> | ||
| ${parent.simple_button("move", fullname, _("Move"), "move", tool_tip=_("Move"))} | ||
| </li> | ||
| %endif | ||
| </ul> | ||
| ## Each comment has a hidden status span that is used to indicate voting errors. | ||
| <span id='${"status_"+thing._fullname}' class="error" style="display: none;"></span> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update docstring