Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AuditableTransitionException Error on Morphmap on transitionTo() #357

Closed
watercayman opened this issue Dec 9, 2017 · 5 comments
Closed
Assignees
Labels
bug Issue, error or unexpected behavior V5

Comments

@watercayman
Copy link

Q A
Bug? yes
New Feature? no
Framework Laravel
Framework version 5.5.22
Package version 5.0.1
PHP version 7.1

Actual Behaviour

Audits work perfectly, storing correct basic data for the change (no many-to-many though).

Failure to find the morph (auditable_type) class when using transitionTo() method. Fails for all classes I am using the trait with, displaying the following error:
Expected Auditable type App\XYZ, got XYZ instead
I have all of my morphable classes (which are all I am using for tracking audits) attached correctly within AppServiceProvider in a morphmap like so:
\Illuminate\Database\Eloquent\Relations\Relation::morphMap([ 'Employee' => \App\Employee::class, ];
These work for all morph_type actions on all other methods throughout the app, as expected.

Expected Behaviour

transitionTo() should read the morphMap like standard Laravel polymorphic relations and thus not fail the test on line 467 of Auditable class

Steps to Reproduce

  • Create standard polymorphic class
  • Create morphmap within AppServiceProvider so Laravel / Auditable knows that a morphable field in the database like 'Team' maps to '\App\Team' -- consistent with Laravel functionality
  • Make successful change to class with Audit trait
  • Attempt to transition back (assuming Article is a class in the morphMap and is set as 'Article' in the DB):
    $article = Article::find(123);
    $audit = $article->audits()->first();
    $article->transitionTo($audit);

Possible Solutions

The functionality is flawless doing this by hand using the final part of the transitionTo() method within Auditable:

foreach ($modified as $attribute => $value) {
            if (array_key_exists($key, $value)) {
                $this->setAttribute($attribute, $value[$key]);
            }
        }

I'd like to use the new internal code as it has excellent error and checking functionality. I'm not sure where to fix this to align with Laravel morph classes.

@quetzyg
Copy link
Contributor

quetzyg commented Dec 9, 2017

Hi @watercayman,

Thanks for reporting. I totally forgot that morphMap was a thing. I'll fix this ASAP.

@quetzyg quetzyg added bug Issue, error or unexpected behavior V5 labels Dec 9, 2017
@quetzyg quetzyg self-assigned this Dec 9, 2017
@watercayman
Copy link
Author

Awesome! Thank you, and thank you for the package.

@quetzyg quetzyg mentioned this issue Dec 9, 2017
@quetzyg quetzyg closed this as completed in f10a16d Dec 9, 2017
@watercayman
Copy link
Author

Confirm successful revert using transitionTo() on morphMapped objects with V5.0.2. Outstanding! Thank you again.

@dhcmega
Copy link

dhcmega commented Dec 11, 2017

@watercayman hi, can you tell me how did you configure the conf file to use polymorphic.
I have 2 tables for users, admins and remote users. Tables are very different.
My conf is like this:

    'user' => [
        'primary_key' => 'id',
        'foreign_key' => 'user_id',
//        'model'       => App\User::class,
        'model'       => App\Models\Administrator::class,
        'resolver'    => function () {
            return Auth::check() ? Auth::user()->getAuthIdentifier() : null;
        },

I do have User (commented out) and Administrator
Thanks!

EDIT: also audits is registering remember token updates, which I don't want to log. Thanks.

@watercayman
Copy link
Author

@dhcmega, sorry I was using poly auditable models, not multiple user models, so I'm afraid I can't help you :(. I'm not sure this is the right place for this question - perhaps stackoverflow would be better suited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue, error or unexpected behavior V5
Projects
None yet
Development

No branches or pull requests

3 participants