Fix account name validator and make sure the location CRUD works in the API for accounts.

app/Api/V1/Controllers/TagController.php

@@ -25,7 +25,8 @@ namespace FireflyIII\Api\V1\Controllers;
 
 use Carbon\Carbon;
 use FireflyIII\Api\V1\Requests\DateRequest;
-use FireflyIII\Api\V1\Requests\TagRequest;
+use FireflyIII\Api\V1\Requests\TagUpdateRequest;
+use FireflyIII\Api\V1\Requests\TagStoreRequest;
 use FireflyIII\Helpers\Collector\GroupCollectorInterface;
 use FireflyIII\Models\Tag;
 use FireflyIII\Repositories\Tag\TagRepositoryInterface;
@@ -161,11 +162,11 @@ class TagController extends Controller
     /**
      * Store new object.
      *
-     * @param TagRequest $request
+     * @param TagStoreRequest $request
      *
      * @return JsonResponse
      */
-    public function store(TagRequest $request): JsonResponse
+    public function store(TagStoreRequest $request): JsonResponse
     {
         $rule    = $this->repository->store($request->getAll());
         $manager = $this->getManager();
@@ -234,12 +235,12 @@ class TagController extends Controller
     /**
      * Update a rule.
      *
-     * @param TagRequest $request
+     * @param TagUpdateRequest $request
      * @param Tag        $tag
      *
      * @return JsonResponse
      */
-    public function update(TagRequest $request, Tag $tag): JsonResponse
+    public function update(TagUpdateRequest $request, Tag $tag): JsonResponse
     {
         $rule    = $this->repository->update($tag, $request->getAll());
         $manager = $this->getManager();

app/Api/V1/Requests/AccountStoreRequest.php

@@ -77,6 +77,9 @@ class AccountStoreRequest extends Request
             'interest'             => 'required_if:type,liability|between:0,100|numeric',
             'interest_period'      => 'required_if:type,liability|in:daily,monthly,yearly',
             'notes'                => 'min:0|max:65536',
+            'latitude'             => 'numeric|min:-90|max:90|nullable|required_with:longitude',
+            'longitude'            => 'numeric|min:-180|max:180|nullable|required_with:latitude',
+            'zoom_level'           => 'numeric|min:0|max:80|nullable|required_with:latitude',
         ];
 
         return $rules;

app/Api/V1/Requests/AccountUpdateRequest.php

@@ -58,6 +58,11 @@ class AccountUpdateRequest extends Request
         if (null !== $this->get('include_net_worth')) {
             $includeNetWorth = $this->boolean('include_net_worth');
         }
+        $updateLocation = false;
+
+        if ($this->has('longitude') && $this->has('latitude') && $this->has('zoom_level')) {
+            $updateLocation = true;
+        }
 
         $data = [
             'name'                    => $this->nullableString('name'),
@@ -79,6 +84,10 @@ class AccountUpdateRequest extends Request
             'notes'                   => $this->nullableNlString('notes'),
             'interest'                => $this->nullableString('interest'),
             'interest_period'         => $this->nullableString('interest_period'),
+            'has_location'            => $updateLocation,
+            'longitude'               => '' === $this->string('longitude') ? null : $this->string('longitude'),
+            'latitude'                => '' === $this->string('latitude') ? null : $this->string('latitude'),
+            'zoom_level'              => '' === $this->string('zoom_level') ? null : $this->integer('zoom_level'),
         ];
 
         if ('liability' === $data['account_type']) {
@@ -102,6 +111,7 @@ class AccountUpdateRequest extends Request
         $accountRoles   = implode(',', config('firefly.accountRoles'));
         $types          = implode(',', array_keys(config('firefly.subTitlesByIdentifier')));
         $ccPaymentTypes = implode(',', array_keys(config('firefly.ccTypes')));
+
         $rules          = [
             'name'                 => sprintf('min:1|uniqueAccountForUser:%d', $account->id),
             'type'                 => sprintf('in:%s', $types),
@@ -124,6 +134,9 @@ class AccountUpdateRequest extends Request
             'interest'             => 'required_if:type,liability|between:0,100|numeric',
             'interest_period'      => 'required_if:type,liability|in:daily,monthly,yearly',
             'notes'                => 'min:0|max:65536',
+            'latitude'             => 'numeric|min:-90|max:90|nullable|required_with:longitude',
+            'longitude'            => 'numeric|min:-180|max:180|nullable|required_with:latitude',
+            'zoom_level'           => 'numeric|min:0|max:80|nullable|required_with:latitude',
         ];
 
         return $rules;

app/Api/V1/Requests/Request.php

@@ -41,13 +41,16 @@ class Request extends FireflyIIIRequest
     {
         $active          = true;
         $includeNetWorth = true;
+        $hasLocation     = false;
         if (null !== $this->get('active')) {
             $active = $this->boolean('active');
         }
         if (null !== $this->get('include_net_worth')) {
             $includeNetWorth = $this->boolean('include_net_worth');
         }
-
+        if ($this->has('longitude') && $this->has('latitude') && $this->has('zoom_level')) {
+            $hasLocation = true;
+        }
         $data = [
             'name'                    => $this->string('name'),
             'active'                  => $active,
@@ -68,6 +71,10 @@ class Request extends FireflyIIIRequest
             'notes'                   => $this->nlString('notes'),
             'interest'                => $this->string('interest'),
             'interest_period'         => $this->string('interest_period'),
+            'has_location'            => $hasLocation,
+            'longitude'               => $this->string('longitude'),
+            'latitude'                => $this->string('latitude'),
+            'zoom_level'              => $this->integer('zoom_level'),
         ];
 
         if ('liability' === $data['account_type']) {

app/Api/V1/Requests/TagStoreRequest.php

@@ -1,7 +1,6 @@
 <?php
-
 /**
- * TagRequest.php
+ * TagStoreRequest.php
  * Copyright (c) 2019 thegrumpydictator@gmail.com
  *
  * This file is part of Firefly III (https://github.com/firefly-iii).
@@ -27,13 +26,12 @@ namespace FireflyIII\Api\V1\Requests;
 use FireflyIII\Models\Tag;
 
 /**
- * Class TagRequest
+ * Class TagStoreRequest
  *
  * @codeCoverageIgnore
  *
- * TODO AFTER 4.8,0: split this into two request classes.
  */
-class TagRequest extends Request
+class TagStoreRequest extends Request
 {
 
     /**
@@ -54,7 +52,7 @@ class TagRequest extends Request
      */
     public function getAll(): array
     {
-        $data = [
+        return [
             'tag'         => $this->string('tag'),
             'date'        => $this->date('date'),
             'description' => $this->string('description'),
@@ -62,8 +60,6 @@ class TagRequest extends Request
             'longitude'   => '' === $this->string('longitude') ? null : $this->string('longitude'),
             'zoom_level'  => $this->integer('zoom_level'),
         ];
-
-        return $data;
     }
 
     /**
@@ -73,7 +69,7 @@ class TagRequest extends Request
      */
     public function rules(): array
     {
-        $rules = [
+        return [
             'tag'         => 'required|min:1|uniqueObjectForUser:tags,tag',
             'description' => 'min:1|nullable',
             'date'        => 'date|nullable',
@@ -81,17 +77,5 @@ class TagRequest extends Request
             'longitude'   => 'numeric|min:-180|max:180|nullable|required_with:latitude',
             'zoom_level'  => 'numeric|min:0|max:80|nullable',
         ];
-        switch ($this->method()) {
-            default:
-                break;
-            case 'PUT':
-            case 'PATCH':
-                /** @var Tag $tag */
-                $tag          = $this->route()->parameter('tagOrId');
-                $rules['tag'] = 'required|min:1|uniqueObjectForUser:tags,tag,' . $tag->id;
-                break;
-        }
-
-        return $rules;
     }
 }

app/Api/V1/Requests/TagUpdateRequest.php

@@ -0,0 +1,82 @@
+<?php
+
+/**
+ * TagUpdateRequest.php
+ * Copyright (c) 2019 thegrumpydictator@gmail.com
+ *
+ * This file is part of Firefly III (https://github.com/firefly-iii).
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+declare(strict_types=1);
+
+namespace FireflyIII\Api\V1\Requests;
+
+/**
+ * Class TagUpdateRequest
+ *
+ * @codeCoverageIgnore
+ *
+ */
+class TagUpdateRequest extends Request
+{
+
+    /**
+     * Authorize logged in users.
+     *
+     * @return bool
+     */
+    public function authorize(): bool
+    {
+        // Only allow authenticated users
+        return auth()->check();
+    }
+
+    /**
+     * Get all data from the request.
+     *
+     * @return array
+     */
+    public function getAll(): array
+    {
+        return [
+            'tag'         => $this->string('tag'),
+            'date'        => $this->date('date'),
+            'description' => $this->string('description'),
+            'latitude'    => '' === $this->string('latitude') ? null : $this->string('latitude'),
+            'longitude'   => '' === $this->string('longitude') ? null : $this->string('longitude'),
+            'zoom_level'  => $this->integer('zoom_level'),
+        ];
+    }
+
+    /**
+     * The rules that the incoming request must be matched against.
+     *
+     * @return array
+     */
+    public function rules(): array
+    {
+        $tag = $this->route()->parameter('tagOrId');
+
+        return [
+            'tag'         => 'required|min:1|uniqueObjectForUser:tags,tag,' . $tag->id,
+            'description' => 'min:1|nullable',
+            'date'        => 'date|nullable',
+            'latitude'    => 'numeric|min:-90|max:90|nullable|required_with:longitude',
+            'longitude'   => 'numeric|min:-180|max:180|nullable|required_with:latitude',
+            'zoom_level'  => 'numeric|min:0|max:80|nullable',
+        ];
+    }
+}

app/Factory/AccountFactory.php

@@ -134,7 +134,7 @@ class AccountFactory
             $this->updateNote($return, $data['notes'] ?? '');
 
             // store location
-            if (true === ($data['has_location'] ?? true) && null !== $return) {
+            if (true === ($data['has_location'] ?? false) && null !== $return) {
                 $location             = new Location;
                 $location->latitude   = $data['latitude'] ?? 52.3167;
                 $location->longitude  = $data['longitude'] ?? 5.55;

app/Services/Internal/Update/AccountUpdateService.php

@@ -101,20 +101,28 @@ class AccountUpdateService
         $this->updateMetaData($account, $data);
 
         // update, delete or create location:
-        $hasLocation = $data['has_location'] ?? false;
+        $updateLocation = $data['has_location'] ?? false;
-        if (false === $hasLocation) {
+
-            $account->locations()->delete();
+        // location must be updated?
-        }
+        if (true === $updateLocation) {
-        if (true === $hasLocation) {
+            // if all set to NULL, delete
-            $location = $this->accountRepository->getLocation($account);
+            if(null === $data['latitude'] && null === $data['longitude'] && null === $data['zoom_level']) {
-            if (null === $location) {
+                $account->locations()->delete();
-                $location = new Location;
+            }
-                $location->locatable()->associate($account);
+
+            // otherwise, update or create.
+            if(!(null === $data['latitude'] && null === $data['longitude'] && null === $data['zoom_level'])) {
+                $location = $this->accountRepository->getLocation($account);
+                if (null === $location) {
+                    $location = new Location;
+                    $location->locatable()->associate($account);
+                }
+
+                $location->latitude = $data['latitude'] ?? config('firefly.default_location.latitude');
+                $location->longitude = $data['longitude'] ?? config('firefly.default_location.longitude');
+                $location->zoom_level = $data['zoom_level'] ?? config('firefly.default_location.zoom_level');
+                $location->save();
             }
-            $location->latitude = $data['latitude'] ?? config('firefly.default_location.latitude');
-            $location->longitude = $data['longitude'] ?? config('firefly.default_location.longitude');
-            $location->zoom_level = $data['zoom_level'] ?? config('firefly.default_location.zoom_level');
-            $location->save();
         }
 
         // has valid initial balance (IB) data?

app/Validation/FireflyValidator.php

@@ -376,20 +376,23 @@ class FireflyValidator extends Validator
     public function validateUniqueAccountForUser($attribute, $value, $parameters): bool
     {
         // because a user does not have to be logged in (tests and what-not).
-
         if (!auth()->check()) {
             return $this->validateAccountAnonymously();
         }
         if (isset($this->data['what'])) {
+
             return $this->validateByAccountTypeString($value, $parameters, $this->data['what']);
         }
         if (isset($this->data['type'])) {
             return $this->validateByAccountTypeString($value, $parameters, $this->data['type']);
         }
-
         if (isset($this->data['account_type_id'])) {
             return $this->validateByAccountTypeId($value, $parameters);
         }
+        $parameterId = $parameters[0] ?? null;
+        if (null !== $parameterId) {
+            return $this->validateByParameterId((int)$parameterId, $value);
+        }
         if (isset($this->data['id'])) {
             return $this->validateByAccountId($value);
         }
@@ -545,16 +548,33 @@ class FireflyValidator extends Validator
         $ignore = $existingAccount->id;
 
         /** @var Collection $set */
-        $set = auth()->user()->accounts()->where('account_type_id', $type->id)->where('id', '!=', $ignore)->get();
+        $entry = auth()->user()->accounts()->where('account_type_id', $type->id)->where('id', '!=', $ignore)
-        // TODO no longer need to loop like this
+                       ->where('name', $value)
-        /** @var Account $entry */
+                       ->first();
-        foreach ($set as $entry) {
-            if ($entry->name === $value) {
-                return false;
-            }
-        }
 
-        return true;
+        return null === $entry;
+    }
+
+
+    /**
+     * @param $value
+     *
+     * @return bool
+     */
+    private function validateByParameterId(int $accountId, $value): bool
+    {
+        /** @var Account $existingAccount */
+        $existingAccount = Account::find($accountId);
+
+        $type   = $existingAccount->accountType;
+        $ignore = $existingAccount->id;
+
+        /** @var Collection $set */
+        $entry = auth()->user()->accounts()->where('account_type_id', $type->id)->where('id', '!=', $ignore)
+                       ->where('name', $value)
+                       ->first();
+
+        return null === $entry;
     }
 
     /**