From 9ddd16b4e865c34e200f14bc7bc042bbef370a05 Mon Sep 17 00:00:00 2001
From: Vassyli <basilius.sauter@gmail.com>
Date: Tue, 9 Jan 2018 08:59:58 +0100
Subject: [PATCH] Adds suggestion plus a few additional tests

---
 src/Events/CharacterEventData.php           |  9 +---
 src/Events/ViewpointDecorationEventData.php |  3 +-
 src/Models/Viewpoint.php                    | 30 ++++---------
 tests/Models/ViewpointTest.php              | 49 +++++++++++++++++++++
 4 files changed, 61 insertions(+), 30 deletions(-)

diff --git a/src/Events/CharacterEventData.php b/src/Events/CharacterEventData.php
index 93ec7c1..79cc971 100644
--- a/src/Events/CharacterEventData.php
+++ b/src/Events/CharacterEventData.php
@@ -1,10 +1,4 @@
 <?php
-/**
- * Created by PhpStorm.
- * User: Basilius Sauter
- * Date: 04.01.2018
- * Time: 12:58
- */
 
 namespace LotGD\Core\Events;
 
@@ -27,7 +21,8 @@ class CharacterEventData extends EventContextData
     {
         $mustHaveForm = ["character"];
         $doesHaveForm = array_keys($data);
-        sort($mustHaveForm); sort($doesHaveForm);
+        sort($mustHaveForm);
+        sort($doesHaveForm);
 
         if ($doesHaveForm !== $mustHaveForm) {
             throw new ArgumentException("A new CharacterEventData event must have a character data field.");
diff --git a/src/Events/ViewpointDecorationEventData.php b/src/Events/ViewpointDecorationEventData.php
index 84b8d01..5d52667 100644
--- a/src/Events/ViewpointDecorationEventData.php
+++ b/src/Events/ViewpointDecorationEventData.php
@@ -21,7 +21,8 @@ class ViewpointDecorationEventData extends EventContextData
     {
         $mustHaveForm = ["viewpoint"];
         $doesHaveForm = array_keys($data);
-        sort($mustHaveForm); sort($doesHaveForm);
+        sort($mustHaveForm);
+        sort($doesHaveForm);
 
         if ($doesHaveForm !== $mustHaveForm) {
             throw new ArgumentException("A new ViewpointDecoration event must have a viewpoint..");
diff --git a/src/Models/Viewpoint.php b/src/Models/Viewpoint.php
index 7eb911c..239c024 100644
--- a/src/Models/Viewpoint.php
+++ b/src/Models/Viewpoint.php
@@ -195,8 +195,8 @@ class Viewpoint implements CreateableInterface
      */
     public function addActionGroup(ActionGroup $group, ?string $after = null): void
     {
-        $groupid = $group->getId();
-        if ($this->findActionGroupById($groupid) == true) {
+        $groupId = $group->getId();
+        if ($this->findActionGroupById($groupId) == true) {
             throw new ArgumentException("Group {$group} is already contained in this viewpoint.");
         }
 
@@ -215,7 +215,7 @@ class Viewpoint implements CreateableInterface
     }
 
     /**
-     * Finds an action group by id.
+     * Returns an action group by id or fails.
      * @param $actionGroupId
      * @return ActionGroup|null
      */
@@ -227,26 +227,10 @@ class Viewpoint implements CreateableInterface
                 return $g;
             }
         }
+
         return null;
     }
 
-    /**
-     * Checks if the viewpoint has a certain action group.
-     * @param string $actionGroupId
-     * @return bool
-     */
-    public function hasActionGroup(string $actionGroupId): bool
-    {
-        $groups = $this->getActionGroups();
-        foreach ($groups as $g) {
-            if ($g->getId() == $actionGroupId) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
     /**
      * Add the specified action to the group with the provided id. Does nothing
      * if the id is not present.
@@ -306,7 +290,7 @@ class Viewpoint implements CreateableInterface
     /**
      * Returns a single data field
      * @param string $fieldname Fieldname
-     * @param type $default default value
+     * @param mixed $default default value
      * @return mixed
      */
     public function getDataField(string $fieldname, $default = null)
@@ -325,9 +309,10 @@ class Viewpoint implements CreateableInterface
 
     /**
      * Returns the action that corresponds to the given ID, if present.
+     * @param string $id
      * @return Action|null
      */
-    public function findActionById(string $id)
+    public function findActionById(string $id): ?Action
     {
         foreach ($this->getActionGroups() as $group) {
             foreach ($group->getActions() as $a) {
@@ -336,6 +321,7 @@ class Viewpoint implements CreateableInterface
                 }
             }
         }
+
         return null;
     }
 
diff --git a/tests/Models/ViewpointTest.php b/tests/Models/ViewpointTest.php
index 1400876..5b80894 100644
--- a/tests/Models/ViewpointTest.php
+++ b/tests/Models/ViewpointTest.php
@@ -210,4 +210,53 @@ class ViewpointTest extends CoreModelTestCase
         $characterScene->addDescriptionParagraph("You enjoy being here.");
         $this->assertSame("You enjoy being here.", $characterScene->getDescription());
     }
+
+    public function testIfGetActionGroupByIdReturnsTheCorrectActionGroupOrNull()
+    {
+        $em = $this->getEntityManager();
+
+        $a1 = new Action(1);
+        $a2 = new Action(2);
+        $a3 = new Action(3);
+
+        $ag1 = new ActionGroup('id1', 'title1', 42);
+        $ag1->setActions([
+            $a1,
+            $a2,
+            $a3
+        ]);
+        $ag2 = new ActionGroup('id2', 'title2', 101);
+        $ag2->setActions([
+            new Action(4)
+        ]);
+
+        $actionGroups = [
+            $ag1,
+            $ag2
+        ];
+
+        $input = $em->getRepository(Viewpoint::class)->find(2);
+        $input->setActionGroups($actionGroups);
+        $input->save($em);
+
+        $em->clear();
+
+        /** @var Viewpoint $viewpoint */
+        $viewpoint = $em->getRepository(Viewpoint::class)->find(2);
+
+        $actionGroupId1 = $viewpoint->findActionGroupById("id1");
+        $actionGroupId2 = $viewpoint->findActionGroupById("id2");
+        $actionGroupId3 = $viewpoint->findActionGroupById("id3");
+
+        $this->assertInstanceOf(ActionGroup::class, $actionGroupId1);
+        $this->assertInstanceOf(ActionGroup::class, $actionGroupId2);
+        $this->assertNull($actionGroupId3);
+
+        $actions = $actionGroupId1->getActions();
+        foreach ($actions as $action) {
+            $this->assertSame($action, $viewpoint->findActionById($action->getId()));
+        }
+
+        $this->assertNull($viewpoint->findActionById("anId"));
+    }
 }