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

ErrorException Undefined index: user #282

Closed
Grummfy opened this issue Aug 3, 2017 · 11 comments
Closed

ErrorException Undefined index: user #282

Grummfy opened this issue Aug 3, 2017 · 11 comments

Comments

@Grummfy
Copy link

Grummfy commented Aug 3, 2017

Q A
Bug? yes
New Feature? no
Framework Laravel (
Framework version 5.4.30
Package version 4.1.1
PHP version 7.1.6

Actual Behaviour

An error of undefined index appears

Expected Behaviour

No error ;)

Steps to Reproduce

dump(MyModel::find($id)->audits()->first()->getMetadata());

ErrorException Undefined index: user

no error with:

dump(MyModel::find($id)->audits()->with('user')->first()->getMetadata());

Possible Solutions

inside \OwenIt\Auditing\Audit::resolveData
replace

if ($user = $this->getRelation('user')) {

by

if ($user = $this->user) {
@quetzyg
Copy link
Contributor

quetzyg commented Aug 3, 2017

Show me the User class, the MyModel class and the audit.php configuration file you have.

@Grummfy
Copy link
Author

Grummfy commented Aug 3, 2017

audit.php

<?php

return [

    /*
    |--------------------------------------------------------------------------
    | Audit implementation
    |--------------------------------------------------------------------------
    |
    | Define which Audit model implementation should be used.
    |
    */

    'implementation' => OwenIt\Auditing\Models\Audit::class,

    /*
    |--------------------------------------------------------------------------
    | User Keys, Model & Resolver
    |--------------------------------------------------------------------------
    |
    | Define the User primary and foreign keys, Eloquent model and ID resolver
    | class.
    |
    */

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

    /*
    |--------------------------------------------------------------------------
    | Default Driver
    |--------------------------------------------------------------------------
    |
    | The default audit driver used to keep track of changes.
    |
    */

    'default' => 'database',

    /*
    |--------------------------------------------------------------------------
    | Audit Drivers
    |--------------------------------------------------------------------------
    |
    | Available audit drivers and respective configurations.
    |
    */
    'drivers' => [
        'database' => [
            'table'      => 'audits',
            'connection' => env('DB_CONNECTION', 'mysql'),
        ],
    ],

    /*
    |--------------------------------------------------------------------------
    | Audit Console?
    |--------------------------------------------------------------------------
    |
    | Whether we should audit console events (eg. php artisan db:seed).
    |
    */

    'console' => false,
];

MyModel is just one model around several others, it's similar to this one

<?php
namespace App\Models\Database;

use App\Models\Model as Eloquent;
use OwenIt\Auditing\Auditable;
use OwenIt\Auditing\Contracts\Auditable as AuditableContract;

class MyModel extends Eloquent implements AuditableContract
{
	use Auditable;
}

User

<?php

namespace App\Models\Database;

use App\Models\Model as Eloquent;
use OwenIt\Auditing\Auditable;
use OwenIt\Auditing\Contracts\Auditable as AuditableContract;

class User extends Eloquent implements AuditableContract
{
	use Auditable;
}

I try to use model as basic as possible and I got the same behaviour.

Note : all my dev are made with error_reporting = E_ALL

@quetzyg
Copy link
Contributor

quetzyg commented Aug 3, 2017

Sorry, I can't reproduce the issue. You say that if you replace

if ($user = $this->getRelation('user')) {

with

if ($user = $this->user) {

it works?

If you dd($this->user) do you get a null or a User instance?

@Grummfy
Copy link
Author

Grummfy commented Aug 3, 2017

I got a user or null following if there is a connected user.
But the fact that it's null or not, change nothing on the error.

@quetzyg
Copy link
Contributor

quetzyg commented Aug 3, 2017

I got a user or null following if there is a connected user.

Can you rephrase that?

But the fact that it's null or not, change nothing on the error.

If you have null, it might explain why you get that issue.

@Grummfy
Copy link
Author

Grummfy commented Aug 3, 2017

I try to reproduce the case with a clean fresh laravel installation with just this package (so with the default user model).

Foo.php

<?php

namespace App;

use Illuminate\Database\Eloquent\Model;

use OwenIt\Auditing\Auditable;
use OwenIt\Auditing\Contracts\Auditable as AuditableContract;

class Foo extends Model  implements AuditableContract
{

	use Auditable;
    //
public $timestamps = false;
	protected $fillable = [
	'name',
];
}

Simple table, with a simple attribute

 \App\Foo::create(['name' => 'abc-' . uniqid()]);
	foreach(\App\Foo::all() as $foo)
	{
		dump($foo->audits()->first()->getMetadata());
	}

exactly the same error.

edit:
in the case of the ->user I got a null in this case (i have made no configuration related to authentication.

But in my project example, in the case where ->user is not null I still have the same error.

@quetzyg
Copy link
Contributor

quetzyg commented Aug 3, 2017

Try: dump($foo->audits()->with('user')->first()->getMetadata());

@Grummfy
Copy link
Author

Grummfy commented Aug 3, 2017

With ->with('user') it's working perfectly. whatever the value of ->user is.
But, I'm not sure that we require to do that each time we want to access to metadata.

@quetzyg
Copy link
Contributor

quetzyg commented Aug 3, 2017

Yeah, I'll have to change that again. We shouldn't be forced to pass ->with('user').

@quetzyg
Copy link
Contributor

quetzyg commented Aug 3, 2017

Fixed in the latest version.

@quetzyg quetzyg closed this as completed Aug 3, 2017
@Grummfy
Copy link
Author

Grummfy commented Aug 3, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants