search

cloudcake / laravel-approval

Attach an approval process to any model, approve and disapprove from any other model, for Laravel.
MIT License
140 stars 15 forks source link

Add reason field #21

Closed nagi1 closed 3 years ago

nagi1 commented 3 years ago

I love everything about this package but one thing, we as humans always need to know why our request rejected.

It would be wonderful if reason field was added to approval and disapproval models for references.

I'm open to PR this idea :)

Thanks.

stephenlake commented 3 years ago

Would appreciate a PR 👍

maggz69 commented 2 years ago

Hey @nagi1 ,

This felt like a breaking change that was not documented appropriately. If one ran the old migrations pre this PR and updated their composer.json to a version post this PR, trying to run any approval or rejection would lead to a database exception due to the reason column not being found in the approval or rejection query.

Illuminate\Database\QueryException
SQLSTATE[42S22]: Column not found: 1054 Unknown column 'reason' in 'where clause' (SQL: select * from `approvals` where (`approver_id` = 34 and `approver_type` = App\User and `modification_id` = 726 and `reason` is null) limit 1)

The readme.md should have contained this information as an upgrade note and allowed users to create a migration that added the reasoncolumn to their Approvalsmodels (or rather any of the schemas that were modified). I do not believe this affects new users as their migrations will contain the reason column.

nagi1 commented 2 years ago

Hey @maggz69, You're absolutely right! I had this problem personally :) but didn't find the time at all to fix, properly document this!