ekmungai / eloquent-ifrs

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

Fix : Refactor to enable use without session #71

Closed hicka closed 3 years ago

hicka commented 3 years ago

Hi @ekmungai , These are the currently required changes to be made to allow the use of code without sessions. Please take a look at Balance Model though, I have moved the part of code from constructor to the model's save method. See if it would work it as you would expect.

I have written a test for Balance sheet, had to write without the default factories because some of them required another factory and could not pass the entity id, for example, the Account Factory creates a Category by default and could not pass the entity_id to it. This test passes as expected.

With regards to multiple entities support, I do not think that this will have any effect on Reports. As you've mentioned earlier the reports depend on the Entity Scope to filter by the current entity, I did changes to the code to include the where statements to filter the model records by the entity_id where needed. I might have missed some, please take a look.

Cheers

ekmungai commented 3 years ago

Hi @hicka,

Thanks for the good work. I'll pull the branch locally and play around with it, possible add some more tests and see how it goes.

Cheers

hicka commented 3 years ago

Hi @ekmungai , Were you able to test the changes?

ekmungai commented 3 years ago

Hi @hicka,

Yes I was and there was an error, but I have an idea on how the EntityScope can be getting the entity_id from the model being queried so I'm putting asside the testing for the moment while I explore the possibility.

hicka commented 3 years ago

Hi edward,

Thats nice to hear and would make this a lot more easy then. Also, what error did you get ? Because, i got no errors while testing before your recent changes. I do get an error now after your changes to the FinancialStatements constructor where you removed the assignment of that class's $entitiy variable because then the entity is not set while running in a non session environment.

public function __construct(ReportingPeriod $period = null, Entity $entity = null)
    {
        $this->entity = $entity; // <-- this
        if (is_null($entity)) {
            $this->entity = Auth::user()->entity;
        }
ekmungai commented 3 years ago

Hi Ahmed,

You're right, that was a typo on my part. I've added it back and pushed the change.

ekmungai commented 3 years ago

Hey @hicka

Everything seeems to be working okay from my end. Do you anticipate any further changes or should I go ahead and merge the pull request?

Cheers

hicka commented 3 years ago

There's no further changes to be made that I can think of and everything works for me too. Please merge the pull request.

hicka commented 3 years ago

Hi @ekmungai , can you create a new release and update on Packagist? Cheers.

ekmungai commented 3 years ago

Hi @hicka,

I will, but I've been working on a few other changes and wanted to combine and make a single release. I should be done in a few days.

Cheers