You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2016/11/30 21:41:29 UTC
[08/14] allura git commit: [#7981] remove old thread subscription
field, use modern Artifact/Mailbox approach
[#7981] remove old thread subscription field, use modern Artifact/Mailbox approach
Project: http://git-wip-us.apache.org/repos/asf/allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/allura/commit/3e3fbdb9
Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/3e3fbdb9
Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/3e3fbdb9
Branch: refs/heads/db/7981
Commit: 3e3fbdb994a631274ed2493085861086e60aed62
Parents: 1e0822a
Author: Dave Brondsema <da...@brondsema.net>
Authored: Fri Nov 18 12:19:54 2016 -0500
Committer: Dave Brondsema <da...@brondsema.net>
Committed: Wed Nov 30 15:26:42 2016 -0500
----------------------------------------------------------------------
Allura/allura/controllers/discuss.py | 13 +++++++------
Allura/allura/model/artifact.py | 11 +++++++++++
Allura/allura/model/discuss.py | 13 -------------
.../allura/templates/widgets/threads_table.html | 2 +-
Allura/allura/tests/functional/test_discuss.py | 20 ++++++++++++--------
Allura/allura/tests/model/test_discussion.py | 1 +
.../forgediscussion/controllers/root.py | 1 +
ForgeDiscussion/forgediscussion/forum_main.py | 2 --
.../tests/functional/test_forum.py | 10 ----------
9 files changed, 33 insertions(+), 40 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/allura/blob/3e3fbdb9/Allura/allura/controllers/discuss.py
----------------------------------------------------------------------
diff --git a/Allura/allura/controllers/discuss.py b/Allura/allura/controllers/discuss.py
index 9cb5f37..d6ba1ff 100644
--- a/Allura/allura/controllers/discuss.py
+++ b/Allura/allura/controllers/discuss.py
@@ -25,6 +25,7 @@ from pylons import tmpl_context as c, app_globals as g
from webob import exc
from ming.base import Object
+from ming.odm import session
from ming.utils import LazyProperty
from allura import model as M
@@ -96,13 +97,13 @@ class DiscussionController(BaseController, FeedController):
def subscribe(self, **kw):
threads = kw.pop('threads', [])
for t in threads:
- thread = self.M.Thread.query.find(dict(_id=t['_id'])).first()
- if 'subscription' in t:
- thread['subscription'] = True
+ thread = self.M.Thread.query.get(_id=t['_id'])
+ if t.get('subscription'):
+ thread.subscribe()
else:
- thread['subscription'] = False
- M.session.artifact_orm_session._get().skip_mod_date = True
- M.session.artifact_orm_session._get().skip_last_updated = True
+ thread.unsubscribe()
+ session(self.M.Thread)._get().skip_mod_date = True
+ session(self.M.Thread)._get().skip_last_updated = True
redirect(request.referer)
def get_feed(self, project, app, user):
http://git-wip-us.apache.org/repos/asf/allura/blob/3e3fbdb9/Allura/allura/model/artifact.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/artifact.py b/Allura/allura/model/artifact.py
index ff4bd76..39316d2 100644
--- a/Allura/allura/model/artifact.py
+++ b/Allura/allura/model/artifact.py
@@ -226,6 +226,17 @@ class Artifact(MappedClass, SearchIndexable):
app_config_id=self.app_config._id,
artifact_index_id=self.index_id())
+ def subscribed(self, user=None):
+ from allura.model import Mailbox
+ if user is None:
+ user = c.user
+ return Mailbox.subscribed(
+ user_id=user._id,
+ project_id=self.app_config.project_id,
+ app_config_id=self.app_config._id,
+ artifact=self,
+ )
+
def primary(self):
"""If an artifact is a "secondary" artifact (discussion of a ticket, for
instance), return the artifact that is the "primary".
http://git-wip-us.apache.org/repos/asf/allura/blob/3e3fbdb9/Allura/allura/model/discuss.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/discuss.py b/Allura/allura/model/discuss.py
index b3cf04b..19c0ea5 100644
--- a/Allura/allura/model/discuss.py
+++ b/Allura/allura/model/discuss.py
@@ -118,9 +118,6 @@ class Discussion(Artifact, ActivityObject):
text=self.description)
return result
- def subscription(self):
- return self.subscriptions.get(str(c.user._id))
-
def delete(self):
# Delete all the threads, posts, and artifacts
self.thread_class().query.remove(dict(discussion_id=self._id))
@@ -428,16 +425,6 @@ class Thread(Artifact, ActivityObject):
text=self.subject)
return result
- def _get_subscription(self):
- return self.subscriptions.get(str(c.user._id))
-
- def _set_subscription(self, value):
- if value:
- self.subscriptions[str(c.user._id)] = True
- else:
- self.subscriptions.pop(str(c.user._id), None)
- subscription = property(_get_subscription, _set_subscription)
-
def delete(self):
for p in self.post_class().query.find(dict(thread_id=self._id)):
p.delete()
http://git-wip-us.apache.org/repos/asf/allura/blob/3e3fbdb9/Allura/allura/templates/widgets/threads_table.html
----------------------------------------------------------------------
diff --git a/Allura/allura/templates/widgets/threads_table.html b/Allura/allura/templates/widgets/threads_table.html
index 2101606..bc20d29 100644
--- a/Allura/allura/templates/widgets/threads_table.html
+++ b/Allura/allura/templates/widgets/threads_table.html
@@ -37,7 +37,7 @@
{% if not c.user.is_anonymous() %}
<td>
<input type="checkbox" name="threads-{{loop.index0}}.subscription"
- {% if thread.subscription %}checked="checked"{% endif %}/>
+ {% if thread.subscribed() %}checked="checked"{% endif %}/>
<input type="hidden" name="threads-{{loop.index0}}._id" value="{{thread._id}}"/>
</td>
{% endif %}
http://git-wip-us.apache.org/repos/asf/allura/blob/3e3fbdb9/Allura/allura/tests/functional/test_discuss.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_discuss.py b/Allura/allura/tests/functional/test_discuss.py
index 836d094..56bb77d 100644
--- a/Allura/allura/tests/functional/test_discuss.py
+++ b/Allura/allura/tests/functional/test_discuss.py
@@ -24,6 +24,7 @@ from ming.odm import session
from allura.tests import TestController
from allura import model as M
+
class TestDiscussBase(TestController):
def _thread_link(self):
@@ -41,24 +42,27 @@ class TestDiscussBase(TestController):
class TestDiscuss(TestDiscussBase):
- def _is_subscribed(self, username, thread_id):
- user_id = str(M.User.by_username(username)._id)
- thread = M.Thread.query.get(_id=thread_id)
- return thread.subscriptions.get(user_id)
+ def _is_subscribed(self, user, thread):
+ return M.Mailbox.query.get(user_id=user._id, artifact_index_id=thread.index_id())
def test_subscribe_unsubscribe(self):
- user = 'test-admin'
+ user = M.User.by_username('test-admin')
thread_id = self._thread_id()
- assert_false(self._is_subscribed(user, thread_id))
+ thread = M.Thread.query.get(_id=thread_id)
+
+ # remove tool-wide subscription, so it doesn't interfere
+ M.Mailbox.query.remove(dict(user_id=user._id, app_config_id=thread.app_config_id))
+
+ assert_false(self._is_subscribed(user, thread))
link = self._thread_link()
params = {
'threads-0._id': thread_id,
'threads-0.subscription': 'on'}
r = self.app.post('/wiki/_discuss/subscribe', params=params)
- assert_true(self._is_subscribed(user, thread_id))
+ assert_true(self._is_subscribed(user, thread))
params = {'threads-0._id': thread_id}
r = self.app.post('/wiki/_discuss/subscribe', params=params)
- assert_false(self._is_subscribed(user, thread_id))
+ assert_false(self._is_subscribed(user, thread))
def _make_post(self, text):
thread_link = self._thread_link()
http://git-wip-us.apache.org/repos/asf/allura/blob/3e3fbdb9/Allura/allura/tests/model/test_discussion.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/model/test_discussion.py b/Allura/allura/tests/model/test_discussion.py
index bd95bd3..b579d18 100644
--- a/Allura/allura/tests/model/test_discussion.py
+++ b/Allura/allura/tests/model/test_discussion.py
@@ -104,6 +104,7 @@ def test_thread_methods():
assert 'wiki/_discuss/' in t.url()
assert t.index()['views_i'] == 0
+ # FIXME
assert not t.subscription
t.subscription = True
assert t.subscription
http://git-wip-us.apache.org/repos/asf/allura/blob/3e3fbdb9/ForgeDiscussion/forgediscussion/controllers/root.py
----------------------------------------------------------------------
diff --git a/ForgeDiscussion/forgediscussion/controllers/root.py b/ForgeDiscussion/forgediscussion/controllers/root.py
index cb4b4f8..8ade51f 100644
--- a/ForgeDiscussion/forgediscussion/controllers/root.py
+++ b/ForgeDiscussion/forgediscussion/controllers/root.py
@@ -203,6 +203,7 @@ class RootController(BaseController, DispatchIndex, FeedController):
objs.append(dict(obj=model.Thread.query.get(_id=data['id']),
subscribed=bool(data.get('subscribed'))))
for obj in objs:
+ # TODO where is this called from?
if obj['subscribed']:
obj['obj'].subscriptions[str(c.user._id)] = True
else:
http://git-wip-us.apache.org/repos/asf/allura/blob/3e3fbdb9/ForgeDiscussion/forgediscussion/forum_main.py
----------------------------------------------------------------------
diff --git a/ForgeDiscussion/forgediscussion/forum_main.py b/ForgeDiscussion/forgediscussion/forum_main.py
index 524b11d..60b97d3 100644
--- a/ForgeDiscussion/forgediscussion/forum_main.py
+++ b/ForgeDiscussion/forgediscussion/forum_main.py
@@ -91,8 +91,6 @@ class ForgeDiscussionApp(Application):
self.root = RootController()
self.api_root = RootRestController()
self.admin = ForumAdminController(self)
- self.default_forum_preferences = dict(
- subscriptions={})
def has_access(self, user, topic):
f = DM.Forum.query.get(shortname=topic.replace('.', '/'),
http://git-wip-us.apache.org/repos/asf/allura/blob/3e3fbdb9/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py
----------------------------------------------------------------------
diff --git a/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py b/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py
index 55da689..6606b32 100644
--- a/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py
+++ b/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py
@@ -374,16 +374,6 @@ class TestForum(TestController):
r = self.app.get('/discussion/markdown_syntax')
assert 'Markdown Syntax' in r
- def test_forum_subscribe(self):
- self.app.post('/discussion/subscribe', params={
- 'forum-0.shortname': 'testforum',
- 'forum-0.subscribed': 'on',
- })
- self.app.post('/discussion/subscribe', params={
- 'forum-0.shortname': 'testforum',
- 'forum-0.subscribed': '',
- })
-
def test_forum_index(self):
self.app.get('/discussion/testforum/')
self.app.get('/discussion/testforum/childforum/')