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

Tests for task #3 and task #4 needs improvements #158

Open
mykolaslisarenko opened this issue Jan 8, 2023 · 1 comment
Open

Tests for task #3 and task #4 needs improvements #158

mykolaslisarenko opened this issue Jan 8, 2023 · 1 comment

Comments

@mykolaslisarenko
Copy link

Tests for task #4 and task #5 needs improvement, because they are marked as passed with the definition of the wrong relations.

Let's take a look at the tables that are created when running a test for task #4:
users table:
image

roles table:
image

users_roles table:
image

The problem with this test data is that the user id and role id are the same and it follows the the error when wrong relation definition return the same result as correct relation definition

    //INCORRECT RELATION
    public function users()
    {
        // TASK: fix this by adding a parameter
        return $this->belongsToMany(
            User::class,
            'users_roles',
            'user_id',
            'role_id',
        );
    }

select roles., (select count() from users inner join users_roles on users.id = users_roles.role_id where roles.id = users_roles.user_id) as users_count from roles

It's forbidden to make join like "users.id = users_roles.role_id", but the test case doesn't check it

     //CORRECT RELATION
    public function users()
    {
        // TASK: fix this by adding a parameter
        return $this->belongsToMany(
            User::class,
            'users_roles',
            'role_id',
            'user_id',

        );
    }

select roles., (select count() from users inner join users_roles on users.id = users_roles.user_id where roles.id = users_roles.role_id) as users_count from roles

@mykolaslisarenko
Copy link
Author

Suggested improvements could be the following: create 2 users for one role in test

    public function test_show_roles_with_users()
    {
        $role = Role::create(['name' => 'Admin']);
        $user = User::factory()->create();

        DB::table('users_roles')->insert([
            'role_id' => $role->id,
            'user_id' => $user->id
        ]);
        $user = User::factory()->create();

        DB::table('users_roles')->insert([
            'role_id' => $role->id,
            'user_id' => $user->id
        ]);

        $response = $this->get('/roles');
        $response->assertStatus(200);
        $this->assertEquals(2, $role->users->count());
    }

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

1 participant