Page MenuHomePhorge

Deleting users/accounts
ClosedPublic

Authored by machniak on Mar 5 2020, 1:59 PM.
Tags
None
Referenced Files
F11592534: D1000.id2524.diff
Fri, Mar 29, 6:20 AM
Unknown Object (File)
Wed, Mar 27, 6:19 PM
Unknown Object (File)
Tue, Mar 26, 8:33 PM
Unknown Object (File)
Sun, Mar 24, 1:16 PM
Unknown Object (File)
Mon, Mar 18, 5:11 AM
Unknown Object (File)
Thu, Mar 7, 7:29 PM
Unknown Object (File)
Thu, Mar 7, 10:58 AM
Unknown Object (File)
Tue, Mar 5, 2:59 PM
Subscribers
Restricted Project

Details

Reviewers
vanmeeuwen
Group Reviewers
Restricted Project
Commits
rK1035ab7af7aa: Deleting users/accounts
Summary
  • Implement proper users listing, added tests for it
  • Deleting users/accounts

TODO: more browser tests

Test Plan

./phpunit

Diff Detail

Repository
rK kolab
Branch
dev/user-delete
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 28429
Build 10183: arc lint + arc unit

Event Timeline

machniak created this revision.
machniak retitled this revision from Implement proper users listing, added tests for it to Deleting users/accounts.Mar 5 2020, 2:01 PM
machniak edited the summary of this revision. (Show Details)
machniak edited the test plan for this revision. (Show Details)
machniak added a reviewer: Restricted Project.
machniak added a subscriber: Restricted Project.
vanmeeuwen subscribed.
vanmeeuwen added inline comments.
src/app/Observers/DomainObserver.php
54

Shouldn't this be a progression on the dispatched deletion process having completed?

57

How about $domain->save()?

This revision now requires changes to proceed.Mar 5 2020, 3:58 PM
src/app/Observers/DomainObserver.php
54

It could, indeed. Then we should change isDeleted() so it also returns true when !empty($this->deleted_at), or not? I don't want to make things more complicated at the moment.

57

save() would work, but would generate updated/updating event which we don't need/want.

src/app/Observers/DomainObserver.php
54

The question is then how the complete process of deleting a domain (or a user for that matter) is reflected in real life, vs. the database record.

I believe I intended for the database to reflect in 'deleted_at' the semantics for UI/UX -- including billing and pro rata temporis if you will, and for STATUS_DELETED to be the final confirmation.

In a sense, deleting could spawn the dispatched process -- it creates a series of actions to need to have occurred much like creating a user does (ldap ready, imap ready).

I think it is there where I figured STATUS_DELETED would add value compared to just deleted_at.

57

I think we need to settle what deleting and deleted is compared to deleted_at vs. STATUS_DELETED, before we need to consider $domain->save() and its triggering of updating()/updated().

src/app/Observers/UserObserver.php
95

Think... "remove entitlements billed to wallets owned".

When john@kolab.org owns a wallet to which all entitlements associated with any @kolab.org-related entitlements are billed -- and how would he not?? --, then iterating over his wallets and entitlements billed to said wallets is a very predictable path.

  • Set domain/user STATUS_DELETED after removing from ldap
  • Added more browser tests (that are failing at the moment)
  • Fixes regarding wallets
  • Use wallets to control access to UI elements
  • Fix dispatching DomainDelete job - use ID instead of the Model object

Will require rebasing on to current master.

src/resources/vue/components/User/Profile.vue
60

This is where a merge on to current origin/master fails. Please double-check the intended result.

This revision now requires changes to proceed.Mar 9 2020, 2:18 PM
src/app/Jobs/DomainDelete.php
30

$domain -> $domain_id.

src/app/User.php
373

This should really be:

return $this->select(['users.*', 'entitlements.wallet_id'])

because Query\Builder::select() only supports 0-1 parameters (see phpstan output).

  • Rebase
  • Small refactoring regarding wallets/accounts
  • Remove redundant code
  • Make UserVerifyTest less fragile - wait up to 10 seconds
This revision is now accepted and ready to land.Mar 10 2020, 11:08 AM
This revision was automatically updated to reflect the committed changes.