From f457a0bdd3db4d5c845add408520a3cfa67e5ca8 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Sun, 13 Nov 2016 19:35:33 +0530 Subject: [PATCH] first_boot: Fix various issues and refactor - Fix major regression so that steps can submitted multiple times in case of errors. - Don't serve the welcome page (and other pages) only once. Show it until action is an taken. This does not apply to the final step. - Eliminate all coupling of one first boot step on another. - Move first boot helper methods to __init__.py instead of middleware as it is more generic than middleware. - Implement caching the first boot state to avoid an SQL query on every page load. The down side is that if first boot state is modified in the backend DB outside Plinth, Plinth will need to be restarted to catch the modified value. - Mark some methods as private. - Refactor middleware code for slightly more simplicity. - Don't show sidebar in pagekite first boot step. Set width like other pages. --- plinth/modules/first_boot/__init__.py | 91 +++++++++++++++ plinth/modules/first_boot/middleware.py | 104 ++++++------------ .../templates/firstboot_welcome.html | 12 +- plinth/modules/first_boot/urls.py | 4 +- plinth/modules/first_boot/views.py | 39 ++++--- .../templates/pagekite_firstboot.html | 68 ++++++------ plinth/modules/pagekite/urls.py | 4 +- plinth/modules/pagekite/views.py | 37 ++++--- plinth/modules/users/views.py | 25 +++-- 9 files changed, 227 insertions(+), 157 deletions(-) diff --git a/plinth/modules/first_boot/__init__.py b/plinth/modules/first_boot/__init__.py index 3653b5ce2..b810cca88 100644 --- a/plinth/modules/first_boot/__init__.py +++ b/plinth/modules/first_boot/__init__.py @@ -19,6 +19,11 @@ Plinth module for first boot wizard """ +from django.urls import reverse +import operator + +from plinth import module_loader + version = 1 is_essential = True @@ -35,3 +40,89 @@ first_boot_steps = [ 'order': 10 } ] + +_all_first_boot_steps = None + +_is_completed = None + + +def is_firstboot_url(path): + """Return whether a path is a firstboot step URL. + + :param path: path of url to be checked + :return: true if its a first boot URL false otherwise + """ + for step in _get_steps(): + if path.startswith(reverse(step['url'])): + return True + + return False + + +def _get_steps(): + """Return list of all firstboot steps.""" + global _all_first_boot_steps + if _all_first_boot_steps is not None: + return _all_first_boot_steps + + steps = [] + modules = module_loader.loaded_modules + for module_object in modules.values(): + if getattr(module_object, 'first_boot_steps', None): + steps.extend(module_object.first_boot_steps) + + _all_first_boot_steps = sorted(steps, key=operator.itemgetter('order')) + return _all_first_boot_steps + + +def next_step(): + """Return the resolved next first boot step URL required to go to. + + If there are no more step remaining, return index page. + """ + return next_step_or_none() or 'index' + + +def next_step_or_none(): + """Return the next first boot step required to run. + + If there are no more step remaining, return None. + """ + from plinth import kvstore + + for step in _get_steps(): + done = kvstore.get_default(step['id'], 0) + if not done: + return step.get('url') + + +def mark_step_done(id): + """Marks the status of a first boot step as done. + + :param id: id of the firstboot step + """ + from plinth import kvstore + + kvstore.set(id, 1) + if not next_step_or_none(): + kvstore.set('setup_state', 1) + + +def is_completed(): + """Return whether first boot process is completed.""" + from plinth import kvstore + + global _is_completed + if _is_completed is None: + _is_completed = kvstore.get_default('setup_state', 0) + + return _is_completed + + +def set_completed(): + """Set the first boot process as completed.""" + from plinth import kvstore + + global _is_completed + _is_completed = True + kvstore.set('setup_state', 1) diff --git a/plinth/modules/first_boot/middleware.py b/plinth/modules/first_boot/middleware.py index d56a9579a..625a6315a 100644 --- a/plinth/modules/first_boot/middleware.py +++ b/plinth/modules/first_boot/middleware.py @@ -24,13 +24,12 @@ from django.http.response import HttpResponseRedirect from django.urls import reverse from django.conf import settings import logging -from operator import itemgetter -from plinth import kvstore, module_loader + +from plinth import kvstore +from plinth.modules import first_boot LOGGER = logging.getLogger(__name__) -firstboot_steps = [] - class FirstBootMiddleware(object): """Forward to firstboot page if firstboot isn't finished yet.""" @@ -38,80 +37,39 @@ class FirstBootMiddleware(object): @staticmethod def process_request(request): """Handle a request as Django middleware request handler.""" - old_state = kvstore.get_default('firstboot_state', 0) - state = kvstore.get_default('setup_state', 0) - if state == 0 and old_state == 10: - state = 1 - kvstore.set('setup_state', 1) - - user_requests_firstboot = is_firstboot(request.path) + # Don't interfere with login page user_requests_login = request.path.startswith( reverse(settings.LOGIN_URL)) - help_index_url = reverse('help:index') - user_requests_help = request.path.startswith(help_index_url) - if not user_requests_login and not user_requests_help: - if state == 1 and user_requests_firstboot: - return HttpResponseRedirect(reverse('index')) - elif state == 0 and not user_requests_firstboot: - url = next_step() - return HttpResponseRedirect(reverse(url)) + if user_requests_login: + return + # Don't interfere with help pages + user_requests_help = request.path.startswith(reverse('help:index')) + if user_requests_help: + return -def is_firstboot(path): - """ - Returns whether the path is a firstboot step url - :param path: path of current url - :return: true if its a first boot url false otherwise - """ - steps = get_firstboot_steps() - for step in steps: - if reverse(step.get('url')) == path: - return True + state = first_boot.is_completed() - return False + # Migrate from old settings variable + if state == 0: + old_state = kvstore.get_default('firstboot_state', 0) + if old_state == 10: + state = 1 + first_boot.set_completed() + user_requests_firstboot = first_boot.is_firstboot_url(request.path) -def get_firstboot_steps(): - """Returns all firstboot steps""" - steps = [] - modules = module_loader.loaded_modules - for (module_name, module_object) in modules.items(): - if getattr(module_object, 'first_boot_steps', None): - for step in module_object.first_boot_steps: - steps.append(step) + # Redirect to first boot if requesting normal page and first + # boot is not complete. + if state == 0 and not user_requests_firstboot: + next_step = first_boot.next_step_or_none() + if next_step: + return HttpResponseRedirect(reverse(next_step)) + else: + # No more steps in first boot + first_boot.set_completed() - steps = sorted(steps, key=itemgetter('order')) - return steps - - -def next_step(): - """ Returns the next first boot step required to run """ - global firstboot_steps - if len(firstboot_steps) == 0: - firstboot_steps = get_firstboot_steps() - - for step in firstboot_steps: - done = kvstore.get_default(step.get('id'), 0) - if done == 0: - return step.get('url') - - -def mark_step_done(id): - """ - Marks the status of a first boot step is done - :param id: id of the firstboot step - """ - kvstore.set(id, 1) - global firstboot_steps - if len(firstboot_steps) == 0: - firstboot_steps = get_firstboot_steps() - - setup_done = True - for step in firstboot_steps: - done = kvstore.get_default(step.get('id'), 0) - if done == 0: - setup_done = False - break - - if setup_done: - kvstore.set('setup_state', 1) + # Redirect to index page if request firstboot after it is + # finished. + if state == 1 and user_requests_firstboot: + return HttpResponseRedirect(reverse('index')) diff --git a/plinth/modules/first_boot/templates/firstboot_welcome.html b/plinth/modules/first_boot/templates/firstboot_welcome.html index ea4951094..a98184b5b 100644 --- a/plinth/modules/first_boot/templates/firstboot_welcome.html +++ b/plinth/modules/first_boot/templates/firstboot_welcome.html @@ -36,10 +36,14 @@ alt="{{ box_name }}" width="640"/>

-

- {% trans "Start Setup" %} -

+
+ {% csrf_token %} + +

+ +

+

{% blocktrans trimmed %} diff --git a/plinth/modules/first_boot/urls.py b/plinth/modules/first_boot/urls.py index 93fe8364f..bf1954307 100644 --- a/plinth/modules/first_boot/urls.py +++ b/plinth/modules/first_boot/urls.py @@ -22,7 +22,7 @@ URLs for the First Boot module from django.conf.urls import url from stronghold.decorators import public -from .views import WelcomeView, complete +from .views import WelcomeView, CompleteView urlpatterns = [ @@ -30,5 +30,5 @@ urlpatterns = [ url(r'^firstboot/$', public(WelcomeView.as_view()), name='index'), url(r'^firstboot/welcome/$', public(WelcomeView.as_view()), name='welcome'), - url(r'^firstboot/complete/$', complete, name='complete'), + url(r'^firstboot/complete/$', CompleteView.as_view(), name='complete'), ] diff --git a/plinth/modules/first_boot/views.py b/plinth/modules/first_boot/views.py index 1aae217c4..6d722a817 100644 --- a/plinth/modules/first_boot/views.py +++ b/plinth/modules/first_boot/views.py @@ -15,12 +15,13 @@ # along with this program. If not, see . # -from django.contrib.auth.models import User -from django.shortcuts import render +from django import http +from django.urls import reverse from django.utils.translation import ugettext as _ from django.views.generic import TemplateView + from plinth import network -from .middleware import mark_step_done, next_step +from plinth.modules import first_boot class WelcomeView(TemplateView): @@ -28,25 +29,29 @@ class WelcomeView(TemplateView): template_name = 'firstboot_welcome.html' - def get_context_data(self, **kwargs): - """Returns the context data for the template.""" - context = super(WelcomeView, self).get_context_data(**kwargs) - mark_step_done('firstboot_welcome') - context['next_url'] = next_step() - return context + def post(self, request, *args, **kwargs): + """On POST, mark this step as done and move to next step.""" + first_boot.mark_step_done('firstboot_welcome') + return http.HttpResponseRedirect(reverse(first_boot.next_step())) -def complete(request): +class CompleteView(TemplateView): """Show summary after all firstboot setup is done. After viewing this page the firstboot module can't be accessed anymore. """ - # Make sure that a user exists before finishing firstboot - if User.objects.all(): - mark_step_done('firstboot_complete') - connections = network.get_connection_list() + template_name = 'firstboot_complete.html' - return render(request, 'firstboot_complete.html', - {'title': _('Setup Complete'), - 'connections': connections}) + def get(self, request, *args, **kwargs): + """Mark as done as soon as page is served.""" + response = super().get(self, request, *args, **kwargs) + first_boot.mark_step_done('firstboot_complete') + return response + + def get_context_data(self, **kwargs): + """Add network connections to context list.""" + context = super().get_context_data(**kwargs) + context['connections'] = network.get_connection_list() + context['title'] = _('Setup Complete') + return context diff --git a/plinth/modules/pagekite/templates/pagekite_firstboot.html b/plinth/modules/pagekite/templates/pagekite_firstboot.html index 7c34c7a89..79f5eddbf 100644 --- a/plinth/modules/pagekite/templates/pagekite_firstboot.html +++ b/plinth/modules/pagekite/templates/pagekite_firstboot.html @@ -22,46 +22,44 @@ {% load i18n %} {% load static %} -{% block content %} +{% block content_row %} -

{% trans "Setup a freedombox.me subdomain with your voucher" %}

+
+

{% trans "Setup a freedombox.me subdomain with your voucher" %}

-

- {% url 'first_boot:complete' as finish_firstboot_url %} - {% blocktrans trimmed %} - Skip this step if you - do not have a voucher or want to configure PageKite later with a - different domain or credentials. - {% endblocktrans %} -

+

+ {% url 'pagekite:firstboot-skip' as finish_firstboot_url %} + {% blocktrans trimmed %} + Skip this step if you + do not have a voucher or want to configure PageKite later with a + different domain or credentials. + {% endblocktrans %} +

-

- {% blocktrans trimmed %} - You can use an already redeemed voucher but it will only work - with the initially registered subdomain. - {% endblocktrans %} -

+

+ {% blocktrans trimmed %} + You can use an already redeemed voucher but it will only work + with the initially registered subdomain. + {% endblocktrans %} +

-
-
-
- {% csrf_token %} + + {% csrf_token %} - {{ form|bootstrap_horizontal:'col-lg-3' }} -
-
- - - {% trans "Skip Registration" %} - -
-
-
-
+ {{ form|bootstrap_horizontal:'col-lg-3' }} +
+
+ + + {% trans "Skip Registration" %} + +
+
+
{% endblock %} diff --git a/plinth/modules/pagekite/urls.py b/plinth/modules/pagekite/urls.py index 82f00cab0..a97cdc98f 100644 --- a/plinth/modules/pagekite/urls.py +++ b/plinth/modules/pagekite/urls.py @@ -22,7 +22,7 @@ URLs for the PageKite module from django.conf.urls import url from .views import StandardServiceView, CustomServiceView, ConfigurationView, \ - DeleteServiceView, index, FirstBootView + DeleteServiceView, index, FirstBootView, first_boot_skip urlpatterns = [ url(r'^sys/pagekite/$', index, name='index'), @@ -36,4 +36,6 @@ urlpatterns = [ name='delete-custom-service'), url(r'^sys/pagekite/firstboot/$', FirstBootView.as_view(), name='firstboot'), + url(r'^sys/pagekite/firstboot/skip/$', first_boot_skip, + name='firstboot-skip'), ] diff --git a/plinth/modules/pagekite/views.py b/plinth/modules/pagekite/views.py index c6ec4cb8e..73b6da663 100644 --- a/plinth/modules/pagekite/views.py +++ b/plinth/modules/pagekite/views.py @@ -16,7 +16,7 @@ # from django.contrib import messages -from django.http.response import HttpResponseRedirect +from django.http import HttpResponseRedirect from django.template.response import TemplateResponse from django.urls import reverse, reverse_lazy from django.utils.translation import ugettext_lazy as _ @@ -26,9 +26,10 @@ from django.views.generic.edit import FormView from . import utils from .forms import ConfigurationForm, StandardServiceForm, \ AddCustomServiceForm, DeleteCustomServiceForm, FirstBootForm +from plinth import cfg from plinth.errors import DomainRegistrationError +from plinth.modules import first_boot from plinth.modules import pagekite -from plinth.modules.first_boot.middleware import mark_step_done subsubmenu = [{'url': reverse_lazy('pagekite:index'), 'text': _('About PageKite')}, @@ -139,10 +140,13 @@ class FirstBootView(FormView): template_name = 'pagekite_firstboot.html' form_class = FirstBootForm - def get(self, *args, **kwargs): - """Respond to GET request.""" - mark_step_done('pagekite_firstboot') - return super(FirstBootView, self).get(*args, **kwargs) + def get(self, request, *args, **kwargs): + """Skip if this first boot step if it is not relavent.""" + if not cfg.danube_edition: + first_boot.mark_step_done('pagekite_firstboot') + return HttpResponseRedirect(reverse(first_boot.next_step())) + + return super().get(request, *args, **kwargs) def form_valid(self, form): """Act on valid form submission.""" @@ -150,10 +154,17 @@ class FirstBootView(FormView): form.register_domain() except DomainRegistrationError as error: messages.error(self.request, error) - return HttpResponseRedirect(reverse_lazy('pagekite:firstboot')) - else: - form.setup_pagekite() - message = _('Pagekite setup finished. The HTTP and HTTPS services ' - 'are activated now.') - messages.success(self.request, message) - return super(FirstBootView, self).form_valid(form) + return self.form_invalid(form) + + form.setup_pagekite() + first_boot.mark_step_done('pagekite_firstboot') + message = _('Pagekite setup finished. The HTTP and HTTPS services ' + 'are activated now.') + messages.success(self.request, message) + return HttpResponseRedirect(reverse(first_boot.next_step())) + + +def first_boot_skip(request): + """Skip the first boot step.""" + first_boot.mark_step_done('pagekite_firstboot') + return HttpResponseRedirect(reverse(first_boot.next_step())) diff --git a/plinth/modules/users/views.py b/plinth/modules/users/views.py index b5edea64d..dfe92b90c 100644 --- a/plinth/modules/users/views.py +++ b/plinth/modules/users/views.py @@ -28,9 +28,8 @@ from django.utils.translation import ugettext as _, ugettext_lazy from .forms import CreateUserForm, UserChangePasswordForm, UserUpdateForm, \ FirstBootForm from plinth import actions -from plinth import cfg from plinth.errors import ActionError -from plinth.modules.first_boot.middleware import mark_step_done, next_step +from plinth.modules import first_boot subsubmenu = [{'url': reverse_lazy('users:index'), 'text': ugettext_lazy('Users')}, @@ -171,20 +170,22 @@ class UserChangePassword(ContextMixin, SuccessMessageMixin, FormView): class FirstBootView(django.views.generic.CreateView): """Create user account and log the user in.""" template_name = 'users_firstboot.html' + form_class = FirstBootForm - success_url = '' - - def __init__(self, *args, **kwargs): - """Initialize the view object.""" - if not cfg.danube_edition: - mark_step_done('pagekite_firstboot') - - mark_step_done('users_firstboot') - self.success_url = next_step() - return super(FirstBootView, self).__init__(*args, **kwargs) def get_form_kwargs(self): """Make request available to the form (to insert messages)""" kwargs = super(FirstBootView, self).get_form_kwargs() kwargs['request'] = self.request return kwargs + + def form_valid(self, form): + """Mark this first boot step as completed and save form.""" + if User.objects.all(): + first_boot.mark_step_done('users_firstboot') + + return super().form_valid(form) + + def get_success_url(self): + """Return the next first boot step after valid form submission.""" + return reverse(first_boot.next_step())