Be part of JetBrains PHPverse 2026 on June 9 – a free online event bringing PHP devs worldwide together.

outofcontrol's avatar

How to improve testing and code review

Pretty certain I am missing something obvious in my code as I am still learning testing with Laravel:

The controller store method:

   public function store(Request $request)
    {
        $products = $request->products; // Products model pivot table

        $request->merge(['is_active' => $request->is_active == 'true']); // Checkbox, ensuring we have true or false

        $validated = $request->validate([
            'name' => 'required',
            'level_id' => 'required',
            'description' => 'required',
            'is_active' => 'nullable',
            'address' => 'nullable',
            'address2' => 'nullable',
            'country' => 'nullable',
            'state' => 'nullable',
            'city' => 'nullable',
            'postcode' => 'nullable',
            'website' => 'nullable',
            'telephone' => 'nullable',
            "contacts.*.name"  => "nullable|string",
            "contacts.*.email"  => "nullable|email",
            "contacts.*.user_id" => "nullable|integer",
            "contacts.*.contact_type_id" => "integer",
        ]);

		// Is there a better way to do these 2 steps?
        $contacts = $validated['contacts']; // Put contacts into separate array
        unset($validated['contacts']); // Remove contacts from validated data

        $org = Organizations::create($validated); // Create new entry in Organizations

		// Is there a better way to do these 2 steps?
        $org->contacts()->delete(); // Delete any contacts that exist for this org
        $org->contacts()->createMany($contacts); // Create new contacts for this org. 

        $org->products()->sync($products); // Sync (pivot table) products

        return redirect(route('organizations.index'));
    }

The above all works as expected. However, testing that contacts have been created is not currently working, as the organizations_id being added when calling createMany($contacts) is not available to the test:

   /** @test */
    public function can_create_org_with_contacts()
    {
        $this->signInAdmin();
        $this->withoutExceptionHandling();

        OrganizationLevels::factory()->create();
        ContactTypes::factory()->count(2)->create();
        $org = Organizations::factory()->make();
        $contacts = Contacts::factory()->count(4)->make()->toArray();
        $org->contacts = $contacts;

        $this->post(route('organizations.store'), $org->toArray())
            ->assertRedirect(route('organizations.index'));

        $this->assertDatabaseHas('contacts', $contacts[1]);
    }

The result is

Failed asserting that a row in the table [contacts] matches the attributes {
    "name": "Janet Lesch",
    "email": "[email protected]",
    "organizations_id": null,
    "contact_type_id": 2,
    "user_id": 1
}.

Found similar results: [
    {
        "id": "2",
        "organizations_id": "1",
        "contact_type_id": "2",
        "user_id": "1",
        "name": "Janet Lesch",
        "email": "[email protected]",
        "created_at": "2021-03-09 17:13:34",
        "updated_at": "2021-03-09 17:13:34"
    }
].

Questions:

  • Is there a cleaner, better way to accomplish what I have shown above?
  • Is there a way to test with success, the added contacts?
0 likes
5 replies
jlrdw's avatar

You could watch the videos that @jeffreyway has on testing. He demos a lot of examples.

Remember everything doesn't need testing.

outofcontrol's avatar

Thanks @jlrdw. I've watched lots of Jeffrey's videos, which is what brought me to this point. You are right that not everything needs testing, but I am getting a failed test, which I am not sure how to work around.

I'll hold out hope for some feedback a little longer :)

jlrdw's avatar

First in most cases, unit tests are not an effective way to find bugs. Let's say you unit tested first before you ever tried it in real life.

Okay you would say my code will not work so you go scrambling to fix good working code.

There are situations where a test will pass, but the real code doesn't work, okay what good does that do you?

Most of these tests are ways to identify the refactoring of code. By testing small blocks at a time, you see how the refactoring will be affected.

But the majority of test that's done in laravel has nothing to do with the real world code already written.

When I said many things don't need to be tested, an example would be if I am following laravel conventions and inserting a record into the database, following the documentation I'm not going to test something like that Taylor is already tested to make sure these things work.

Likewise things like pagination, I already know pagination works I'm not going to test it.

But just some thoughts and observations on the subject.

I would say the biggest thing about testing is to understand why you are testing.

But it boils down to one thing, the only real true test to make sure a controller method works is to do it in real life, you can test all day long, but that's not going to tell you, does my real code really work.

As I said most of this stuff is to study for refactoring.

If you have a controller that you never plan on refactoring whatsoever really it doesn't need testing if everything is working.

so

Start your code again but with a small chunk at a time and build it up until you find out what you are doing wrong, I glanced it over but I admit I haven't spotted the problem.

Also refresh your memory by looking at the documentation again and working a couple of the examples.

1 like
Daaf's avatar
Daaf
Best Answer
Level 10

The result of your test shows that the organizations_id's doesn't match (null vs 1).

You can test that your organization has 4 contacts, rather than testing that the database has the given contact.

$this->assertEquals(4, $org->fresh()->contacts()->count())

I think the following will also work, if you only want to test that the contact with given name was saved to DB, but then you are not testing that the contact was correctly inserted in DB with organization_id:

$this->assertDatabaseHas('contacts', $contacts[1]['name');

Also consider changing the 'organizations' model to singular, as would be the normal convention.

outofcontrol's avatar

Arg. was on the phone and accidentally hit best answer. Your answer is best so far... but I was hoping to wait a little longer. Thanks for the tips @daaf, the plural/singular I get confused over. Checking for number of rows will suffice for my purpose. Thanks for the suggestions, every bit helps.

Please or to participate in this conversation.