Comment 1 Sönke Schwardt-Krummrich 2016-05-26 11:16:39 CEST

> * do not delete, but deactivate with an expiration data
>   - users past the expiration data can be deleted (should this be
> implemented?)

→ I think this can be implemented in an additional script, that searches and optionally deletes users whose account is expired for at least X days. But for now, this has only low priority and when the worst comes to the worst I can be delivered after the release as 4.1R2 erratum. 

>   - if a user should be created and deactivated user with the same SUID+RUID
> exists → reactivate the account instead

→ correct

> * store an expiration data (that is in the future) for the user, so it can
> access its data for some days, before deletion

→ correct

Comment 2 Daniel Tröder 2016-05-30 17:30:52 CEST

Configuration for this is done like this:

"user_deletion": {
    "delete":	bool: if the user should be deleted (false -> it will be deactivated)
    "expiration": int: number of days before the account will be deleted or deactivated
}


> * simply delete (already implemented ;)
user_deletion": {
    "delete": True,
    "expiration": 0
}

> * do not delete, but deactivate with an expiration data
>   - users past the expiration data can be deleted
user_deletion": {
    "delete": False,
    "expiration": 7
}

> * store an expiration data (that is in the future) for the user, so it can
> access its data for some days, before deletion
user_deletion": {
    "delete": True,
    "expiration": 7
}

Comment 3 Daniel Tröder 2016-05-31 09:23:11 CEST

Question: should reactivated users be reported as added or as modified users?

Comment 4 Sönke Schwardt-Krummrich 2016-05-31 09:43:20 CEST

(In reply to Daniel Tröder from comment #3)
> Question: should reactivated users be reported as added or as modified users?

Good question: if possible, as "reactivated" otherwise I would prefer "modified".

Comment 5 Daniel Tröder 2016-05-31 15:57:00 CEST

Implemented in r69656.

To add or change a deletion variant, overwrite ucsschool.importer.mass_import.user_import.UserImport.do_delete()


I'm not really happy with
    user_deletion": {
      "delete": boot,
      "expiration": int
    }
The delete=False is interpreted as "deactivate", but that is not intuitive.


(In reply to Sönke Schwardt-Krummrich from comment #4)
> (In reply to Daniel Tröder from comment #3)
> > Question: should reactivated users be reported as added or as modified users?
> 
> Good question: if possible, as "reactivated" otherwise I would prefer
> "modified".
For now users are listed as 'modified'. Wouldn't be difficult to add a list of 'reactivated' users to UserImport and to list them separately in UserImport.log_stats().

Comment 6 Sönke Schwardt-Krummrich 2016-07-05 15:09:49 CEST

Please write a ucs-test script resp. extend 34_import-users_via_cli_v2.

Comment 7 Daniel Tröder 2016-09-09 15:08:49 CEST

Writing a test for this bug revealed a unsafe combination of account expiration with User.disabled and calling User.modify() twice.

r72463: hand over account expiration date to ucsschool lib

Comment 8 Daniel Tröder 2016-09-09 15:28:43 CEST

r72465 is a followup to the previous change (removing the dependency on the LDAP object for expire())

Comment 9 Daniel Tröder 2016-09-09 15:42:05 CEST

r72466: added test for different deletion methods to 90_ucsschool/34_import-users_via_cli_v2 in test_delete_variants()