Address CodeRabbit review feedback

- Fix null-safe operator on currentTeam() call in Upgrade.php
- Add --rm flag to docker run in upgrade.sh for cleanup consistency
- Store beforeunload handler as named reference and clean up on success
- Add clarifying comments for upgrade method calls
- Add error state handling with close option in upgrade modal
- Add step mapping documentation comment in upgrade-progress component

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Andras Bacsai 2025-12-13 21:42:19 +01:00
parent dc12cbc6a0
commit 20a9855c04
4 changed files with 78 additions and 7 deletions

View file

@ -55,7 +55,7 @@ public function upgrade()
public function getUpgradeStatus(): array
{
// Only root team members can view upgrade status
if (auth()->user()?->currentTeam()->id !== 0) {
if (auth()->user()?->currentTeam()?->id !== 0) {
return ['status' => 'none'];
}

View file

@ -1,5 +1,14 @@
@props(['step' => 0])
{{--
Step Mapping (Backend UI):
Backend steps 1-2 (config download, env update) UI Step 1: Preparing
Backend step 3 (pulling images) UI Step 2: Helper + UI Step 3: Image
Backend steps 4-5 (stop/start containers) UI Step 4: Restart
Backend step 6 (complete) mapped in JS mapStepToUI() in upgrade.blade.php
The currentStep variable is inherited from parent Alpine component (upgradeModal).
--}}
<div class="w-full max-w-md mx-auto" x-data="{ activeStep: {{ $step }} }" x-effect="activeStep = $el.closest('[x-data]')?.__x?.$data?.currentStep ?? {{ $step }}">
<div class="flex items-center justify-between">
{{-- Step 1: Preparing --}}

View file

@ -51,7 +51,7 @@ class="relative w-full py-6 border rounded-sm min-w-full lg:min-w-[36rem] max-w-
{{ $currentVersion }} <span class="mx-1">&rarr;</span> {{ $latestVersion }}
</div>
</div>
<button x-show="!showProgress" @click="modalOpen=false"
<button x-show="!showProgress || upgradeError" @click="upgradeError ? closeErrorModal() : modalOpen=false"
class="absolute top-0 right-0 flex items-center justify-center w-8 h-8 mt-5 mr-5 text-gray-600 rounded-full hover:text-gray-800 hover:bg-gray-50 dark:text-neutral-400 dark:hover:text-white dark:hover:bg-coolgray-300">
<svg class="w-5 h-5" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24"
stroke-width="1.5" stroke="currentColor">
@ -79,7 +79,7 @@ class="absolute top-0 right-0 flex items-center justify-center w-8 h-8 mt-5 mr-5
{{-- Current Status Message --}}
<div class="p-4 rounded-lg bg-neutral-200 dark:bg-coolgray-200">
<div class="flex items-center gap-3">
<template x-if="!upgradeComplete">
<template x-if="!upgradeComplete && !upgradeError">
<svg class="w-5 h-5 text-warning animate-spin"
xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24">
<circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor"
@ -97,6 +97,14 @@ class="absolute top-0 right-0 flex items-center justify-center w-8 h-8 mt-5 mr-5
clip-rule="evenodd" />
</svg>
</template>
<template x-if="upgradeError">
<svg class="w-5 h-5 text-error" xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 20 20" fill="currentColor">
<path fill-rule="evenodd"
d="M18 10a8 8 0 11-16 0 8 8 0 0116 0zm-7 4a1 1 0 11-2 0 1 1 0 012 0zm-1-9a1 1 0 00-1 1v4a1 1 0 102 0V6a1 1 0 00-1-1z"
clip-rule="evenodd" />
</svg>
</template>
<span x-text="currentStatus" class="text-sm"></span>
</div>
</div>
@ -113,6 +121,18 @@ class="font-bold text-warning"></span> seconds...
</x-forms.button>
</div>
</template>
{{-- Error State with Close Button --}}
<template x-if="upgradeError">
<div class="flex flex-col items-center gap-4">
<p class="text-sm text-neutral-600 dark:text-neutral-400">
Check the logs on the server at /data/coolify/source/upgrade*.
</p>
<x-forms.button @click="closeErrorModal()" type="button">
Close
</x-forms.button>
</div>
</template>
</div>
</template>
@ -168,6 +188,7 @@ class="w-24 dark:bg-coolgray-200 dark:hover:bg-coolgray-300">Cancel
elapsedTime: 0,
currentStep: 0,
upgradeComplete: false,
upgradeError: false,
successCountdown: 3,
currentVersion: config.currentVersion || '',
latestVersion: config.latestVersion || '',
@ -178,12 +199,16 @@ class="w-24 dark:bg-coolgray-200 dark:hover:bg-coolgray-300">Cancel
this.currentStep = 1;
this.currentStatus = 'Starting upgrade...';
this.startTimer();
// Trigger server-side upgrade script via Livewire
this.$wire.$call('upgrade');
// Start client-side status polling
this.upgrade();
window.addEventListener('beforeunload', (event) => {
// Prevent accidental navigation during upgrade
this.beforeUnloadHandler = (event) => {
event.preventDefault();
event.returnValue = '';
});
};
window.addEventListener('beforeunload', this.beforeUnloadHandler);
},
startTimer() {
@ -259,6 +284,11 @@ class="w-24 dark:bg-coolgray-200 dark:hover:bg-coolgray-300">Cancel
clearInterval(this.elapsedInterval);
this.elapsedInterval = null;
}
// Remove beforeunload handler now that upgrade is complete
if (this.beforeUnloadHandler) {
window.removeEventListener('beforeunload', this.beforeUnloadHandler);
this.beforeUnloadHandler = null;
}
this.upgradeComplete = true;
this.currentStep = 5;
@ -278,6 +308,38 @@ class="w-24 dark:bg-coolgray-200 dark:hover:bg-coolgray-300">Cancel
window.location.reload();
},
showError(message) {
// Stop all intervals
if (this.checkHealthInterval) {
clearInterval(this.checkHealthInterval);
this.checkHealthInterval = null;
}
if (this.checkUpgradeStatusInterval) {
clearInterval(this.checkUpgradeStatusInterval);
this.checkUpgradeStatusInterval = null;
}
if (this.elapsedInterval) {
clearInterval(this.elapsedInterval);
this.elapsedInterval = null;
}
// Remove beforeunload handler so user can close modal
if (this.beforeUnloadHandler) {
window.removeEventListener('beforeunload', this.beforeUnloadHandler);
this.beforeUnloadHandler = null;
}
this.upgradeError = true;
this.currentStatus = `Error: ${message}`;
},
closeErrorModal() {
this.modalOpen = false;
this.showProgress = false;
this.upgradeError = false;
this.currentStatus = '';
this.currentStep = 0;
},
upgrade() {
if (this.checkUpgradeStatusInterval) return true;
this.currentStep = 1;
@ -294,7 +356,7 @@ class="w-24 dark:bg-coolgray-200 dark:hover:bg-coolgray-300">Cancel
} else if (data.status === 'complete') {
this.showSuccess();
} else if (data.status === 'error') {
this.currentStatus = `Error: ${data.message}`;
this.showError(data.message);
}
} catch (error) {
// Service is down - switch to health check mode

View file

@ -244,7 +244,7 @@ nohup bash -c "
else
log 'Using standard docker-compose configuration'
log 'Running docker compose up...'
docker run -v /data/coolify/source:/data/coolify/source -v /var/run/docker.sock:/var/run/docker.sock \${DOCKER_CONFIG_MOUNT} \${REGISTRY_URL:-ghcr.io}/coollabsio/coolify-helper:\${LATEST_HELPER_VERSION} bash -c \"LATEST_IMAGE=\${LATEST_IMAGE} docker compose --project-name coolify --env-file /data/coolify/source/.env -f /data/coolify/source/docker-compose.yml -f /data/coolify/source/docker-compose.prod.yml up -d --remove-orphans --wait --wait-timeout 60\" >>\"\$LOGFILE\" 2>&1
docker run -v /data/coolify/source:/data/coolify/source -v /var/run/docker.sock:/var/run/docker.sock \${DOCKER_CONFIG_MOUNT} --rm \${REGISTRY_URL:-ghcr.io}/coollabsio/coolify-helper:\${LATEST_HELPER_VERSION} bash -c \"LATEST_IMAGE=\${LATEST_IMAGE} docker compose --project-name coolify --env-file /data/coolify/source/.env -f /data/coolify/source/docker-compose.yml -f /data/coolify/source/docker-compose.prod.yml up -d --remove-orphans --wait --wait-timeout 60\" >>\"\$LOGFILE\" 2>&1
fi
log 'Docker compose up completed'