From 796c72dd09bc668cd4ac1abfd84f03b7f369a9b0 Mon Sep 17 00:00:00 2001 From: Deon George Date: Thu, 21 Apr 2022 14:41:26 +1000 Subject: [PATCH] Home screen improvements, testing for role, work on user/account models --- app/Http/Controllers/HomeController.php | 23 +-- app/Models/Account.php | 55 +++--- app/Models/Rtm.php | 17 +- app/Models/Service.php | 19 +- app/Models/User.php | 164 ++++++++++++++---- app/Traits/QueryCacheableConfig.php | 17 ++ app/Traits/ScopeServiceUserAuthorised.php | 4 +- database/factories/RtmFactory.php | 27 +++ database/factories/UserFactory.php | 64 +++++-- .../2022_04_20_183016_updates_to_accounts.php | 48 +++++ .../common/account/widget/summary.blade.php | 10 +- .../adminlte/r/account/widgets/list.blade.php | 35 ++-- .../theme/backend/adminlte/r/home.blade.php | 77 -------- .../adminlte/r/home/widgets/home.blade.php | 18 ++ .../theme/backend/adminlte/u/home.blade.php | 47 ++++- .../adminlte/u/invoice/widgets/next.blade.php | 10 +- .../u/service/widgets/active.blade.php | 2 +- tests/Feature/AccountRoleTest.php | 132 ++++++++++++++ 18 files changed, 528 insertions(+), 241 deletions(-) create mode 100644 app/Traits/QueryCacheableConfig.php create mode 100644 database/factories/RtmFactory.php create mode 100644 database/migrations/2022_04_20_183016_updates_to_accounts.php delete mode 100644 resources/views/theme/backend/adminlte/r/home.blade.php create mode 100644 resources/views/theme/backend/adminlte/r/home/widgets/home.blade.php create mode 100644 tests/Feature/AccountRoleTest.php diff --git a/app/Http/Controllers/HomeController.php b/app/Http/Controllers/HomeController.php index 9d8a1b7..effc8f6 100644 --- a/app/Http/Controllers/HomeController.php +++ b/app/Http/Controllers/HomeController.php @@ -29,26 +29,10 @@ class HomeController extends Controller */ public function home(User $o): View { - // If we are passed a user to view, we'll open up their home page. - if ($o->exists) { - $o->load(['accounts','services']); - return View('u.home',['o'=>$o]); - } + if (! $o->exists) + $o = Auth::user(); - // If User was null, then test and see what type of logged on user we have - $o = Auth::user(); - - switch (Auth::user()->role()) { - case 'customer': - return View('u.home',['o'=>$o]); - - case 'reseller': - case 'wholesaler': - return View('r.home',['o'=>$o]); - - default: - abort(404,'Unknown role: '.$o->role()); - } + return View('u.home',['o'=>$o]); } /** @@ -126,7 +110,6 @@ class HomeController extends Controller */ public function service_progress(Service $o,string $status) { - abort(500,'deprecated'); return redirect()->to($o->action($status) ?: url('u/service',$o->id)); } } \ No newline at end of file diff --git a/app/Models/Account.php b/app/Models/Account.php index ef238aa..bb70aa7 100644 --- a/app/Models/Account.php +++ b/app/Models/Account.php @@ -24,25 +24,17 @@ class Account extends Model implements IDs { use HasFactory,ScopeActive; - const CREATED_AT = 'date_orig'; - const UPDATED_AT = 'date_last'; + /* INTERFACES */ - protected $appends = [ - 'active_display', - 'name', - 'services_count_html', - 'switch_url', - ]; + public function getLIDAttribute(): string + { + return sprintf('%04s',$this->id); + } - public $dateFormat = 'U'; - - protected $visible = [ - 'id', - 'active_display', - 'name', - 'services_count_html', - 'switch_url', - ]; + public function getSIDAttribute(): string + { + return sprintf('%02s-%s',$this->site_id,$this->getLIDAttribute()); + } /* RELATIONS */ @@ -135,6 +127,7 @@ class Account extends Model implements IDs public function getActiveDisplayAttribute($value) { + abort(500,'deprecated'); return sprintf('%s',$this->active ? 'success' : 'danger',$this->active ? 'Active' : 'Inactive'); } @@ -143,6 +136,7 @@ class Account extends Model implements IDs */ public function getAccountIdAttribute() { + abort(500,'deprecated'); return $this->getAIDAttribute(); } @@ -151,6 +145,7 @@ class Account extends Model implements IDs */ public function getAccountIdUrlAttribute() { + abort(500,'deprecated'); return $this->getUrlAdminAttribute(); } @@ -175,41 +170,29 @@ class Account extends Model implements IDs */ public function getAIDAttribute() { + abort(500,'deprecated'); return $this->getSIDAttribute(); } /** - * Account Local ID + * Return the account name * - * @return string + * @return mixed|string */ - public function getLIDAttribute(): string - { - return sprintf('%04s',$this->id); - } - - public function getNameAttribute() + public function getNameAttribute(): string { return $this->company ?: ($this->user_id ? $this->user->SurFirstName : 'AID:'.$this->id); } public function getServicesCountHtmlAttribute() { + abort(500,'deprecated'); return sprintf('%s /%s',$this->services()->noEagerLoads()->where('active',TRUE)->count(),$this->services()->noEagerLoads()->count()); } - /** - * Account System ID - * - * @return string - */ - public function getSIDAttribute(): string - { - return sprintf('%02s-%s',$this->site_id,$this->getLIDAttribute()); - } - public function getSwitchUrlAttribute() { + abort(500,'deprecated'); return sprintf('',$this->user_id); } @@ -225,6 +208,7 @@ class Account extends Model implements IDs */ public function getUrlAdminAttribute(): string { + abort(500,'deprecated'); return sprintf('%s',$this->id,$this->account_id); } @@ -235,6 +219,7 @@ class Account extends Model implements IDs */ public function getUrlUserAttribute(): string { + abort(500,'deprecated'); return sprintf('%s',$this->id,$this->account_id); } diff --git a/app/Models/Rtm.php b/app/Models/Rtm.php index 53d9d21..eead218 100644 --- a/app/Models/Rtm.php +++ b/app/Models/Rtm.php @@ -2,10 +2,25 @@ namespace App\Models; +use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; class Rtm extends Model { - protected $table = 'ab_rtm'; + use HasFactory; + + protected $table = 'rtm'; public $timestamps = FALSE; + + /* RELATIONS */ + + /** + * Subordinate RTM entries + * + * @return \Illuminate\Database\Eloquent\Relations\HasMany + */ + public function children() + { + return $this->hasMany(self::class,'parent_id'); + } } \ No newline at end of file diff --git a/app/Models/Service.php b/app/Models/Service.php index 262a6da..f78c7ac 100644 --- a/app/Models/Service.php +++ b/app/Models/Service.php @@ -18,6 +18,7 @@ use Leenooks\Carbon; use Symfony\Component\HttpKernel\Exception\HttpException; use App\Interfaces\IDs; +use App\Traits\ScopeServiceUserAuthorised; /** * Class Service @@ -46,7 +47,7 @@ use App\Interfaces\IDs; // @todo All the methods/attributes in this file need to be checked. class Service extends Model implements IDs { - use HasFactory; + use HasFactory,ScopeServiceUserAuthorised; protected $appends = [ 'account_name', @@ -86,15 +87,11 @@ class Service extends Model implements IDs 'status', ]; - /* protected $with = [ - 'account.language', - 'charges', 'invoice_items', - 'product', + 'product.type.supplied', 'type', ]; - */ // @todo Change to self::INACTIVE_STATUS private $inactive_status = [ @@ -413,15 +410,6 @@ class Service extends Model implements IDs }); } - /** - * Only query records that the user is authorised to see - */ - public function scopeAuthorised($query,User $uo) - { - return $query - ->whereIN($this->getTable().'.account_id',$uo->all_accounts()->pluck('id')->unique()->toArray()); - } - /** * Find inactive services. * @@ -1345,6 +1333,7 @@ class Service extends Model implements IDs * @return Collection * @throws Exception * @todo Use self::isBilled(); + * @todo This query is expensive. */ public function next_invoice_items(bool $future,Carbon $billdate=NULL): Collection { diff --git a/app/Models/User.php b/app/Models/User.php index 5022bf1..0a1a310 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -2,29 +2,32 @@ namespace App\Models; +use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Notifications\Notifiable; use Illuminate\Foundation\Auth\User as Authenticatable; use Illuminate\Support\Collection; use Illuminate\Database\Eloquent\Collection as DatabaseCollection; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Session; use Laravel\Passport\HasApiTokens; use Leenooks\Carbon; use Leenooks\Traits\UserSwitch; use App\Notifications\ResetPassword as ResetPasswordNotification; -use App\Traits\SiteID; +use App\Traits\{QueryCacheableConfig,SiteID}; +/** + * Class User + * + * Attributes for users: + * + role : User's role + */ class User extends Authenticatable { - use HasApiTokens,Notifiable,UserSwitch,SiteID; + use HasFactory,HasApiTokens,Notifiable,UserSwitch,QueryCacheableConfig,SiteID; - protected $appends = [ - 'active_display', - 'services_count_html', - 'surfirstname', - 'switch_url', - 'user_id_url', - ]; + private const CACHE_TIME = 3600; protected $dates = [ 'created_at', @@ -51,18 +54,6 @@ class User extends Authenticatable 'remember_token', ]; - protected $visible = [ - 'active_display', - 'id', - 'level', - 'services_count_html', - 'switch_url', - 'surfirstname', - 'user_id_url', - ]; - - protected $with = ['accounts']; - /** * Role hierarchy order * @var array @@ -79,10 +70,14 @@ class User extends Authenticatable * The accounts that this user manages * * @return \Illuminate\Database\Eloquent\Relations\HasMany + * @note This cannot be loaded with "with"? */ public function accounts() { - return $this->hasMany(Account::class); + return $this->hasMany(Account::class) + ->orWhereIn('id',$this->rtm_accounts()->pluck('id')) + ->active() + ->with(['services']); } /** @@ -136,6 +131,16 @@ class User extends Authenticatable return $this->hasManyThrough(Payment::class,Account::class); } + /** + * Return the routes to market account for this user + * + * @return \Illuminate\Database\Eloquent\Relations\HasOneThrough + */ + public function rtm() + { + return $this->hasOneThrough(Rtm::class,Account::class); + } + /** * THe services this user has * @@ -181,6 +186,7 @@ class User extends Authenticatable public function getActiveDisplayAttribute($value) { + abort(500,'deprecated:'.__METHOD__); return sprintf('%s',$this->active ? 'primary' : 'danger',$this->active ? 'Active' : 'Inactive'); } @@ -223,6 +229,16 @@ class User extends Authenticatable return new Carbon($value); } + /** + * Return my accounts + * + * @return Collection + */ + public function getMyAccountsAttribute(): Collection + { + return $this->accounts->where('user_id',$this->id); + } + /** * @deprecated Use static::getFullNameAttribute() * @return mixed @@ -245,8 +261,18 @@ class User extends Authenticatable ->reverse(); } + /** + * Return a friendly string of this persons role + * @return string + */ + public function getRoleAttribute(): string + { + return ucfirst($this->role()); + } + public function getServicesCountHtmlAttribute() { + abort(500,'deprecated:'.__METHOD__); return sprintf('%s /%s',$this->services->where('active',TRUE)->count(),$this->services->count()); } @@ -257,16 +283,19 @@ class User extends Authenticatable public function getSwitchUrlAttribute() { + abort(500,'deprecated:'.__METHOD__); return sprintf('',$this->id); } public function getUserIdAttribute() { + abort(500,'deprecated:'.__METHOD__); return sprintf('%02s-%04s',$this->site_id,$this->id); } public function getUserIdUrlAttribute() { + abort(500,'deprecated:'.__METHOD__); return sprintf('%s',$this->id,$this->user_id); } @@ -327,7 +356,7 @@ class User extends Authenticatable return $query; } - /* GENERAL METHODS */ + /* METHODS */ /** * Determine if the user is an admin of the user with $id @@ -337,16 +366,19 @@ class User extends Authenticatable */ public function isAdmin($id): bool { - return $id AND $this->isReseller() AND in_array($id,$this->all_accounts()->pluck('user_id')->toArray()); + return $id AND $this->isReseller() AND $this->accounts->pluck('user_id')->contains($id); } /** * Get a list of accounts for the clients of this user * * @return DatabaseCollection + * @deprecated Use rtm_accounts() */ public function all_accounts(): DatabaseCollection { + throw new \Exception('deprecated'); + abort(500,'deprecated:'.__METHOD__); $result = new DatabaseCollection(); $clients = $this->all_clients(); @@ -376,7 +408,9 @@ class User extends Authenticatable * Get a list of clients that this user is responsible for. * * @param int $level - * @return Collection + * @param DatabaseCollection|null $clients + * @return DatabaseCollection + * @deprecated Use rtm_accounts() to determine this */ public function all_clients($level=0,DatabaseCollection $clients=NULL): DatabaseCollection { @@ -398,6 +432,10 @@ class User extends Authenticatable return $result; } + /** + * @return mixed + * @deprecated Use rtm_accounts() to determine this list + */ public function all_client_service_inactive() { $s = Service::InActive(); @@ -413,6 +451,7 @@ class User extends Authenticatable * * @param int $level * @return Collection + * @deprecated Use rtm_accounts() */ public function all_agents($level=0) { @@ -443,7 +482,7 @@ class User extends Authenticatable public function client_service_movements(): DatabaseCollection { return Service::active() - ->authorised($this) + ->serviceUserAuthorised($this) ->where('order_status','!=','ACTIVE') ->with(['account','product']) ->get(); @@ -558,7 +597,7 @@ class User extends Authenticatable ]) ->from($this->query_invoice_items(),'II') ->join('ab_invoice',['ab_invoice.id'=>'II.invoice_id']) - ->whereIN('account_id',$this->all_accounts()->pluck('id')->unique()->toArray()) + ->whereIN('account_id',$this->accounts->pluck('id')) ->where('ab_invoice.active',TRUE) ->groupBy(['invoice_id']); @@ -574,7 +613,7 @@ class User extends Authenticatable ]) ->from($this->query_payment_items(),'PI') ->join('payments',['payments.id'=>'PI.payment_id']) - ->whereIN('account_id',$this->all_accounts()->pluck('id')->unique()->toArray()) + ->whereIN('account_id',$this->accounts->pluck('id')) //->where('payments.active',TRUE) // @todo To implement ->groupBy(['invoice_id']); @@ -647,16 +686,67 @@ class User extends Authenticatable */ public function role() { - // If I have agents and no parent, I am the wholesaler - if (is_null($this->parent_id) AND ($this->all_agents()->count() OR $this->all_clients()->count())) - return 'wholesaler'; + // Cache our role for this session + $cache_key = sprintf('%s:%s:%s',$this->id,__METHOD__,Session::getId()); - // If I have agents and a parent, I am a reseller - elseif ($this->parent_id AND ($this->all_agents()->count() OR $this->all_clients()->count())) - return 'reseller'; + return Cache::remember($cache_key,self::CACHE_TIME,function() { + // Get the RTM for our accounts + $rtms = Rtm::whereIn('account_id',$this->accounts->pluck('id'))->get(); - // If I have no agents and a parent, I am a customer - elseif (! $this->all_agents()->count() AND ! $this->all_clients()->count()) - return 'customer'; + // If I have no parent, I am the wholesaler + if ($rtms->whereNull('parent_id')->count()) + return 'wholesaler'; + + // If I exist in the RTM table, I'm a reseller + else if ($rtms->count()) + return 'reseller'; + + // Otherwise a client + else + return 'customer'; + }); + } + + /** + * Return the accounts that this user can manage + * This method is a helper to User::accounts() - use $user->accounts instead + * + * @return Collection + */ + private function rtm_accounts(): Collection + { + return Account::whereIn('rtm_id',$this->rtm_list()->pluck('id')) + ->get(); + } + + /** + * Return the RTM hierarchy that this user can manage + * + * @param Rtm|null $rtm + * @return Collection + */ + public function rtm_list(Rtm $rtm=NULL): Collection + { + // If this user doesnt manage any accounts + if (! $this->exists || ! $this->rtm) + return collect(); + + $list = collect(); + + // Add this RTM to the list + if (! $rtm) { + $list->push($this->rtm); + $children = $this->rtm->children; + + } else { + $list->push($rtm); + $children =$rtm->children; + } + + // Capture any children + foreach ($children as $child) + $list->push($this->rtm_list($child)); + + return $rtm ? $list : $list->flatten()->unique(function($item) { return $item->id; }); } } \ No newline at end of file diff --git a/app/Traits/QueryCacheableConfig.php b/app/Traits/QueryCacheableConfig.php new file mode 100644 index 0000000..cfdeaa4 --- /dev/null +++ b/app/Traits/QueryCacheableConfig.php @@ -0,0 +1,17 @@ +whereIN('services.account_id',$uo->all_accounts()->pluck('id')->unique()->toArray()); + ->whereIN('services.account_id',$uo->accounts->pluck('id')); } -} +} \ No newline at end of file diff --git a/database/factories/RtmFactory.php b/database/factories/RtmFactory.php new file mode 100644 index 0000000..0d30d15 --- /dev/null +++ b/database/factories/RtmFactory.php @@ -0,0 +1,27 @@ + + */ +class RtmFactory extends Factory +{ + /** + * Define the model's default state. + * + * @return array + */ + public function definition() + { + return [ + 'id' => $this->faker->numberBetween(2048,65535), + //* 'site_id', // Needs to be passed in + //* 'account_id', // Needs to be passed in + // 'name', + //* 'parent_id', // Needs to be passed in + ]; + } +} diff --git a/database/factories/UserFactory.php b/database/factories/UserFactory.php index bbeb809..2881ee6 100644 --- a/database/factories/UserFactory.php +++ b/database/factories/UserFactory.php @@ -1,23 +1,49 @@ define(App\Models\User::class, function (Faker $faker) { - return [ - 'name' => $faker->name, - 'email' => $faker->unique()->safeEmail, - 'password' => '$2y$10$TKh8H1.PfQx37YgCzwiKb.KjNyWgaHb9cbcoQgdIVFlYg7B77UdFm', // secret - 'remember_token' => str_random(10), - ]; -}); +/** + * @extends \Illuminate\Database\Eloquent\Factories\Factory<\App\Models\User> + */ +class UserFactory extends Factory +{ + /** + * Define the model's default state. + * + * @return array + */ + public function definition() + { + // Create Dependencies - should be loaded by seeding. + $co = Country::findOrFail(61); + $lo = Language::findOrFail(1); + + return [ + 'id' => $this->faker->numberBetween(2048,65535), + // 'created_at' + // 'updated_at' + //* 'site_id', // Needs to be passed in + 'email' => $this->faker->unique()->safeEmail, + 'password' => '$2y$10$TKh8H1.PfQx37YgCzwiKb.KjNyWgaHb9cbcoQgdIVFlYg7B77UdFm', // secret + 'remember_token' => Str::random(10), + 'active' => 1, + // 'title' + 'firstname' => $this->faker->name, + 'lastname' => $this->faker->name, + 'country_id' => $co->id, + // 'address1' + // 'address2' + // 'city' + // 'state' + // 'postcode' + 'emailable' => true, + // 'parent_id'' + 'language_id' => $lo->id, + ]; + } +} diff --git a/database/migrations/2022_04_20_183016_updates_to_accounts.php b/database/migrations/2022_04_20_183016_updates_to_accounts.php new file mode 100644 index 0000000..a3faecc --- /dev/null +++ b/database/migrations/2022_04_20_183016_updates_to_accounts.php @@ -0,0 +1,48 @@ +datetime('created_at')->nullable()->after('id'); + $table->datetime('updated_at')->nullable()->after('created_at'); + $table->date('expire_at')->nullable()->after('updated_at'); + }); + + // Convert our dates + foreach (\App\Models\Account::withoutGlobalScope(\App\Models\Scopes\SiteScope::class)->cursor() as $o) { + if ($o->date_orig) + $o->created_at = \Carbon\Carbon::createFromTimestamp($o->date_orig); + if ($o->date_last) + $o->updated_at = \Carbon\Carbon::createFromTimestamp($o->date_last); + if ($o->date_expire) + $o->expire_at = \Carbon\Carbon::createFromTimestamp($o->date_expire); + + $o->save(); + } + + Schema::table('accounts', function (Blueprint $table) { + $table->dropColumn(['date_orig','date_last','date_expire']); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + abort(500,'Cant go back'); + } +}; diff --git a/resources/views/theme/backend/adminlte/common/account/widget/summary.blade.php b/resources/views/theme/backend/adminlte/common/account/widget/summary.blade.php index b0b8304..6dd44f7 100644 --- a/resources/views/theme/backend/adminlte/common/account/widget/summary.blade.php +++ b/resources/views/theme/backend/adminlte/common/account/widget/summary.blade.php @@ -1,17 +1,17 @@ @if ($o->accounts->count() > 1) -
+
Linked Accounts - {{ number_format($o->accounts->count()) }} + {{ number_format($o->my_accounts->count()) }}
@endif -
+
@@ -23,7 +23,7 @@
-
+
@@ -34,7 +34,7 @@
-
+
diff --git a/resources/views/theme/backend/adminlte/r/account/widgets/list.blade.php b/resources/views/theme/backend/adminlte/r/account/widgets/list.blade.php index 7661405..5ab33a1 100644 --- a/resources/views/theme/backend/adminlte/r/account/widgets/list.blade.php +++ b/resources/views/theme/backend/adminlte/r/account/widgets/list.blade.php @@ -4,21 +4,29 @@
- @if ($user->all_accounts()->count()) - + @if ($user->accounts->count()) +
- - + + + @foreach ($user->accounts as $ao) + + + + + + @endforeach + - - + +
Profile NameActiveServicesServices
{{ $ao->name }}{{ $ao->services->where('active',TRUE)->count() }} /{{ $ao->services->count() }}
Count {{ $user->all_accounts()->count() }} Count {{ $user->accounts->count() }} 
@@ -35,21 +43,12 @@ +@append \ No newline at end of file diff --git a/resources/views/theme/backend/adminlte/u/invoice/widgets/next.blade.php b/resources/views/theme/backend/adminlte/u/invoice/widgets/next.blade.php index 3e6684b..8b4d0e9 100644 --- a/resources/views/theme/backend/adminlte/u/invoice/widgets/next.blade.php +++ b/resources/views/theme/backend/adminlte/u/invoice/widgets/next.blade.php @@ -10,11 +10,11 @@ ${{ number_format($oo->sum('total'),2) }} - @foreach ($oo->groupBy('service_id') as $ooo) - - {{ $ooo->first()->service->sid }} - {{ $ooo->first()->service->sname }}: {{ $ooo->first()->service->sdesc }} - + @foreach ($oo->groupBy('service_id') as $ooo) + + {{ $ooo->first()->service->sid }} + {{ $ooo->first()->service->sname }}: {{ $ooo->first()->service->sdesc }} + @foreach ($ooo as $io) diff --git a/resources/views/theme/backend/adminlte/u/service/widgets/active.blade.php b/resources/views/theme/backend/adminlte/u/service/widgets/active.blade.php index edf4453..a58c7e9 100644 --- a/resources/views/theme/backend/adminlte/u/service/widgets/active.blade.php +++ b/resources/views/theme/backend/adminlte/u/service/widgets/active.blade.php @@ -59,7 +59,7 @@