allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From brond...@apache.org
Subject [3/9] allura git commit: [#7981] remove old thread subscription field, use modern Artifact/Mailbox approach
Date Mon, 05 Dec 2016 16:39:36 GMT
[#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/59a6a343
Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/59a6a343
Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/59a6a343

Branch: refs/heads/master
Commit: 59a6a343f1d5953c1c1a53244fe7d8763a71b529
Parents: e7a39fc
Author: Dave Brondsema <dave@brondsema.net>
Authored: Fri Nov 18 12:19:54 2016 -0500
Committer: Dave Brondsema <dave@brondsema.net>
Committed: Mon Dec 5 11:39:20 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/59a6a343/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/59a6a343/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/59a6a343/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/59a6a343/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/59a6a343/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/59a6a343/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/59a6a343/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/59a6a343/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/59a6a343/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/')


Mime
View raw message