#502 make members widget match the mockup
Merged 6 years ago by abompard. Opened 6 years ago by ryanlerch.
ryanlerch/fedora-hubs members-widget  into  develop

file modified
+1
@@ -115,6 +115,7 @@ 

          user = hubs.models.User.get_or_create(

              username=flask.session["auth"]["nickname"],

              fullname=flask.session["auth"]["fullname"])

+         user.last_active = datetime.datetime.utcnow()

          flask.g.user = user

      else:

          flask.g.auth = munch.Munch(logged_in=False)

@@ -0,0 +1,45 @@ 

+ # This Alembic database migration is part of the Fedora Hubs project.

+ # Copyright (C) 2018 The Fedora Project

+ #

+ # This program is free software: you can redistribute it and/or modify

+ # it under the terms of the GNU Affero General Public License as published by

+ # the Free Software Foundation, either version 3 of the License, or

+ # (at your option) any later version.

+ #

+ # This program is distributed in the hope that it will be useful,

+ # but WITHOUT ANY WARRANTY; without even the implied warranty of

+ # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

+ # GNU Affero General Public License for more details.

+ #

+ # You should have received a copy of the GNU Affero General Public License

+ # along with this program.  If not, see <http://www.gnu.org/licenses/>.

+ """

+ add user last activity field

+ 

+ Revision ID: a245837dd23c

+ Revises: 20b23e867aeb

+ Create Date: 2018-01-05 07:32:29.858380

+ """

+ 

+ from __future__ import absolute_import, unicode_literals

+ 

+ from alembic import op

+ import sqlalchemy as sa

+ 

+ 

+ # revision identifiers, used by Alembic.

+ revision = 'a245837dd23c'

+ down_revision = u'20b23e867aeb'

+ branch_labels = None

+ depends_on = None

+ 

+ 

+ def upgrade():

+     op.add_column('users', sa.Column('last_active',

+                                      sa.DateTime(),

+                                      nullable=True))

+ 

+ 

+ def downgrade():

+     with op.batch_alter_table("users") as batch_op:

+         batch_op.drop_column('last_active')

file modified
+3
@@ -49,6 +49,7 @@ 

      saved_notifications = relation(

          'SavedNotification', backref='user', lazy='dynamic',

          order_by=SavedNotification.created.desc())

+     last_active = sa.Column(sa.DateTime, default=datetime.datetime.utcnow)

  

      def __json__(self):

          return {
@@ -56,6 +57,8 @@ 

              'avatar': username2avatar(self.username),

              'fullname': self.fullname,

              'created_on': self.created_on,

+             'last_active': self.last_active,

+ 

              # We'll need hubs subscribed to, owned, etc..

              # 'hubs': [hub.idx for hub in self.hubx],

          }

file modified
+5
@@ -2,6 +2,7 @@ 

  

  import os

  from hashlib import sha256, md5

+ import humanize

  

  import fedmsg.config

  import fedmsg.meta
@@ -46,3 +47,7 @@ 

              "%s</div>") % (hubname2monogramcolour(hub.name),

                             hubname2monogramcolour(hub.name),

                             hub.name[0].upper())

+ 

+ 

+ def relative_time(date):

+     return humanize.naturaltime(date)

file modified
+2 -1
@@ -8,7 +8,7 @@ 

  from importlib import import_module

  

  from hubs.models.constants import HUB_TYPES

- from hubs.utils import hub2groupavatar

+ from hubs.utils import hub2groupavatar, relative_time

  

  from .caching import CachedFunction

  from .view import WidgetView
@@ -142,6 +142,7 @@ 

              )

              env.filters['tojson'] = flask.json.tojson_filter

              env.filters['hub2groupavatar'] = hub2groupavatar

+             env.filters['relative_time'] = relative_time

              env.globals.update({

                  'session': flask.app.session,

                  'g': flask.g,

@@ -1,10 +1,12 @@ 

  from __future__ import unicode_literals

  

+ from datetime import datetime

+ 

  from hubs.widgets.base import Widget

  from hubs.widgets.view import RootWidgetView

  

  

- ELLIPSIS_LIMIT = 3

+ ELLIPSIS_LIMIT = 5

  

  """

  This widget displays who is a member of the Hub it is placed on.
@@ -16,6 +18,7 @@ 

      name = "memberships"

      label = "Members"

      position = "both"

+     hub_types = ['team']

  

  

  class BaseView(RootWidgetView):
@@ -35,7 +38,10 @@ 

                  owners.append(member.username)

  

          oldest_members = sorted(

-             members, key=lambda m: m.get('created_on'))[:ELLIPSIS_LIMIT]

+             members,

+             key=lambda m: m['last_active'] or datetime.utcfromtimestamp(0),

+             reverse=True)[:ELLIPSIS_LIMIT]

+ 

          return dict(

              memberships=list(members),

              oldest_members=list(oldest_members),

@@ -1,32 +1,24 @@ 

- <div class="card-block">

-   {% if memberships|length > oldest_members|length %}

-   {% for member in oldest_members %}

-   <div class="card-block">

-     <img class="img-responsive membership-avatar" src="{{ member.avatar }}" alt="Hub avatar for {{ member.name }}">

-     <a href="{{ url_for('hub', hub_name=member.username, hub_type='u')}}">{{ member.username }}</a>

-     {% if member.username in owners %}

-       <p class="text-muted">Owner</p>

-     {% else %}

-       <p>Member</p>

-     {% endif %}

-   </div>

-   {% endfor %}

-   <a href="#" class="btn btn-secondary" data-toggle="modal" data-target="#membersModal">View All</a>

-   {% else %}

-   {% for member in memberships %}

-   <div class="card-block">

-     <img class="img-responsive membership-avatar" src="{{ member.avatar }}" alt="Hub avatar for {{ member.name }}">

-     <a href="{{ url_for('hub', hub_name=member.username, hub_type='u')}}">{{ member }}</a>

-     {% if member.username in owners %}

-       <p class="text-muted">Owner</p>

-     {% else %}

-       <p>Member</p>

+     <div class="list-group list-group-flush membership-list">

+         {% for member in oldest_members %}        

+       <a class="list-group-item list-group-item-action d-flex justify-content-between align-items-center" href="{{ url_for('hub', hub_name=member.username, hub_type='u')}}">

+         <div class="d-flex">

+           <img class="img-responsive membership-avatar" src="{{ member.avatar }}" alt="Hub avatar for {{ member.name }}">

+           <div class="align-self-center ml-3">

+             <h5 class="text-primary"><strong>{{ member.username }}</strong></h5>

+             {% if member.last_active %}

+             <div><small class="text-muted">last active {{ member.last_active | relative_time }}</small></div>

+             {% endif %}

+           </div>

+         </div>

+       </a>

+       {% endfor %}

+       

+     </div>

+     {% if memberships|length > oldest_members|length %}   

+       <a href="#" class="btn btn-secondary mt-2" data-toggle="modal" data-target="#membersModal">View All {{memberships|length}} Members</a>

      {% endif %}

-   </div>

-   {% endfor %}

-   {%endif%}

- </div>

  

+ {% if memberships|length > oldest_members|length %}   

  <!-- View all modal -->

  <div class="modal fade" id="membersModal" tabindex="-1" role="dialog" aria-labelledby="membersModal" aria-hidden="true">

    <div class="modal-dialog">
@@ -38,20 +30,15 @@ 

          <div class="card-block">

            <div class="row">

              {% for member in memberships %}

-             <div class="col-sm-6">

-               <img class="img-responsive membership-avatar"

-                 src="{{ member.avatar }}" alt="Hub avatar for {{ member.username }}">

-               <a href="{{ url_for('hub', hub_name=member.username, hub_type='u')}}">{{ member.username }}</a>

-               {% if member.username in owners %}

-               <p class="text-muted">Owner</p>

-               {% else %}

-               <p>Member</p>

-               {% endif %}

-             </div>

-             {% if (loop.index % 2) == 0 %}

-               </div>

-               <div class="row">

-             {% endif %}

+             <a class="list-group-item list-group-item-action d-flex justify-content-between align-items-center" href="{{ url_for('hub', hub_name=member.username, hub_type='u')}}">

+                 <div class="d-flex">

+                   <img class="img-responsive membership-avatar" src="{{ member.avatar }}" alt="Hub avatar for {{ member.name }}">

+                   <div class="align-self-center ml-3">

+                     <h5 class="text-primary"><strong>{{ member.username }}</strong></h5>

+                     <div><small class="text-muted">last active {{ member.last_active | relative_time }}</small></div>

+                   </div>

+                 </div>

+               </a>

            {% endfor %}

          </div>

          <div class="modal-footer clearfix">
@@ -60,12 +47,19 @@ 

        </div>

      </div>

    </div>

- 

+ {% endif %}

+   

  <style>

   .membership-avatar {

-    height: 4em;

-    width: 4em;

+    height: 72px;

+    width: 72px;

+  }

+ 

+  .modal-body .membership-avatar {

+    height: 42px;

+    width: 42px;

   }

+ 

   .modal-header {

     border-bottom: 0;

   }
@@ -73,4 +67,8 @@ 

   #owners {

     padding: 10px;

   }

+ 

+  .membership-list .list-group-item{

+    border:none!important;

+  }

  </style>

file modified
+1
@@ -16,6 +16,7 @@ 

  fmn.rules

  gunicorn

  html5lib==0.9999999

+ humanize

  iso3166

  markdown

  munch

no initial comment

rebased onto ab115504949b894b14960884295aaded23fd6d60

6 years ago

Since you've added a field to the model, you need to make a migration file with Alembic. You'll find the command to run in hubs/migrations/README.rst.

Since you've added a field to the model, you need to make a migration file with Alembic. You'll find the command to run in hubs/migrations/README.rst.

done!

rebased onto 098e79a328146720e846e0de419c5a9ce780e6d9

6 years ago

The file seems wrong, there is no action in the upgrade or downgrade functions.
You have to run Alembic when your current database has the old format and your code has the new attribute, so it can detect the difference and generate the migration file.

(also, please set the copyright date at the top)

Okies, i think i fixed it properly now!

rebased onto 753d09b752e2e793483e78d62b8405302af8eda2

6 years ago

OK, two more things about the migration file:

  • Can you remove the ### comment generated by Alembic?
  • The plain drop_column operation will fail on SQLite because it does not support it (sadly), please use the mechanism involving batch_op that you'll see in bed8bbc0f78e_authz.py for example.

rebased onto 9a167f0a30469292b8c605d1e93dd89f933088d6

6 years ago

rebased onto 6965a78b52a0685712f3fbcbfa10eb22d460d165

6 years ago

okies, tweaked the migration as suggested, also rebased and merged the changes from the recent streams mega PR.

rebased onto 668b245ffeac9fa43696f59e5d0325f71412df5d

6 years ago

just rebased to latest develop.

Since the last_active timestamp is updated on login, there are cases where a group member will contain people who have never logged in. In that case last_active is None and the widget crashes. I suggest the following modifications:

diff --git a/hubs/widgets/memberships/__init__.py b/hubs/widgets/memberships/__init__.py
index 8455e1e5..17318494 100644
--- a/hubs/widgets/memberships/__init__.py
+++ b/hubs/widgets/memberships/__init__.py
@@ -1,5 +1,7 @@
 from __future__ import unicode_literals

+from datetime import datetime
+
 from hubs.widgets.base import Widget
 from hubs.widgets.view import RootWidgetView

@@ -37,7 +39,7 @@ class BaseView(RootWidgetView):

         oldest_members = sorted(
             members,
-            key=lambda m: m.get('last_active'),
+            key=lambda m: m['last_active'] or datetime.utcfromtimestamp(0),
             reverse=True)[:ELLIPSIS_LIMIT]

         return dict(
diff --git a/hubs/widgets/memberships/templates/root.html b/hubs/widgets/memberships/templates/root.html
index 0d3b74e9..90b05fab 100644
--- a/hubs/widgets/memberships/templates/root.html
+++ b/hubs/widgets/memberships/templates/root.html
@@ -5,7 +5,9 @@
           <img class="img-responsive membership-avatar" src="{{ member.avatar }}" alt="Hub avatar for {{ member.name }}">
           <div class="align-self-center ml-3">
             <h5 class="text-primary"><strong>{{ member.username }}</strong></h5>
+            {% if member.last_active %}
             <div><small class="text-muted">last active {{ member.last_active | relative_time }}</small></div>
+            {% endif %}
           </div>
         </div>
       </a>

rebased onto 72a62236ea6e96a6ce32745005ce8df32fa2fb60

6 years ago

Applied your changes @abompard.

However, i must have been a little confused as to how the default DB values work -- i assumed that this line in the model set the last active value when the user was created:

https://pagure.io/fedora-hubs/pull-request/502#3_5

rebased onto 6fd9c69

6 years ago

rebased onto 6fd9c69

6 years ago

You're correct, I got the bug because I used an existing user, so the actual issue was that the migration did not set the value for existing user.
You could also set nullable=False in that field.

should we leave this as-is? i.e. are you happy? or do you want me to change it?

Commit 89a5242 fixes this pull-request

Pull-Request has been merged by abompard

6 years ago

Pull-Request has been merged by abompard

6 years ago