ekmungai / eloquent-ifrs

Eloquent Double Entry Accounting with a focus on IFRS Compliant Reporting
MIT License
334 stars 68 forks source link

Suggestion: Change how entity_id is fetched for Logged in users #29

Closed hicka closed 3 years ago

hicka commented 3 years ago

Hi Edward. Nice package πŸ‘πŸ½!

In this package, the entity is tied to the logged in User. There might be cases where the entity is say, A Business which the user has access to. Changing the Model is doable with the said config file where we change the default user_model

'user_model' => App\Business::class,

In this case, by default this package runs migrations on the said Business and it adds the entity_id to this Business Model.

Now what we can do is add the entity method for logged in user in User.php (not Business.php) This enables us to fetch the entity id for the logged in User

    public function entity()
    {
        //return $this->belongsTo(Entity::class);
        return $this->business->entity;  // or some other way the entity_id is fetched
    }

Wouldn't it be nice if we change how the entity_id is retrieved/saved for users?

The real problem is how the package retrieves the entity_id even if we provide a custom user class, in our case the entity_id is saved in Business model but this EntityScope still looks for the entity_id in the users table. What we can do is modify the EntityScope like so, retrieving the logged in users entity which is returned from the Business Model or any other custom way that the user defined it.

class EntityScope implements Scope
{
    public function apply(Builder $builder, Model $model)
    {
        $model = null;
        $user = Auth::user();
        if (!is_null($user)) {
              //$builder->where('entity_id', Auth::user()->entity_id);

              $builder->where('entity_id', Auth::user()->entity->id);
        }
    }
}

now the current workflow would be something like this, this is just an example.


// our user
$user = \App\Users::find(1);;
    Auth::login($user);

// our custom user_class / the model that holds entity_id
$business = Business::where('id','=',1)->first();

// new entity creation
$entity = Entity::create([
     "name" => "MALA",
]);

// save the entity_id to the custom model
$business->entity_id = $entity->id;
$business->save();

Now the entity is retrieved like so via the authenticated user, as we have added an entity() method to Users which returns the entity relationship on the Business model.


Auth::user()->entity->id; // entity is returned from Business

now we can continue using the entity for all other methods like so


    $bankAccount = Account::create([
        'name' => "Sales Account",
        'account_type' => Account::BANK,
    ]);

I really believe a package like this shouldn't be tied to the User class like this, but be more flexible instead. There might be some breaking changes in doing this but i am willing to refactor the code in a way that the current users of this package wont be affected.

What do you think?

hicka commented 3 years ago

Hi, I have changed the necessary files in order to make this work without breaking changes.

Seems like these changes were made in this commit 9b2f85629dfa8702be2dc44bd376cb69de93f3aa . But i think these needs to be changed back to allow custom models with entities, in a scenario where the Authenticated user model does not store the entity_id.

File Scopes\EntityScope.php line 32

    public function apply(Builder $builder, Model $model)
    {
        $model = null;
        $user = Auth::user();
        if (!is_null($user)) {
            //$builder->where('entity_id', Auth::user()->entity_id);
              $builder->where('entity_id', Auth::user()->entity->id);
        }
    }

File Traits\Segregating.php line 44

    public static function bootSegregating()
    {
        static::addGlobalScope(new EntityScope);

        static::creating(
            function ($model) {

                // only users can be created without requiring to be logged on
                if (!Auth::check() && !is_a($model, config('ifrs.user_model'))) {
                    throw new UnauthorizedUser();
                }

                if (Auth::check() && is_null($model->entity_id)) {
                    //$model->entity_id = Auth::user()->entity_id;
                      $model->entity_id = Auth::user()->entity->id;
                }
            }
        );
        return null;
    }

File Models\Currency.php line 99

    public function save(array $options = []): bool
    {
        if (!isset($this->entity_id) && Auth::user()->entity) {
            //$this->entity_id = Auth::user()->entity_id;
            $this->entity_id = Auth::user()->entity->id;
        }

        return parent::save($options);
    }

πŸ‘πŸΌπŸ‘πŸ‘πŸ‘ After making these changes it seems to work fine when the authenticated user doesn't have entity_id in it.


README issues. Also there are errors in the github README.md 1.0 This is where you describe the process to add a LineItem, description seems to have changed to narration instead.

  $cashSaleLineItem = LineItem::create([
        'vat_id' => $outputVat->id,
        'account_id' => $revenueAccount->id,
        'vat_account_id' => $salesVatAccount->id,
       //'description' => "Example Cash Sale Line Item", // wrong in docs
       'narration' => "Example Cash Sale Line Item",
        'quantity' => 1,
        'amount' => 100,
    ]);

2.0 This is where we create a ReportingPeriod, year seems to have changed to calendar_year instead.

  $period = ReportingPeriod::create([
        'period_count' => 1,
        //'year' => 2021,  // wrong
        'calendar_year' => 2021, 
    ]);

3.0 This is where you describe how to create a VAT

$outputVat = Vat::create([
    'name' => "Standard Output Vat",
    'code' => "O",
    'rate' => 20,
]);

it should be

//create sales vat account first
$salesVatAccount = Account::create([
    'name' => "Sales VAT Account",
    'account_type' => Account::CONTROL,
]);

//then create  vat account
 $outputVat = Vat::create([
        'name' => "Standard Output Vat",
        'code' => "S",
        'account_id' => $salesVatAccount->id, // <-- this
        'rate' => 20,
    ]);

4.0 This is where you describe how to create accounts

$salesVatAccount = Account::create([
    'name' => "Sales VAT Account",
    //'account_type' => Account::CONTROL_ACCOUNT,
    'account_type' => Account::CONTROL, // <-- changed to Account::CONTROL
]);

$purchasesVatAccount = Account::create([
    'name' => "Input VAT Account",
    //'account_type' => Account::CONTROL_ACCOUNT,
    'account_type' => Account::CONTROL, // <-- changed to Account::CONTROL
]);
ekmungai commented 3 years ago

Hi hicka,

Thanks alot for your thorough overview. As you saw on the commit, initially I had the same idea as you did but I changed it back for some reason that I cant even remember right now. I'll review your PR and merge it.

Cheers, Edward