uninstall: Fix issue with uninstall of apps that have no backup

Closes: #2342.

- Define a helper method to figure if app has backup. Use the helper method in
main AppView.

- Minor refactor of repeated code in uninstall view that gets the app and
app_id.

- Pass whether an app has backup or not into form. Delete backup related form
fields when backup is not supported. UX when fields are disabled is not nice, it
is not explained why fields are disabled. Better UX seems to be to remove the
backup fields entirely.

Tests:

- Install wireguard. In the uninstall form backup fields don't appear. Uninstall
is successful.

- Install Bepasty. In the uninstall form back fields are shown. Uninstall is
successful with and without a backup. In case backup is chosen, backup is
created and can successfully be restored.

- Run functional tests for bepasty. For wireguard uninstall test succeeds.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
This commit is contained in:
Sunil Mohan Adapa 2023-04-28 11:55:09 -07:00 committed by James Valleroy
parent 43a2734917
commit 8b7c0fb0bf
No known key found for this signature in database
GPG Key ID: 77C0C75E7B650808
2 changed files with 43 additions and 17 deletions

View File

@ -39,6 +39,14 @@ class UninstallForm(forms.Form):
repository = forms.ChoiceField(label=_('Repository to backup to'),
choices=_get_repository_choices)
def __init__(self, **kwargs):
"""Determine if backup fields must be used."""
has_backup_restore = kwargs.pop('has_backup_restore', True)
super().__init__(**kwargs)
if not has_backup_restore:
del self.fields['should_backup']
del self.fields['repository']
class DomainSelectionForm(forms.Form):
"""Form for selecting a domain name to be used for

View File

@ -66,6 +66,15 @@ def _get_redirect_url_from_param(request):
return reverse('index')
def _has_backup_restore(app):
"""Return whether an app implements backup/restore."""
from plinth.modules.backups.components import BackupRestore
return any([
component.has_data
for component in app.get_components_of_type(BackupRestore)
])
@public
def index(request):
"""Serve the main index page."""
@ -272,11 +281,7 @@ class AppView(FormView):
from plinth.modules.firewall.components import Firewall
context['firewall'] = self.app.get_components_of_type(Firewall)
from plinth.modules.backups.components import BackupRestore
context['has_backup_restore'] = any([
component.has_data
for component in self.app.get_components_of_type(BackupRestore)
])
context['has_backup_restore'] = _has_backup_restore(self.app)
return context
@ -400,21 +405,35 @@ class UninstallView(FormView):
form_class = forms.UninstallForm
template_name = 'uninstall.html'
def __init__(self, **kwargs):
"""Initialize extra instance members."""
super().__init__(**kwargs)
self.app = None
def setup(self, request, *args, **kwargs):
"""Initialize attributes shared by all view methods."""
super().setup(request, *args, **kwargs)
app_id = self.kwargs['app_id']
self.app = app_module.App.get(app_id)
self.has_backup_restore = _has_backup_restore(self.app)
def dispatch(self, request, *args, **kwargs):
"""Don't allow the view to be used on essential apps."""
app_id = self.kwargs['app_id']
app = app_module.App.get(app_id)
if app.info.is_essential:
if self.app.info.is_essential:
raise Http404
return super().dispatch(request, *args, **kwargs)
def get_form_kwargs(self):
"""Return the keyword arguments for instantiating the form."""
kwargs = super().get_form_kwargs()
kwargs['has_backup_restore'] = self.has_backup_restore
return kwargs
def get_context_data(self, *args, **kwargs):
"""Add app information to the context data."""
context = super().get_context_data(*args, **kwargs)
app_id = self.kwargs['app_id']
app = app_module.App.get(app_id)
context['app_info'] = app.info
context['app_info'] = self.app.info
return context
def get_success_url(self):
@ -423,10 +442,8 @@ class UninstallView(FormView):
def form_valid(self, form):
"""Uninstall the app."""
app_id = self.kwargs['app_id']
# Backup the app
if form.cleaned_data['should_backup']:
if self.has_backup_restore and form.cleaned_data['should_backup']:
repository_id = form.cleaned_data['repository']
import plinth.modules.backups.repository as repository_module
@ -436,11 +453,12 @@ class UninstallView(FormView):
name = datetime.datetime.now().strftime(
'%Y-%m-%d:%H:%M:%S') + ' ' + str(
_('before uninstall of {app_id}')).format(app_id=app_id)
repository.create_archive(name, [app_id])
_('before uninstall of {app_id}')).format(
app_id=self.app.app_id)
repository.create_archive(name, [self.app.app_id])
# Uninstall
setup.run_uninstall_on_app(app_id)
setup.run_uninstall_on_app(self.app.app_id)
return super().form_valid(form)