I wonder if you should simply deduce this "inactive" property dynamically, for example based on the last login date? After 6 months?

it's fine to me. I just wonder if 6 months isn't a bit long... 2 or 3 months would be great. is it worth to let the coordinator set this property on admin page per team?

I think 6 months fine, because it's a GNOME release time.
But deduce this "inactive" dynamically is not ok, for me. For two reasons:

- Performance. If you have a large numbers of users/translators, calculate this at run time can be relatively slow. Then, we could add a field model that represent this property, but also put some sort of cron job that will update this field automatically.

- We need to "inactive" a user for another reason. However, for now, I don't have anything in mind.

I will try send this patch, ASAP.

(In reply to comment #3)
> - Performance. If you have a large numbers of users/translators, calculate this
> at run time can be relatively slow. Then, we could add a field model that
> represent this property, but also put some sort of cron job that will update
> this field automatically.

As we already fetch the user data from the DB and the last_login field is then at hand, I don't think a simple date comparison for each user would compromise performance.

Created attachment 174358 [details] [review]
Add method is_active in Person

Claude,

I finished the patch.
But I and John Wendell don't liked this.

Jonh Wendell says on irc channel:
"
It's better to let the team coordinator to change the status of an individual rather than do it automatically specially in cases where 1 person belongs to 2 teams, he can be inactive in only one team so, it gives more flexibility and also it's easier to implement (I guess)
"

Then active/inactive must be the roles, not the person.

What do you think?

I admit setting an inactive flag in the Role model would give the maximum flexibility. However, it adds another piece of data to manually keep up-to-date, that's why I'm not very enthousiast about it.

Let's take the automatic route first, and we can always add a flag in a model later.

To finish the patch, you still need to reflect the inactive property in the team list display. Jonh proposed on IRC that inactive translators are listed separately on the bottom of the team page.

Review of attachment 174358 [details] [review]:

::: people/models.py
@@ +20,3 @@
 # 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
+from datetime import *

I don't like the import * syntax. I'd rather "from datetime import timedelta, datetime".

@@ +92,3 @@
+    def is_active_person(self):
+        timedelta = datetime.now() - self.last_login
+        months = timedelta.days/30  

The class is already Person, so no need to add "_person" to the method name

::: people/tests/__init__.py
@@ +36,3 @@
         self.assertRedirects(response, reverse('register_success'))
+        
+    def test_is_active_person(self):

I don't like testing on object properties which are in fixtures. Fixtures are good for general data loading, but when you have to test a specific property, create that property in the test also, so you don't have to dive into fixtures when something don't work as expected.

Review of attachment 174358 [details] [review]:

- I fixed the import
- The method name has "_person" because Person inherits from User, and User has is_active, but these are different things. Or not?
- I removed the fixture and adapted the test.
- Additional, I rewrited the method.

Now, I'll update the team list display.

As discussed on IRC, here is the way to go:

1. Create a field in the Role model to store the inactive flag
2. Automatically inactivate when login older than 6 months (all roles)
3. Automatic activation when he submits a translation (matching team role)
4. Add an UI for coordinators to manually mark a translator as inactive

Created attachment 176779 [details] [review]
Patch to 1th-3th steps

Created attachment 176813 [details] [review]
Adorilson's patch reviewed

Here is my review of your patch. Feel free to ask me if there are any question about it. Many thanks for your great work!

Step 4 just committed. Thanks Adorilson.
http://git.gnome.org/browse/damned-lies/commit/?id=aa37623