fix: fix display relationship without a partner (#2598)

CHANGELOG

@@ -10,6 +10,7 @@ Enhancements:
 
 Fixes:
 
+* Fix display relationship without a ofContact property
 * Fix register request validate
 * Fix relationship create
 

app/Http/Controllers/Contacts/RelationshipsController.php

@@ -105,12 +105,14 @@ public function store(Request $request, Contact $contact)
      * Show the form for editing the specified resource.
      *
      * @param Contact $contact
-     * @param Contact $otherContact  significant other contact
+     * @param Relationship $relationship
      *
      * @return \Illuminate\View\View
      */
-    public function edit(Contact $contact, Contact $otherContact)
+    public function edit(Contact $contact, Relationship $relationship)
     {
+        $otherContact = $relationship->ofContact;
+
         $now = now();
         $age = (string) (! is_null($otherContact->birthdate) ? $otherContact->birthdate->getAge() : 0);
         $birthdate = ! is_null($otherContact->birthdate) ? $otherContact->birthdate->date->toDateString() : $now->toDateString();
@@ -119,9 +121,6 @@ public function edit(Contact $contact, Contact $otherContact)
 
         $hasBirthdayReminder = is_null($otherContact->birthday_reminder_id) ? 0 : 1;
 
-        // Get the nature of the current relationship
-        $type = $contact->getRelationshipNatureWith($otherContact);
-
         return view('people.relationship.edit')
             ->withContact($contact)
             ->withPartner($otherContact)
@@ -130,27 +129,29 @@ public function edit(Contact $contact, Contact $otherContact)
             ->withDays(DateHelper::getListOfDays())
             ->withMonths(DateHelper::getListOfMonths())
             ->withBirthdate($birthdate)
-            ->withType($type->relationship_type_id)
+            ->withRelationshipId($relationship->id)
+            ->withType($relationship->relationship_type_id)
             ->withBirthdayState($otherContact->getBirthdayState())
             ->withDay($day)
             ->withMonth($month)
             ->withAge($age)
             ->withGenders(GendersHelper::getGendersInput())
-            ->withHasBirthdayReminder($hasBirthdayReminder)
-            ->withRelationshipId($type->id);
+            ->withHasBirthdayReminder($hasBirthdayReminder);
     }
 
     /**
      * Update the specified resource in storage.
      *
      * @param Request $request
      * @param Contact $contact
-     * @param Contact $otherContact  significant other contact
+     * @param Relationship $relationship
      *
      * @return \Illuminate\Http\RedirectResponse
      */
-    public function update(Request $request, Contact $contact, Contact $otherContact)
+    public function update(Request $request, Contact $contact, Relationship $relationship)
     {
+        $otherContact = $relationship->ofContact;
+
         if ($otherContact->is_partial) {
             $datas = $this->validateAndGetDatas($request);
 
@@ -168,7 +169,7 @@ public function update(Request $request, Contact $contact, Contact $otherContact
         // update the relationship
         app(UpdateRelationship::class)->execute([
             'account_id' => auth()->user()->account_id,
-            'relationship_id' => $request->get('relationship_id'),
+            'relationship_id' => $relationship->id,
             'relationship_type_id' => $request->get('relationship_type_id'),
         ]);
 
@@ -230,22 +231,20 @@ private function validateAndGetDatas(Request $request)
      * Remove the specified resource from storage.
      *
      * @param Contact $contact
-     * @param Contact $otherContact
+     * @param Relationship $relationship
      *
      * @return \Illuminate\Http\RedirectResponse
      */
-    public function destroy(Contact $contact, Contact $otherContact)
+    public function destroy(Contact $contact, Relationship $relationship)
     {
         if ($contact->account_id != auth()->user()->account_id) {
             return redirect()->route('people.index');
         }
 
-        if ($otherContact->account_id != auth()->user()->account_id) {
+        if ($relationship->account_id != auth()->user()->account_id) {
             return redirect()->route('people.index');
         }
 
-        $relationship = $contact->getRelationshipNatureWith($otherContact);
-
         app(DestroyRelationship::class)->execute([
             'account_id' => auth()->user()->account_id,
             'relationship_id' => $relationship->id,

app/Models/Contact/Contact.php

@@ -1300,22 +1300,6 @@ public function setSpecialDateFromAge($occasion, int $age)
         return $specialDate;
     }
 
-    /**
-     * Get the Relationship object representing the relation between two contacts.
-     *
-     * @param  Contact $otherContact
-     * @return Relationship|null
-     */
-    public function getRelationshipNatureWith(self $otherContact)
-    {
-        return Relationship::where([
-            'account_id' => $this->account_id,
-            'contact_is' => $this->id,
-            'of_contact' => $otherContact->id,
-        ])
-            ->first();
-    }
-
     /**
      * Delete all related objects.
      *

app/Models/Relationship/Relationship.php

@@ -4,7 +4,7 @@
 
 use App\Models\Account\Account;
 use App\Models\Contact\Contact;
-use Illuminate\Database\Eloquent\Model;
+use App\Models\ModelBinding as Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
 
 /**

resources/views/people/relationship/_relationship.blade.php

@@ -1,4 +1,7 @@
 @foreach ($relationships as $relationship)
+  @if (! $relationship->ofContact)
+    @continue
+  @endif
   <div class="sidebar-box-paragraph">
     <span class="silver fw3 ba br2 ph1 {{ htmldir() == 'ltr' ? '' : 'fr' }}">{{ $relationship->relationshipType->getLocalizedName(null, false, $relationship->ofContact->gender ? $relationship->ofContact->gender->type : null) }}</span>
 
@@ -17,14 +20,14 @@
     @endif
 
     {{-- ACTIONS: EDIT/DELETE --}}
-    <a href="{{ route('people.relationships.edit', [$contact, $relationship->ofContact]) }}" class="action-link {{ $contact->hashID() }}-edit-relationship">
+    <a href="{{ route('people.relationships.edit', [$contact, $relationship]) }}" class="action-link {{ $contact->hashID() }}-edit-relationship">
         {{ trans('app.edit') }}
       </a>
       <a href="#" onclick="if (confirm('{{ trans('people.relationship_unlink_confirmation') }}')) { $(this).closest('.sidebar-box-paragraph').find('.entry-delete-form').submit(); } return false;" class="action-link">
       {{ trans('app.delete') }}
     </a>
 
-    <form method="POST" action="{{ route('people.relationships.destroy', [$contact, $relationship->ofContact]) }}" class="entry-delete-form hidden">
+    <form method="POST" action="{{ route('people.relationships.destroy', [$contact, $relationship]) }}" class="entry-delete-form hidden">
       {{ method_field('DELETE') }}
       {{ csrf_field() }}
     </form>

resources/views/people/relationship/edit.blade.php

@@ -6,7 +6,7 @@
 
   {{-- Breadcrumb --}}
   <div class="mt4 mw7 center mb3">
-    <p><a href="{{ route('people.show', $contact) }}">< {{ $contact->name }}</a></p>
+    <p><a href="{{ route('people.show', $contact) }}">&lt; {{ $contact->name }}</a></p>
     <div class="mt4 mw7 center mb3">
       <h3 class="f3 fw5">{{ trans('people.relationship_form_edit') }}</h3>
     </div>
@@ -22,10 +22,9 @@
 
     @include('partials.errors')
 
-    <form action="{{ route('people.relationships.update', [$contact, $partner]) }}" method="POST">
-      {{ method_field('PUT') }}
-      {{ csrf_field() }}
-      <input type="hidden" name="relationship_id" value="{{ $relationshipId }}">
+    <form action="{{ route('people.relationships.update', [$contact, $relationshipId]) }}" method="POST">
+      @method('PUT') }}
+      @csrf
       <input type="hidden" name="type" value="{{ $type }}">
 
       @if ($partner->is_partial)

routes/web.php

@@ -120,12 +120,9 @@
         });
 
         // Relationships
-        Route::resource('people/{contact}/relationships', 'Contacts\\RelationshipsController')->only(['create', 'store']);
-        Route::name('relationships.')->group(function () {
-            Route::get('/people/{contact}/relationships/{otherContact}/edit', 'Contacts\\RelationshipsController@edit')->name('edit');
-            Route::put('/people/{contact}/relationships/{otherContact}', 'Contacts\\RelationshipsController@update')->name('update');
-            Route::delete('/people/{contact}/relationships/{otherContact}', 'Contacts\\RelationshipsController@destroy')->name('destroy');
-        });
+        Route::resource('people/{contact}/relationships', 'Contacts\\RelationshipsController')->only([
+            'create', 'store', 'edit', 'update', 'destroy',
+        ]);
 
         // Pets
         Route::resource('people/{contact}/pets', 'Contacts\\PetsController')->only([

tests/Feature/RelationshipTest.php

@@ -14,6 +14,20 @@ class RelationshipTest extends FeatureTestCase
 {
     use DatabaseTransactions, WithFaker;
 
+    public function test_create_a_relationship()
+    {
+        $user = $this->signIn();
+        $contact = factory(Contact::class)->create([
+            'account_id' => $user->account_id,
+        ]);
+
+        $response = $this->get('/people/'.$contact->hashID().'/relationships/create');
+
+        $response->assertStatus(200);
+
+        $response->assertSee('This person is...');
+    }
+
     public function test_user_can_add_a_relationship()
     {
         $user = $this->signIn();
@@ -35,6 +49,8 @@ public function test_user_can_add_a_relationship()
 
         $response = $this->post('/people/'.$contact->hashID().'/relationships', $params);
 
+        $response->assertStatus(302);
+
         $this->assertDatabaseHas('relationships', [
             'account_id' => $user->account_id,
             'contact_is' => $contact->id,
@@ -68,6 +84,8 @@ public function test_user_can_add_a_relationship_new_user_birthdate_unknown()
 
         $response = $this->post('/people/'.$contact->hashID().'/relationships', $params);
 
+        $response->assertStatus(302);
+
         $this->assertDatabaseHas('contacts', [
             'account_id' => $user->account_id,
             'first_name' => 'Arnold',
@@ -107,6 +125,8 @@ public function test_user_can_add_a_relationship_new_user_partial()
 
         $response = $this->post('/people/'.$contact->hashID().'/relationships', $params);
 
+        $response->assertStatus(302);
+
         $this->assertDatabaseHas('contacts', [
             'account_id' => $user->account_id,
             'first_name' => 'Arnold',
@@ -147,6 +167,8 @@ public function test_user_can_add_a_relationship_new_user_birthdate_known()
 
         $response = $this->post('/people/'.$contact->hashID().'/relationships', $params);
 
+        $response->assertStatus(302);
+
         $this->assertDatabaseHas('contacts', [
             'account_id' => $user->account_id,
             'first_name' => 'Arnold',
@@ -167,6 +189,30 @@ public function test_user_can_add_a_relationship_new_user_birthdate_known()
         ]);
     }
 
+    public function test_edit_a_relationship()
+    {
+        $user = $this->signIn();
+        $contact = factory(Contact::class)->create([
+            'account_id' => $user->account_id,
+        ]);
+        $partner = factory(Contact::class)->create([
+            'account_id' => $user->account_id,
+            'first_name' => 'Homer',
+            'last_name' => 'Simpson',
+        ]);
+        $relationship = factory(Relationship::class)->create([
+            'account_id' => $user->account_id,
+            'contact_is' => $contact->id,
+            'of_contact' => $partner->id,
+        ]);
+
+        $response = $this->get('/people/'.$contact->hashID().'/relationships/'.$relationship->id.'/edit');
+
+        $response->assertStatus(200);
+
+        $response->assertSee('Homer Simpson is...');
+    }
+
     public function test_user_can_update_a_relationship()
     {
         $user = $this->signIn();
@@ -190,7 +236,9 @@ public function test_user_can_update_a_relationship()
             'relationship_type_id' => $relationshipType->id,
         ];
 
-        $response = $this->put('/people/'.$contact->hashID().'/relationships/'.$partner->hashID(), $params);
+        $response = $this->put('/people/'.$contact->hashID().'/relationships/'.$relationship->id, $params);
+
+        $response->assertStatus(302);
 
         $this->assertDatabaseHas('relationships', [
             'id' => $relationship->id,
@@ -230,7 +278,9 @@ public function test_user_can_update_a_relationship_partial_user()
             'birthdayDate' => '1947-07-30',
         ];
 
-        $response = $this->put('/people/'.$contact->hashID().'/relationships/'.$partner->hashID(), $params);
+        $response = $this->put('/people/'.$contact->hashID().'/relationships/'.$relationship->id, $params);
+
+        $response->assertStatus(302);
 
         $this->assertDatabaseHas('contacts', [
             'account_id' => $user->account_id,
@@ -252,4 +302,31 @@ public function test_user_can_update_a_relationship_partial_user()
             'relationship_type_id' => $relationshipType->id,
         ]);
     }
+
+    public function test_user_can_destroy_a_relationship()
+    {
+        $user = $this->signIn();
+        $contact = factory(Contact::class)->create([
+            'account_id' => $user->account_id,
+        ]);
+        $partner = factory(Contact::class)->create([
+            'account_id' => $user->account_id,
+        ]);
+        $relationship = factory(Relationship::class)->create([
+            'account_id' => $user->account_id,
+            'contact_is' => $contact->id,
+            'of_contact' => $partner->id,
+        ]);
+
+        $response = $this->delete('/people/'.$contact->hashID().'/relationships/'.$relationship->id);
+
+        $response->assertStatus(302);
+
+        $this->assertDatabaseMissing('relationships', [
+            'id' => $relationship->id,
+            'account_id' => $user->account_id,
+            'contact_is' => $contact->id,
+            'of_contact' => $partner->id,
+        ]);
+    }
 }

tests/Unit/Models/ContactTest.php

@@ -947,32 +947,6 @@ public function test_set_name_returns_true()
         );
     }
 
-    public function test_it_gets_the_relationship_between_two_contacts()
-    {
-        $account = factory(Account::class)->create([]);
-        $contact = factory(Contact::class)->create(['account_id' => $account->id]);
-        $partner = factory(Contact::class)->create(['account_id' => $account->id]);
-        $relationshipType = factory(RelationshipType::class)->create([
-            'account_id' => $account->id,
-            'name' => 'godfather',
-        ]);
-        $relationship = factory(Relationship::class)->create([
-            'account_id' => $account->id,
-            'contact_is' => $contact->id,
-            'of_contact' => $partner->id,
-            'relationship_type_id' => $relationshipType->id,
-        ]);
-
-        $foundRelationship = $contact->getRelationshipNatureWith($partner);
-
-        $this->assertInstanceOf(Relationship::class, $foundRelationship);
-
-        $this->assertEquals(
-            $relationship->id,
-            $foundRelationship->id
-        );
-    }
-
     public function test_it_gets_related_relationships_of_a_certain_relationshiptype_group_name()
     {
         $account = factory(Account::class)->create([]);