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

Download 502.patch
no initial comment

rebased onto ab115504949b894b14960884295aaded23fd6d60

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

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

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

rebased onto 6965a78b52a0685712f3fbcbfa10eb22d460d165

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

rebased onto 668b245ffeac9fa43696f59e5d0325f71412df5d

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

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 6fd9c699e656eda0fb7b2e977addb2ed20baa7e4

rebased onto 6fd9c699e656eda0fb7b2e977addb2ed20baa7e4

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 89a52423 fixes this pull-request

Pull-Request has been merged by abompard

Pull-Request has been merged by abompard

Metadata