incubator-allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tvansteenbu...@apache.org
Subject [6/6] git commit: [#6694] Refactoring and cleanup
Date Fri, 08 Nov 2013 04:21:16 GMT
[#6694] Refactoring and cleanup

Signed-off-by: Tim Van Steenburgh <tvansteenburgh@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/7eb73bcc
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/7eb73bcc
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/7eb73bcc

Branch: refs/heads/tv/6694
Commit: 7eb73bcc5d76f9120a8c692df93e7a5fd86391dd
Parents: de17be5
Author: Tim Van Steenburgh <tvansteenburgh@gmail.com>
Authored: Fri Nov 8 04:20:30 2013 +0000
Committer: Tim Van Steenburgh <tvansteenburgh@gmail.com>
Committed: Fri Nov 8 04:20:30 2013 +0000

----------------------------------------------------------------------
 .../user_profile/templates/send_message.html    |  3 +-
 .../ext/user_profile/templates/user_index.html  | 37 ++++++------
 Allura/allura/ext/user_profile/user_main.py     | 59 +++++++++++---------
 Allura/allura/lib/app_globals.py                | 16 ++++++
 Allura/allura/model/auth.py                     | 49 +++++++++++-----
 .../tests/functional/test_user_profile.py       | 19 ++++---
 Allura/allura/tests/model/test_auth.py          | 17 +++---
 7 files changed, 125 insertions(+), 75 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/ext/user_profile/templates/send_message.html
----------------------------------------------------------------------
diff --git a/Allura/allura/ext/user_profile/templates/send_message.html b/Allura/allura/ext/user_profile/templates/send_message.html
index 44b64b0..eb99f41 100644
--- a/Allura/allura/ext/user_profile/templates/send_message.html
+++ b/Allura/allura/ext/user_profile/templates/send_message.html
@@ -36,7 +36,8 @@
     {%if not expire_time%}
         {{c.form.display(method = 'POST', action='send_user_message', user=user)}}
     {% else %}
-        <h2>Sorry, you can send an email after {{expire_time}}</h2>
+      <h2>Sorry, messaging is rate-limited.</h2>
+      <p>You can send another message in: {{expire_time}}</p>
     {% endif %}
     </div>
 {% endblock %}

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/ext/user_profile/templates/user_index.html
----------------------------------------------------------------------
diff --git a/Allura/allura/ext/user_profile/templates/user_index.html b/Allura/allura/ext/user_profile/templates/user_index.html
index 88c54c7..e3f2833 100644
--- a/Allura/allura/ext/user_profile/templates/user_index.html
+++ b/Allura/allura/ext/user_profile/templates/user_index.html
@@ -104,7 +104,7 @@
         <div class="grid-18">
            {{user.get_pref('display_name')}}'s account(s):
            <ul>
-             {% for i in user.get_pref('socialnetworks') %}            
+             {% for i in user.get_pref('socialnetworks') %}
                 <li>{{i.socialnetwork}}: <a href="{{i.accounturl}}">{{i.accounturl}}</a></li>
              {% endfor %}
            </ul>
@@ -118,7 +118,7 @@
         <div class="grid-18">
            {{user.get_pref('display_name')}}'s website(s):
            <ul>
-             {% for i in user.get_pref('webpages') %}            
+             {% for i in user.get_pref('webpages') %}
                 <li><a href="{{i}}">{{i}}</a></li>
              {% endfor %}
            </ul>
@@ -132,7 +132,7 @@
         <div class="grid-18">
            {{user.get_pref('display_name')}}'s telephone number(s):
            <ul>
-             {% for i in user.get_pref('telnumbers') %}            
+             {% for i in user.get_pref('telnumbers') %}
                 <li>{{i}}</li>
              {% endfor %}
            </ul>
@@ -153,14 +153,14 @@
 
           {% if c.user.get_pref('timezone') %}
           <div class="grid-18" id="timeslotsconverted" style="visibility: visible; display:none;">
-             {{user.get_pref('display_name')}}'s availability time-slots. 
+             {{user.get_pref('display_name')}}'s availability time-slots.
              <div style="float:right;">
                See timeslots in:
-               <a href="JavaScript:void(0);" onclick="changeTimezone('utc')">UTC</a>
| 
-               <a href="JavaScript:void(0);" onclick="changeTimezone('local')"> 
+               <a href="JavaScript:void(0);" onclick="changeTimezone('utc')">UTC</a>
|
+               <a href="JavaScript:void(0);" onclick="changeTimezone('local')">
                   {{user.get_pref('display_name')}}'s local time
                </a> |
-               <b>Your local time</b> 
+               <b>Your local time</b>
              </div>
              <ul>
                {% for i in user.get_localized_availability(c.user.get_pref('timezone')) %}
@@ -171,13 +171,13 @@
           {% endif %}
 
           <div class="grid-18" id="timeslotsutc" style="visibility: visible; display:block;">
-             {{user.get_pref('display_name')}}'s availability time-slots. 
+             {{user.get_pref('display_name')}}'s availability time-slots.
              <div style="float:right;">
                See timeslots in:
-               <b>UTC</b> | 
-               <a href="JavaScript:void(0);" onclick="changeTimezone('local')"> 
+               <b>UTC</b> |
+               <a href="JavaScript:void(0);" onclick="changeTimezone('local')">
                   {{user.get_pref('display_name')}}'s local time
-               </a> 
+               </a>
                {% if c.user.get_pref('timezone') %} |
                   <a href="JavaScript:void(0);" onclick="changeTimezone('converted')">
                     Your local time
@@ -192,13 +192,13 @@
           </div>
 
           <div class="grid-18" id="timeslotslocal" style="visibility: visible; display:none;">
-             {{user.get_pref('display_name')}}'s availability time-slots. 
+             {{user.get_pref('display_name')}}'s availability time-slots.
              <div style="float:right;">
                See timeslots in:
-               <a href="JavaScript:void(0);" onclick="changeTimezone('utc')">UTC</a>
| 
-               <b> 
+               <a href="JavaScript:void(0);" onclick="changeTimezone('utc')">UTC</a>
|
+               <b>
                   {{user.get_pref('display_name')}}'s local time
-               </b> 
+               </b>
                {% if c.user.get_pref('timezone') %} |
                   <a href="JavaScript:void(0);" onclick="changeTimezone('converted')">
                     Your local time
@@ -228,11 +228,12 @@
         </div>
       </div>
     {% endif %}
- {% if has_email %}
+
+    {% if user.get_pref('email_address') and c.user.get_pref('email_address') %}
     <div class="grid-24">
-        <b><a href="send_message">Send me a message</a></b>
+      <b><a href="send_message">Send me a message</a></b>
     </div>
-  {% endif %}
+    {% endif %}
   </div><!-- end of Personal data section -->
   <div class="grid-24">
     <b>Current {{user.get_pref('display_name')}}'s skills list</b>

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/ext/user_profile/user_main.py
----------------------------------------------------------------------
diff --git a/Allura/allura/ext/user_profile/user_main.py b/Allura/allura/ext/user_profile/user_main.py
index 76246c7..0dbfee1 100644
--- a/Allura/allura/ext/user_profile/user_main.py
+++ b/Allura/allura/ext/user_profile/user_main.py
@@ -92,14 +92,28 @@ class UserProfileController(BaseController, FeedController):
     def _check_security(self):
         require_access(c.project, 'read')
 
+    def _check_can_message(self, from_user, to_user):
+        if from_user is User.anonymous():
+            flash('You must be logged in to send user messages.', 'info')
+            redirect(request.referer)
+
+        if not (from_user and from_user.get_pref('email_address')):
+            flash('In order to send messages, you must have an email address '
+                    'associated with your account.', 'info')
+            redirect(request.referer)
+
+        if not (to_user and to_user.get_pref('email_address')):
+            flash('This user can not receive messages because they do not have '
+                    'an email address associated with their account.', 'info')
+            redirect(request.referer)
+
     @expose('jinja:allura.ext.user_profile:templates/user_index.html')
     def index(self, **kw):
         user = c.project.user_project_of
         if not user:
             raise exc.HTTPNotFound()
         provider = AuthenticationProvider.get(request)
-        has_email = c.user.get_pref('email_address') is not None
-        return dict(user=user, reg_date=provider.user_registration_date(user), has_email=has_email)
+        return dict(user=user, reg_date=provider.user_registration_date(user))
 
     def get_feed(self, project, app, user):
         """Return a :class:`allura.controllers.feed.FeedArgs` object describing
@@ -116,41 +130,34 @@ class UserProfileController(BaseController, FeedController):
 
     @expose('jinja:allura.ext.user_profile:templates/send_message.html')
     def send_message(self):
-        user = c.project.user_project_of
-        both_have_emails = user and user.get_pref('email_address') and c.user.get_pref('email_address')
-        if not both_have_emails:
-            raise exc.HTTPNotFound()
+        """Render form for sending a message to another user.
 
-        time_interval = config['user_message.time_interval']
-        max_messages = config['user_message.max_messages']
-        expire_time = None
+        """
+        self._check_can_message(c.user, c.project.user_project_of)
 
-        if not c.user.check_send_emails_times(time_interval, max_messages):
-            expire_seconds = c.user.send_emails_times[0] + timedelta(seconds=int(time_interval))
- datetime.utcnow()
-            h, remainder = divmod(expire_seconds.total_seconds(), 3600)
-            m, s = divmod(remainder, 60)
-            expire_time = '%s:%s:%s' % (int(h), int(m), int(s))
+        delay = c.user.time_to_next_user_message()
+        expire_time = str(delay) if delay else None
         c.form = F.send_message
-        return dict(user=user, expire_time=expire_time)
+        return dict(user=c.project.user_project_of, expire_time=expire_time)
 
     @require_post()
     @expose()
     @validate(dict(subject=validators.NotEmpty,
                    message=validators.NotEmpty))
     def send_user_message(self, subject='', message='', cc=None):
-        user = c.project.user_project_of
-        both_have_emails = user and user.get_pref('email_address') and c.user.get_pref('email_address')
-        if not both_have_emails:
-            raise exc.HTTPNotFound()
+        """Handle POST for sending a message to another user.
+
+        """
+        self._check_can_message(c.user, c.project.user_project_of)
 
-        time_interval = config['user_message.time_interval']
-        max_messages = config['user_message.max_messages']
         if cc:
             cc = c.user.get_pref('email_address')
-        user = c.project.user_project_of
-        if c.user.check_send_emails_times(time_interval, max_messages):
-            c.user.send_user_message(user, subject, message, cc)
+        if c.user.can_send_user_message():
+            c.user.send_user_message(c.project.user_project_of, subject, message, cc)
+            flash("Message sent.")
         else:
-            flash("You can't send more than %s messages per %s seconds" % (max_messages,
time_interval), 'error')
-        return redirect(user.url())
+            flash("You can't send more than %i messages per %i seconds" % (
+                c.user.user_message_max_messages,
+                c.user.user_message_time_interval), 'error')
+        return redirect(c.project.user_project_of.url())
 

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/lib/app_globals.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/app_globals.py b/Allura/allura/lib/app_globals.py
index 14fa5a1..88cb43b 100644
--- a/Allura/allura/lib/app_globals.py
+++ b/Allura/allura/lib/app_globals.py
@@ -433,6 +433,22 @@ class Globals(object):
         return asbool(config.get('debug')) == False
 
     @LazyProperty
+    def user_message_time_interval(self):
+        """The rolling window of time (in seconds) during which no more than
+        :meth:`user_message_max_messages` may be sent by any one user.
+
+        """
+        return int(config.get('user_message.time_interval', 3600))
+
+    @LazyProperty
+    def user_message_max_messages(self):
+        """The number of user messages that can be sent within
+        meth:`user_message_time_interval` before rate-limiting is enforced.
+
+        """
+        return int(config.get('user_message.max_messages', 20))
+
+    @LazyProperty
     def server_name(self):
         p1 = Popen(['hostname', '-s'], stdout=PIPE)
         server_name = p1.communicate()[0].strip()

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/model/auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/auth.py b/Allura/allura/model/auth.py
index 228c3ed..a4be2cd 100644
--- a/Allura/allura/model/auth.py
+++ b/Allura/allura/model/auth.py
@@ -27,13 +27,12 @@ from email import header
 from hashlib import sha256
 import uuid
 from pytz import timezone
-from datetime import timedelta, date, datetime, time
+from datetime import timedelta, datetime, time
 
 import iso8601
 import pymongo
 from pylons import tmpl_context as c, app_globals as g
 from pylons import request
-from tg import flash
 
 from ming import schema as S
 from ming import Field, collection
@@ -334,7 +333,7 @@ class User(MappedClass, ActivityNode, ActivityObject):
         end_time=dict(h=int, m=int))])
     localization=FieldProperty(dict(city=str,country=str))
     timezone=FieldProperty(str)
-    send_emails_times=FieldProperty([S.DateTime])
+    sent_user_message_times=FieldProperty([S.DateTime])
     inactiveperiod=FieldProperty([dict(
         start_date=S.DateTime,
         end_date=S.DateTime)])
@@ -354,26 +353,46 @@ class User(MappedClass, ActivityNode, ActivityObject):
     #Statistics
     stats_id = FieldProperty(S.ObjectId, if_missing=None)
 
-    def check_send_emails_times(self, time_interval, max_messages):
-        times = []
-        time_interval = timedelta(seconds=int(time_interval))
-        for t in self.send_emails_times:
-            if t + time_interval > datetime.utcnow():
-                times.append(t)
-        self.send_emails_times = times
-        return len(times) < int(max_messages)
+    def can_send_user_message(self):
+        """Return true if User is permitted to send a mesage to another user.
+
+        Returns False if User has exceeded the user message rate limit, in
+        which case another message may not be sent until sufficient time has
+        passed to clear the limit.
+
+        """
+        now = datetime.utcnow()
+        time_interval = timedelta(seconds=g.user_message_time_interval)
+        self.sent_user_message_times = [t for t in self.sent_user_message_times
+                if t + time_interval > now]
+        return len(self.sent_user_message_times) < g.user_message_max_messages
+
+    def time_to_next_user_message(self):
+        """Return a timedelta of the time remaining before this user can send
+        another user message.
+
+        Returns zero if user message can be sent immediately.
+
+        """
+        if self.can_send_user_message():
+            return 0
+        return self.sent_user_message_times[0] + \
+                timedelta(seconds=g.user_message_time_interval) - \
+                datetime.utcnow()
 
     def send_user_message(self, user, subject, message, cc):
+        """Send a user message (email) to ``user``.
+
+        """
         allura.tasks.mail_tasks.sendsimplemail.post(
             toaddr=user.get_pref('email_address'),
-            fromaddr=c.user.get_pref('email_address'),
-            reply_to=c.user.get_pref('email_address'),
+            fromaddr=self.get_pref('email_address'),
+            reply_to=self.get_pref('email_address'),
             message_id=h.gen_message_id(),
             subject=subject,
             text=message,
             cc=cc)
-        self.send_emails_times.append(datetime.utcnow())
-        flash("email sent")
+        self.sent_user_message_times.append(datetime.utcnow())
 
     @property
     def activity_name(self):

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/tests/functional/test_user_profile.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_user_profile.py b/Allura/allura/tests/functional/test_user_profile.py
index 2727a07..7b17d28 100644
--- a/Allura/allura/tests/functional/test_user_profile.py
+++ b/Allura/allura/tests/functional/test_user_profile.py
@@ -18,6 +18,7 @@
 from formencode.variabledecode import variable_encode
 
 import mock
+from nose.tools import assert_equal
 
 from allura.model import Project, User
 from allura.tests import decorators as td
@@ -64,12 +65,12 @@ class TestUserProfile(TestController):
     @td.with_user_project('test-user')
     @mock.patch('allura.tasks.mail_tasks.sendsimplemail')
     @mock.patch('allura.lib.helpers.gen_message_id')
-    @mock.patch('allura.model.User.check_send_emails_times')
+    @mock.patch('allura.model.User.can_send_user_message')
     def test_send_message(self, check, gen_message_id, sendsimplemail):
         check.return_value = True
         gen_message_id.return_value = 'id'
         test_user = User.by_username('test-user')
-        test_user.set_pref('email_address', 'test-user@sf.net')
+        test_user.set_pref('email_address', 'test-user@example.com')
         response = self.app.get('/u/test-user/profile/send_message', status=200)
         assert '<b>From:</b> &#34;Test Admin&#34; &lt;test-admin@users.localhost&gt;'
in response
         self.app.post('/u/test-user/profile/send_user_message',
@@ -101,23 +102,27 @@ class TestUserProfile(TestController):
 
         check.return_value = False
         response = self.app.get('/u/test-user/profile/send_message', status=200)
-        assert 'Sorry, you can send an email after' in response
+        assert 'Sorry, messaging is rate-limited' in response
 
     @td.with_user_project('test-user')
     def test_send_message_for_anonymous(self):
-        self.app.get('/u/test-user/profile/send_message',
+        r = self.app.get('/u/test-user/profile/send_message',
                      extra_environ={'username': '*anonymous'},
-                     status=404)
+                     status=302)
+        assert 'You must be logged in to send user messages.' in self.webflash(r)
 
-        self.app.post('/u/test-user/profile/send_user_message',
+        r = self.app.post('/u/test-user/profile/send_user_message',
                       params={'subject': 'test subject',
                               'message': 'test message',
                               'cc': 'on'},
                       extra_environ={'username': '*anonymous'},
-                      status=404)
+                      status=302)
+        assert 'You must be logged in to send user messages.' in self.webflash(r)
 
     @td.with_user_project('test-user')
     def test_link_to_send_message_form(self):
+        User.by_username('test-admin').set_pref('email_address', 'admin@example.com')
+        User.by_username('test-user').set_pref('email_address', 'user@example.com')
         r = self.app.get('/u/test-user/profile',
                          status=200)
         assert '<a href="send_message">Send me a message</a>' in r

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/7eb73bcc/Allura/allura/tests/model/test_auth.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/model/test_auth.py b/Allura/allura/tests/model/test_auth.py
index fe80fac..a2bb2da 100644
--- a/Allura/allura/tests/model/test_auth.py
+++ b/Allura/allura/tests/model/test_auth.py
@@ -24,8 +24,8 @@ from nose.tools import with_setup, assert_equal
 from pylons import tmpl_context as c, app_globals as g
 from webob import Request
 from datetime import datetime, timedelta
-import mock
 
+from mock import patch
 from pymongo.errors import DuplicateKeyError
 from ming.orm.ormsession import ThreadLocalORMSession
 
@@ -33,7 +33,6 @@ from allura import model as M
 from allura.lib import plugin
 from allura.tests import decorators as td
 from alluratest.controller import setup_basic_test, setup_global_objects
-from allura.tasks import mail_tasks
 
 
 def setUp():
@@ -228,13 +227,15 @@ def test_user_projects_by_role():
     assert_equal(set(p.shortname for p in c.user.my_projects()), set(['test', 'test2', 'u/test-admin',
'adobe-1', '--init--']))
     assert_equal(set(p.shortname for p in c.user.my_projects('Admin')), set(['test', 'u/test-admin',
'adobe-1', '--init--']))
 
-
-def test_check_send_emails_times():
+@patch.object(g, 'user_message_max_messages', 3)
+def test_check_sent_user_message_times():
     user1 = M.User.register(dict(username='test-user'), make_project=False)
     time1 = datetime.utcnow() - timedelta(minutes=30)
     time2 = datetime.utcnow() - timedelta(minutes=45)
     time3 = datetime.utcnow() - timedelta(minutes=70)
-    user1.send_emails_times = [time1, time2, time3]
-    assert not user1.check_send_emails_times(3600, 1)
-    assert_equal(len(user1.send_emails_times), 2)
-    assert user1.check_send_emails_times(3600, 3)
+    user1.sent_user_message_times = [time1, time2, time3]
+    assert user1.can_send_user_message()
+    assert_equal(len(user1.sent_user_message_times), 2)
+    user1.sent_user_message_times.append(
+            datetime.utcnow() - timedelta(minutes=15))
+    assert not user1.can_send_user_message()


Mime
View raw message