Fix bug preventing changing of the server startup on first save attempt.

DaneEveritt · DaneEveritt · commit 26eeffd7649c · 2017-11-11T15:07:01.000-06:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,10 @@ This project follows [Semantic Versioning](http://semver.org) guidelines.
 * `[beta.1]` — Fixes bug causing inability to create new servers on the Panel.
 * `[beta.1]` — Fixes bug causing inability to delete an allocation due to misconfigured JS.
 * `[beta.1]` — Fixes bug causing inability to set the IP alias for an allocation to an empty value.
+* `[beta.1]` — Fixes bug that caused startup changes to not propigate to the server correctly on the first save.
+
+### Changed
+* Moved Docker image setting to be on the startup management page for a server rather than the details page. This value changes based on the Nest and Egg that are selected.
 
 ## v0.7.0-beta.1 (Derelict Dermodactylus)
 ### Added
diff --git a/app/Contracts/Repository/ServerRepositoryInterface.php b/app/Contracts/Repository/ServerRepositoryInterface.php
@@ -95,14 +95,15 @@ public function getDataForCreation(Server $server, bool $refresh = false): Serve
     public function getWithDatabases($id);
 
     /**
-     * Return data about the daemon service in a consumable format.
+     * Get data for use when updating a server on the Daemon. Returns an array of
+     * the egg and pack UUID which are used for build and rebuild. Only loads relations
+     * if they are missing, or refresh is set to true.
      *
-     * @param int $id
+     * @param \Pterodactyl\Models\Server $server
+     * @param bool                       $refresh
      * @return array
-     *
-     * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException
      */
-    public function getDaemonServiceData($id);
+    public function getDaemonServiceData(Server $server, bool $refresh = false): array;
 
     /**
      * Return an array of server IDs that a given user can access based on owner and subuser permissions.
diff --git a/app/Http/Controllers/Admin/ServersController.php b/app/Http/Controllers/Admin/ServersController.php
@@ -410,25 +410,6 @@ public function setDetails(Request $request, Server $server)
         return redirect()->route('admin.servers.view.details', $server->id);
     }
 
-    /**
-     * Set the new docker container for a server.
-     *
-     * @param \Illuminate\Http\Request   $request
-     * @param \Pterodactyl\Models\Server $server
-     * @return \Illuminate\Http\RedirectResponse
-     *
-     * @throws \Pterodactyl\Exceptions\DisplayException
-     * @throws \Pterodactyl\Exceptions\Model\DataValidationException
-     * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException
-     */
-    public function setContainer(Request $request, Server $server)
-    {
-        $this->detailsModificationService->setDockerImage($server, $request->input('docker_image'));
-        $this->alert->success(trans('admin/server.alerts.docker_image_updated'))->flash();
-
-        return redirect()->route('admin.servers.view.details', $server->id);
-    }
-
     /**
      * Toggles the install status for a server.
      *
diff --git a/app/Repositories/Eloquent/ServerRepository.php b/app/Repositories/Eloquent/ServerRepository.php
@@ -187,21 +187,27 @@ public function getWithDatabases($id)
     }
 
     /**
-     * {@inheritdoc}
+     * Get data for use when updating a server on the Daemon. Returns an array of
+     * the egg and pack UUID which are used for build and rebuild. Only loads relations
+     * if they are missing, or refresh is set to true.
+     *
+     * @param \Pterodactyl\Models\Server $server
+     * @param bool                       $refresh
+     * @return array
      */
-    public function getDaemonServiceData($id)
+    public function getDaemonServiceData(Server $server, bool $refresh = false): array
     {
-        Assert::integerish($id, 'First argument passed to getDaemonServiceData must be integer, received %s.');
+        if (! $server->relationLoaded('egg') || $refresh) {
+            $server->load('egg');
+        }
 
-        $instance = $this->getBuilder()->with('egg.nest', 'pack')->find($id, $this->getColumns());
-        if (! $instance) {
-            throw new RecordNotFoundException();
+        if (! $server->relationLoaded('pack') || $refresh) {
+            $server->load('pack');
         }
 
         return [
-            'type' => $instance->egg->nest->folder,
-            'option' => $instance->egg->tag,
-            'pack' => (! is_null($instance->pack_id)) ? $instance->pack->uuid : null,
+            'egg' => $server->getRelation('egg')->uuid,
+            'pack' => is_null($server->getRelation('pack')) ? null : $server->getRelation('pack')->uuid,
         ];
     }
 
diff --git a/app/Services/Servers/StartupModificationService.php b/app/Services/Servers/StartupModificationService.php
@@ -99,14 +99,17 @@ public function handle(Server $server, array $data)
             });
         }
 
-        $daemonData = ['build' => [
-            'env|overwrite' => $this->environmentService->handle($server),
-        ]];
-
+        $daemonData = [];
         if ($this->isUserLevel(User::USER_LEVEL_ADMIN)) {
             $this->updateAdministrativeSettings($data, $server, $daemonData);
         }
 
+        $daemonData = array_merge_recursive($daemonData, [
+            'build' => [
+                'env|overwrite' => $this->environmentService->handle($server),
+            ],
+        ]);
+
         try {
             $this->daemonServerRepository->setNode($server->node_id)->setAccessServer($server->uuid)->update($daemonData);
         } catch (RequestException $exception) {
@@ -136,17 +139,15 @@ private function updateAdministrativeSettings(array $data, Server &$server, arra
             'egg_id' => array_get($data, 'egg_id', $server->egg_id),
             'pack_id' => array_get($data, 'pack_id', $server->pack_id) > 0 ? array_get($data, 'pack_id', $server->pack_id) : null,
             'skip_scripts' => isset($data['skip_scripts']),
+            'image' => array_get($data, 'docker_image', $server->image),
         ]);
 
-        if (
-            $server->nest_id != array_get($data, 'nest_id', $server->nest_id) ||
-            $server->egg_id != array_get($data, 'egg_id', $server->egg_id) ||
-            $server->pack_id != array_get($data, 'pack_id', $server->pack_id)
-        ) {
-            $daemonData['service'] = array_merge(
-                $this->repository->withColumns(['id', 'egg_id', 'pack_id'])->getDaemonServiceData($server->id),
+        $daemonData = array_merge($daemonData, [
+            'build' => ['image' => $server->image],
+            'service' => array_merge(
+                $this->repository->getDaemonServiceData($server, true),
                 ['skip_scripts' => isset($data['skip_scripts'])]
-            );
-        }
+            ),
+        ]);
     }
 }
diff --git a/resources/themes/pterodactyl/admin/servers/view/details.blade.php b/resources/themes/pterodactyl/admin/servers/view/details.blade.php
@@ -39,7 +39,7 @@
     </div>
 </div>
 <div class="row">
-    <div class="col-sm-6">
+    <div class="col-xs-12">
         <div class="box box-primary">
             <div class="box-header with-border">
                 <h3 class="box-title">Base Information</h3>
@@ -63,15 +63,6 @@
                         <textarea name="description" rows="3" class="form-control">{{ old('description', $server->description) }}</textarea>
                         <p class="text-muted small">A brief description of this server.</p>
                     </div>
-                    <div class="form-group">
-                        <label for="name" class="control-label">Daemon Secret Token</label>
-                        <input type="text" disabled value="{{ $server->daemonSecret }}" class="form-control" />
-                        <p class="text-muted small">This token should not be shared with anyone as it has full control over this server.</p>
-                    </div>
-                    <div class="form-group">
-                        <input type="checkbox" name="reset_token" id="pResetToken"/> <label for="pResetToken">Reset Daemon Token</label>
-                        <p class="text-muted small">Resetting this token will cause any requests using the old token to fail.</p>
-                    </div>
                 </div>
                 <div class="box-footer">
                     {!! csrf_field() !!}
@@ -81,27 +72,6 @@
             </form>
         </div>
     </div>
-    <div class="col-sm-6">
-        <div class="box box-success">
-            <div class="box-header with-border">
-                <h3 class="box-title">Container Setup</h3>
-            </div>
-            <form action="{{ route('admin.servers.view.details.container', $server->id) }}" method="POST">
-                <div class="box-body">
-                    <div class="form-group">
-                        <label for="name" class="control-label">Docker Image</label>
-                        <input type="text" name="docker_image" value="{{ $server->image }}" class="form-control" />
-                        <p class="text-muted small">The docker image to use for this server. The default image for this service and option combination is <code>{{ $server->egg->docker_image }}</code>.</p>
-                    </div>
-                </div>
-                <div class="box-footer">
-                    {!! csrf_field() !!}
-                    {!! method_field('PATCH') !!}
-                    <input type="submit" class="btn btn-sm btn-primary" value="Update Docker Container" />
-                </div>
-            </form>
-        </div>
-    </div>
 </div>
 @endsection
 
diff --git a/resources/themes/pterodactyl/admin/servers/view/startup.blade.php b/resources/themes/pterodactyl/admin/servers/view/startup.blade.php
@@ -109,6 +109,18 @@
                     </div>
                 </div>
             </div>
+            <div class="box">
+                <div class="box-header with-border">
+                    <h3 class="box-title">Docker Container Configuration</h3>
+                </div>
+                <div class="box-body">
+                    <div class="form-group">
+                        <label for="pDockerImage" class="control-label">Image</label>
+                        <input type="text" name="docker_image" id="pDockerImage" value="{{ $server->image }}" class="form-control" />
+                        <p class="text-muted small">The Docker image to use for this server. The default image for the selected egg is <code id="setDefaultImage"></code>.</p>
+                    </div>
+                </div>
+            </div>
         </div>
         <div class="col-md-6">
             <div class="row" id="appendVariablesTo"></div>
@@ -143,7 +155,11 @@
             var parentChain = _.get(Pterodactyl.nests, $('#pNestId').val(), null);
             var objectChain = _.get(parentChain, 'eggs.' + $(this).val(), null);
 
-            $('#pDefaultContainer').val(_.get(objectChain, 'docker_image', 'not defined!'));
+            $('#setDefaultImage').html(_.get(objectChain, 'docker_image', 'undefined'));
+            $('#pDockerImage').val(_.get(objectChain, 'docker_image', 'undefined'));
+            if (objectChain.id === parseInt('{{ $server->egg_id }}')) {
+                $('#pDockerImage').val('{{ $server->image }}');
+            }
 
             if (!_.get(objectChain, 'startup', false)) {
                 $('#pDefaultStartupCommand').val(_.get(parentChain, 'startup', 'ERROR: Startup Not Defined!'));
diff --git a/routes/admin.php b/routes/admin.php
@@ -105,7 +105,6 @@
     Route::post('/view/{server}/delete', 'ServersController@delete');
 
     Route::patch('/view/{server}/details', 'ServersController@setDetails');
-    Route::patch('/view/{server}/details/container', 'ServersController@setContainer')->name('admin.servers.view.details.container');
     Route::patch('/view/{server}/database', 'ServersController@resetDatabasePassword');
 
     Route::delete('/view/{server}/database/{database}/delete', 'ServersController@deleteDatabase')->name('admin.servers.view.database.delete');
diff --git a/tests/Unit/Services/Servers/StartupModificationServiceTest.php b/tests/Unit/Services/Servers/StartupModificationServiceTest.php
@@ -107,6 +107,7 @@ public function testStartupModificationAsAdminUser()
     {
         $model = factory(Server::class)->make([
             'egg_id' => 123,
+            'image' => 'docker:image',
         ]);
 
         $this->connection->shouldReceive('beginTransaction')->withNoArgs()->once()->andReturnNull();
@@ -121,22 +122,29 @@ public function testStartupModificationAsAdminUser()
             'variable_id' => 1,
         ], ['variable_value' => 'stored-value'])->once()->andReturnNull();
 
-        $this->environmentService->shouldReceive('handle')->with($model)->once()->andReturn(['env']);
-
         $this->repository->shouldReceive('update')->with($model->id, m::subset([
             'installed' => 0,
             'egg_id' => 456,
             'pack_id' => 789,
+            'image' => 'docker:image',
         ]))->once()->andReturn($model);
-        $this->repository->shouldReceive('withColumns->getDaemonServiceData')->with($model->id)->once()->andReturn([]);
+        $this->repository->shouldReceive('getDaemonServiceData')->with($model, true)->once()->andReturn([
+            'egg' => 'abcd1234',
+            'pack' => 'xyz987',
+        ]);
+
+        $this->environmentService->shouldReceive('handle')->with($model)->once()->andReturn(['env']);
 
         $this->daemonServerRepository->shouldReceive('setNode')->with($model->node_id)->once()->andReturnSelf();
         $this->daemonServerRepository->shouldReceive('setAccessServer')->with($model->uuid)->once()->andReturnSelf();
         $this->daemonServerRepository->shouldReceive('update')->with([
             'build' => [
                 'env|overwrite' => ['env'],
+                'image' => $model->image,
             ],
             'service' => [
+                'egg' => 'abcd1234',
+                'pack' => 'xyz987',
                 'skip_scripts' => false,
             ],
         ])->once()->andReturnSelf();
@@ -145,7 +153,7 @@ public function testStartupModificationAsAdminUser()
 
         $service = $this->getService();
         $service->setUserLevel(User::USER_LEVEL_ADMIN);
-        $service->handle($model, ['egg_id' => 456, 'pack_id' => 789, 'environment' => ['test' => 'abcd1234']]);
+        $service->handle($model, ['docker_image' => 'docker:image', 'egg_id' => 456, 'pack_id' => 789, 'environment' => ['test' => 'abcd1234']]);
         $this->assertTrue(true);
     }
 
