From 99bd8e17f8ac78af45937673a38a15c72f1e0278 Mon Sep 17 00:00:00 2001
From: Alexis Saettler <alexis@saettler.org>
Date: Tue, 22 Jun 2021 18:59:31 +0200
Subject: [PATCH] fix: fix tags list filtering (#5123)

---
 app/Http/Controllers/ContactsController.php | 64 +++++++--------------
 app/Models/Contact/Tag.php                  |  2 +-
 resources/js/components/people/Tags.vue     | 21 +++----
 resources/views/people/index.blade.php      |  2 +-
 tests/Feature/ContactTest.php               | 33 ++++++++++-
 5 files changed, 64 insertions(+), 58 deletions(-)

diff --git a/app/Http/Controllers/ContactsController.php b/app/Http/Controllers/ContactsController.php
index a8c422b04be..01a894037b6 100644
--- a/app/Http/Controllers/ContactsController.php
+++ b/app/Http/Controllers/ContactsController.php
@@ -86,31 +86,22 @@ private function contacts(Request $request, bool $active)
             $nbArchived = $contacts->count();
         }
 
+        $tagsCount = Tag::contactsCount();
         $tags = null;
-        $url = '';
+        $url = null;
         $count = 1;
 
-        if ($request->input('tag1')) {
+        if ($request->input('tags')) {
+            $tagsInput = $request->input('tags');
 
-            // get contacts with selected tags
-            $tags = collect();
+            $tags = $tagsCount->filter(function ($tag) use ($tagsInput) {
+                return in_array($tag->name, $tagsInput);
+            });
 
-            while ($request->input('tag'.$count)) {
-                $tag = Tag::where([
-                    'account_id' => auth()->user()->account_id,
-                    'name_slug' => $request->input('tag'.$count),
-                ]);
-                if ($tag->count() > 0) {
-                    $tag = $tag->get();
+            $url = $tags->map(function ($tag) {
+                return 'tags[]='.urlencode($tag->name);
+            })->join('&');
 
-                    if (! $tags->contains($tag[0])) {
-                        $tags = $tags->concat($tag);
-                    }
-
-                    $url .= 'tag'.$count.'='.$tag[0]->name_slug.'&';
-                }
-                $count++;
-            }
             if ($tags->count() === 0) {
                 return redirect()->route('people.index');
             } else {
@@ -131,14 +122,14 @@ private function contacts(Request $request, bool $active)
 
         return view('people.index')
             ->withAccountHasLimitations($accountHasLimitations)
-            ->with('hidingDeceased', $showDeceased != 'true')
-            ->with('deceasedCount', $deceasedCount)
+            ->withHidingDeceased($showDeceased !== 'true')
+            ->withDeceasedCount($deceasedCount)
             ->withActive($active)
             ->withContactsCount($contactsCount)
             ->withHasArchived($nbArchived > 0)
             ->withArchivedContacts($nbArchived)
             ->withTags($tags)
-            ->withTagsCount(Tag::contactsCount())
+            ->withTagsCount($tagsCount)
             ->withUrl($url)
             ->withTagCount($count)
             ->withTagLess($request->input('no_tag') ?? false);
@@ -730,8 +721,6 @@ public function list(Request $request)
         }
 
         $tags = null;
-        $url = '';
-        $count = 1;
 
         $contacts = $user->account->contacts()->real();
 
@@ -747,30 +736,17 @@ public function list(Request $request)
             $contacts = $contacts->alive();
         }
 
-        if ($request->input('no_tag')) {
-            // get tag less contacts
-            $contacts = $contacts->tags('NONE');
-        } elseif ($request->input('tag1')) {
-            // get contacts with selected tags
-            $tags = collect();
-
-            while ($request->input('tag'.$count)) {
-                $tag = Tag::where([
-                    'account_id' => $accountId,
-                    'name_slug' => $request->input('tag'.$count),
-                ])->get();
-
-                if (! ($tags->contains($tag[0]))) {
-                    $tags = $tags->concat($tag);
-                }
-
-                $url = $url.'tag'.$count.'='.$tag[0]->name_slug.'&';
+        if ($request->input('tags')) {
+            $tags = Tag::where('account_id', $accountId)
+                    ->whereIn('name', $request->input('tags'))
+                    ->get();
 
-                $count++;
-            }
             if ($tags->count() > 0) {
                 $contacts = $contacts->tags($tags);
             }
+        } elseif ($request->input('no_tag')) {
+            // get tag less contacts
+            $contacts = $contacts->tags('NONE');
         }
 
         // get the number of contacts per page
diff --git a/app/Models/Contact/Tag.php b/app/Models/Contact/Tag.php
index ef91dfa12d9..9e5b5d6857d 100644
--- a/app/Models/Contact/Tag.php
+++ b/app/Models/Contact/Tag.php
@@ -47,7 +47,7 @@ public function contacts()
      */
     public static function contactsCount()
     {
-        return DB::table('contact_tag')->selectRaw('COUNT(tag_id) AS contact_count, name, name_slug')
+        return DB::table('contact_tag')->selectRaw('COUNT(tag_id) AS contact_count, name, tag_id AS id')
                     ->join('tags', function ($join) {
                         $join->on('tags.id', '=', 'contact_tag.tag_id')
                            ->on('tags.account_id', '=', 'contact_tag.account_id');
diff --git a/resources/js/components/people/Tags.vue b/resources/js/components/people/Tags.vue
index 17a2c6d09a4..30f53411a8b 100644
--- a/resources/js/components/people/Tags.vue
+++ b/resources/js/components/people/Tags.vue
@@ -8,6 +8,11 @@
     background-color: #4AAE9B;
     color: white;
   }
+
+.tag-link,
+.tag-link:hover{
+    text-decoration: none;
+}
 </style>
 
 <template>
@@ -15,14 +20,14 @@
     <!-- list of existing tags -->
     <ul>
       <li v-for="tag in contactTags" :key="tag.id" class="di mr2">
-        <span class="bg-white ph2 pb1 pt0 dib br3 b--light-gray ba mb2">
-          <span v-show="!editMode" class="pointer" @click="navigateTo(tag)">
-            {{ tag.name }}
-          </span>
-          <span v-show="editMode">
+        <a v-if="!editMode" :href="`people?tags[]=${encodeURIComponent(tag.name)}`" class="tag-link bg-white ph2 pb1 pt0 dib br3 b--light-gray ba mb2">
+          {{ tag.name }}
+        </a>
+        <span v-else class="bg-white ph2 pb1 pt0 dib br3 b--light-gray ba mb2">
+          <span>
             {{ tag.name }}
           </span>
-          <span v-show="editMode" class="pointer" @click="removeTag(tag)">
+          <span class="pointer" @click="removeTag(tag)">
             ×
           </span>
         </span>
@@ -209,10 +214,6 @@ export default {
         });
     },
 
-    navigateTo(tag) {
-      window.location.href = 'people?tag1=' + tag.name_slug;
-    },
-
     handleClickOutside(evt) {
       if (!this.$el.contains(evt.target)) {
         this.isOpen = false;
diff --git a/resources/views/people/index.blade.php b/resources/views/people/index.blade.php
index c339b719a7e..d388a329a09 100644
--- a/resources/views/people/index.blade.php
+++ b/resources/views/people/index.blade.php
@@ -135,7 +135,7 @@
               @foreach ($tagsCount as $tag)
                 @if ($tag->contact_count > 0)
                 <li>
-                    <span class="pretty-tag"><a href="{{ route('people.index') }}?{{$url}}tag{{$tagCount}}={{ $tag->name_slug }}">{{ $tag->name }}</a></span>
+                    <span class="pretty-tag"><a href="{{ route('people.index') }}?{{$url}}tags[]={{ urlencode($tag->name) }}">{{ $tag->name }}</a></span>
                     <span class="number-contacts-per-tag">{{ trans_choice('people.people_list_contacts_per_tags', $tag->contact_count, ['count' => $tag->contact_count]) }}</span>
                 </li>
                 @endif
diff --git a/tests/Feature/ContactTest.php b/tests/Feature/ContactTest.php
index e800f667e71..f28568ade92 100644
--- a/tests/Feature/ContactTest.php
+++ b/tests/Feature/ContactTest.php
@@ -193,7 +193,7 @@ public function test_user_can_list_one_contact_firstname()
         ]);
     }
 
-    public function test_user_can_list_contacts_with_tag()
+    public function test_user_can_list_contacts_with_tags()
     {
         $user = $this->signIn();
 
@@ -206,6 +206,7 @@ public function test_user_can_list_contacts_with_tag()
 
         $tag = factory(Tag::class)->create([
             'account_id' => $user->account_id,
+            'name' => 'c++',
         ]);
         $contact->tags()->sync([
             $tag->id => [
@@ -213,13 +214,41 @@ public function test_user_can_list_contacts_with_tag()
             ],
         ]);
 
-        $response = $this->get('/people/list?tag1='.$tag->name_slug);
+        $response = $this->get('/people/list?tags[]='.urlencode($tag->name));
 
         $response->assertSuccessful();
         $response->assertJsonFragment([
             'id' => $contact->id,
             'complete_name' => $contact->first_name.' '.$contact->last_name,
         ]);
+        $response->assertJsonCount(1, 'contacts');
+    }
+
+    public function test_user_can_show_contacts_with_tags()
+    {
+        $user = $this->signIn();
+
+        factory(Contact::class, 10)->state('named')->create([
+            'account_id' => $user->account_id,
+        ]);
+        $contact = Contact::where('account_id', $user->account_id)
+                            ->inRandomOrder()
+                            ->first();
+
+        $tag = factory(Tag::class)->create([
+            'account_id' => $user->account_id,
+            'name' => 'c++',
+        ]);
+        $contact->tags()->sync([
+            $tag->id => [
+                'account_id' => $user->account_id,
+            ],
+        ]);
+
+        $response = $this->get('/people?tags[]='.urlencode($tag->name));
+
+        $response->assertSuccessful();
+        $response->assertSee('1 contact');
     }
 
     public function test_user_can_see_contacts()