Skip to content

Fix/server cancellation revoke #1355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
KroZen-Dev wants to merge 5 commits into
base: development
Choose a base branch
from
+87 −11
Open

Fix/server cancellation revoke #1355

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
08df3a9
Added a method to the web route
KroZen-Dev Jun 16, 2026
b618398
Update index.blade.php
KroZen-Dev Jun 16, 2026
7cd4f6d
Update ServerController.php
KroZen-Dev Jun 16, 2026
9919e1f
Fix 405 error on server cancellation
KroZen-Dev Jun 16, 2026
3fc8d79
Corrected for a short class, since imports are already higher
KroZen-Dev Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions app/Http/Controllers/ServerController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,19 +354,57 @@ private function handleServerDeletion(Server $server): void
Cache::forget('user_credits_left:' . $server->user_id);
}

public function cancel(Server $server): RedirectResponse
public function cancel(Server $server): \Illuminate\Http\Response|RedirectResponse
{
if ($server->user_id !== Auth::id()) {
return back()->with('error', __('This is not your Server!'));
}

try {
$server->update(['canceled' => now()]);

if (request()->expectsJson()) {
return response()->noContent();
}

return redirect()->route('servers.index')
->with('success', __('Server canceled'));
} catch (Exception $e) {
report($e);

if (request()->expectsJson()) {
return response()->json(['error' => __('Server cancellation failed')], 500);
}

return redirect()->route('servers.index')
->with('error', __('Server cancellation failed'));
}
}

public function uncancel(Server $server): \Illuminate\Http\Response|RedirectResponse
{
if ($server->user_id !== Auth::id()) {
return back()->with('error', __('This is not your Server!'));
}

try {
$server->update(['canceled' => null]);

if (request()->expectsJson()) {
return response()->noContent();
}

return redirect()->route('servers.index')
->with('success', __('Server cancellation has been revoked'));
} catch (Exception $e) {
report($e);

if (request()->expectsJson()) {
return response()->json(['error' => __('Server cancellation revoke failed')], 500);
}

return redirect()->route('servers.index')
->with('error', __('Server cancellation failed: ') . $e->getMessage());
->with('error', __('Server cancellation revoke failed'));
}
}

Expand Down
1 change: 1 addition & 0 deletions routes/web.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
Route::get('notifications/readAll', [NotificationController::class, 'readAll'])->name('notifications.readAll');
Route::resource('notifications', NotificationController::class);
Route::patch('/servers/cancel/{server}', [ServerController::class, 'cancel'])->name('servers.cancel');
Route::patch('/servers/{server}/uncancel', [App\Http\Controllers\ServerController::class, 'uncancel'])->name('servers.uncancel');

@MrWeez MrWeez Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Why are you not following existing route naming? We have /servers/cancel/:id, so why are you using /servers/:id/uncancel. But at the same time I can agree that /servers/:id/:action format is better from hierarchy point, so if you update old route to use same format it will be ok.
  2. You're using full namespace while we already have ServerController class imported on L27

@KroZen-Dev KroZen-Dev Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's exactly the same style: /servers/{server}/billing_priority

Screenshot_2026-06-18-08-09-55-200_com.github.android.png

@MrWeez MrWeez Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's exactly the same style: /servers/{server}/billing_priority

Screenshot_2026-06-18-08-09-55-200_com.github.android.png

I'm not arguing that you chose a more correct format, but we're talking about uncancel route that does opposite of cancel so logically new route should follow its naming pattern OR old one should be adjusted to have same naming pattern as new and other routes.

Route::post('/servers/validateDeploymentVariables', [ServerController::class, 'validateDeploymentVariables'])->name('servers.validateDeploymentVariables');
Route::patch('/servers/{server}/billing_priority', [ServerController::class, 'updateBillingPriority'])->name('servers.updateBillingPriority');
Route::delete('/servers/{server}', [ServerController::class, 'destroy'])->name('servers.destroy');
Expand Down
55 changes: 46 additions & 9 deletions themes/default/views/servers/index.blade.php
View file
Open in desktop

@MrWeez MrWeez Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you removed all comments? They can be useful. Leaving comments in the code is a good practice so that feature devs can easily understand code/function meaning without reading it

@KroZen-Dev KroZen-Dev Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very rare for contributors to leave comments in code. You do not write such remarks to them, so, please do not write to me too

@MrWeez MrWeez Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not asking you to leave comments, I'm asking you not to delete existing ones without reason

@KroZen-Dev KroZen-Dev Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please tell me, is this comment in the right place? ("delete server" in the logic of the cancel button)

Screenshot_2026-06-18-08-13-11-550_com.github.android.png

@KroZen-Dev KroZen-Dev Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for the full namespace of the controller, I agree, I will correct for a short class

@MrWeez MrWeez Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please tell me, is this comment in the right place? ("delete server" in the logic of the cancel button)

Screenshot_2026-06-18-08-13-11-550_com.github.android.png

I agree that the comment is incorrect, and was probably just copied from the delete method and forgotten to be renamed. But is that a reason to delete it? Why not just fix it if you notice a typo instead of deleting it?

@KroZen-Dev KroZen-Dev Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mentioned not deleting comments without a reason. The reason here is that the comment was completely wrong and misleading (stating "Delete" instead of "Cancel")

​The code fetch(route('servers.cancel', ...)) is already 100% self-explanatory and doesn't need a comment to explain what it does. Keeping obvious or broken comments just litters the codebase

Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,6 @@ class="float-right mr-2 text-center btn btn-danger"

<script>
const handleServerCancel = (serverId) => {
// Handle server cancel with sweetalert
Swal.fire({
title: "{{ __('Cancel Server?') }}",
text: "{{ __('This will cancel your current server to the next billing period. It will get suspended when the current period runs out.') }}",
Expand All @@ -233,14 +232,56 @@ class="float-right mr-2 text-center btn btn-danger"
reverseButtons: true
}).then((result) => {
if (result.value) {
// Delete server
fetch("{{ route('servers.cancel', ['server' => ':serverId']) }}".replace(':serverId', serverId), {
method: 'PATCH',
headers: {
'X-CSRF-TOKEN': '{{ csrf_token() }}'
'X-CSRF-TOKEN': '{{ csrf_token() }}',
'Accept': 'application/json',
'X-Requested-With': 'XMLHttpRequest'
}
}).then((response) => {
if (response.ok) {
window.location.reload();
} else {
throw new Error('Server error');
}
}).catch((error) => {
Swal.fire({
title: "{{ __('Error') }}",
text: "{{ __('Something went wrong, please try again later.') }}",
icon: 'error',
confirmButtonColor: '#d9534f',
})
})
}
})
}

const handleServerUncancel = (serverId) => {
Swal.fire({
title: "{{ __('Renew Subscription?') }}",
text: "{{ __('This will revoke the cancellation and keep your server active.') }}",
icon: 'question',
confirmButtonColor: '#5cb85c',
showCancelButton: true,
confirmButtonText: "{{ __('Yes, renew!') }}",
cancelButtonText: "{{ __('No, abort!') }}",
reverseButtons: true
}).then((result) => {
if (result.value) {
fetch("{{ route('servers.uncancel', ['server' => ':serverId']) }}".replace(':serverId', serverId), {
method: 'PATCH',
headers: {
'X-CSRF-TOKEN': '{{ csrf_token() }}',
'Accept': 'application/json',
'X-Requested-With': 'XMLHttpRequest'
}
}).then((response) => {
if (response.ok) {
window.location.reload();
} else {
throw new Error('Server error');
}
}).then(() => {
window.location.reload();
}).catch((error) => {
Swal.fire({
title: "{{ __('Error') }}",
Expand All @@ -249,7 +290,6 @@ class="float-right mr-2 text-center btn btn-danger"
confirmButtonColor: '#d9534f',
})
})
return
}
})
}
Expand All @@ -266,7 +306,6 @@ class="float-right mr-2 text-center btn btn-danger"
reverseButtons: true
}).then((result) => {
if (result.value) {
// Delete server
fetch("{{ route('servers.destroy', ['server' => ':serverId']) }}".replace(':serverId', serverId), {
method: 'DELETE',
headers: {
Expand All @@ -282,10 +321,8 @@ class="float-right mr-2 text-center btn btn-danger"
confirmButtonColor: '#d9534f',
})
})
return
}
});

}

document.addEventListener('DOMContentLoaded', () => {
Expand Down
Loading