diff --git a/plinth/modules/samba/__init__.py b/plinth/modules/samba/__init__.py index 9f1fe5e48..118aec07f 100644 --- a/plinth/modules/samba/__init__.py +++ b/plinth/modules/samba/__init__.py @@ -5,7 +5,6 @@ FreedomBox app to configure samba. import grp import json -import os import pwd import socket @@ -151,15 +150,6 @@ def get_shares(): return json.loads(output) -def disk_name(mount_point): - """Get a disk name.""" - share_name = os.path.basename(mount_point) - if not share_name: - share_name = 'disk' - - return share_name - - def backup_pre(packet): """Save registry share configuration.""" actions.superuser_run('samba', ['dump-shares']) diff --git a/plinth/modules/samba/templates/samba.html b/plinth/modules/samba/templates/samba.html index f4b822a7d..2bad8cfee 100644 --- a/plinth/modules/samba/templates/samba.html +++ b/plinth/modules/samba/templates/samba.html @@ -12,11 +12,19 @@ {% endblock %} - {% block configuration %} {{ block.super }} @@ -28,84 +36,112 @@ not the whole disk. {% endblocktrans %}

- - - - - - - - - - {% for disk in disks %} - - - - - - {% endfor %} - -
{% trans "Disk Name" %}{% trans "Shares" %}{% trans "Used" %}
{{ disk.name|default_if_none:"" }} -
- {% csrf_token %} - - {% for share_type in share_types %} - - {% endfor %} -
-
+ {% for disk in disks %} +
+
+
+ + {{ disk.name }} +
+
- {% if disk.percent_used < 75 %} -
- {{ disk.percent_used }}% -
+
+ {{ disk.percent_used }}% +
{{ disk.used_str }} / {{ disk.size_str }}
-
+ + -

+

+ {% csrf_token %} + + + + + + + + + + + {% for share_type in share_types %} + + + + + + {% endfor %} + +
{% trans 'Type' %}{% trans 'Name' %}{% trans 'Status' %}
+
+ + {% endfor %} + +

{% url 'storage:index' as storage_url %} {% url 'users:index' as users_url %} {% blocktrans trimmed %} You can find additional information about disks on the storage module page and configure - access to the shares on the users module page. - {% endblocktrans %}

+ access to the shares on the users module + page. + {% endblocktrans %} +

-

{% trans "Users who can currently access group and home shares" %}: - {{ users.access_ok|join:", " }}

+

{% trans "Users who can currently access group and home shares" %}: + {{ users.access_ok|join:", " }}

- {% if users.password_re_enter_needed %} -

{% trans "Users needing to re-enter their password on the password change page to access group and home shares" %}: - {{ users.password_re_enter_needed|join:", " }}.

- {% endif %} + {% if users.password_re_enter_needed %} +

{% trans "Users needing to re-enter their password on the password change page to access group and home shares" %}: + {{ users.password_re_enter_needed|join:", " }}.

+ {% endif %} {% if unavailable_shares %}

{% trans "Unavailable Shares" %}

- {% blocktrans trimmed %} - Shares that are configured but the disk is not available. If the disk - is plugged back in, sharing will be automatically enabled. - {% endblocktrans %} + {% blocktrans trimmed %} + Shares that are configured but the disk is not available. If the disk + is plugged back in, sharing will be automatically enabled. + {% endblocktrans %}

diff --git a/plinth/modules/samba/tests/test_functional.py b/plinth/modules/samba/tests/test_functional.py index 5b2319fb0..260cf5d64 100644 --- a/plinth/modules/samba/tests/test_functional.py +++ b/plinth/modules/samba/tests/test_functional.py @@ -48,17 +48,16 @@ def samba_share_should_not_be_available(share_type): def _set_share(browser, share_type, status='enabled'): """Enable or disable samba share.""" disk_name = 'disk' - share_type_name = '{0}_share'.format(share_type) + share_row_id = 'samba-share-{0}-{1}'.format(disk_name, share_type) + functional.nav_to_module(browser, 'samba') - for elem in browser.find_by_tag('td'): - if elem.text == disk_name: - share_form = elem.find_by_xpath('(..//*)[2]/form').first - share_btn = share_form.find_by_name(share_type_name).first - if status == 'enabled' and share_btn['value'] == 'enable': - share_btn.click() - elif status == 'disabled' and share_btn['value'] == 'disable': - share_btn.click() - break + share = browser.find_by_id(share_row_id) + share_btn = share.find_by_css('.share-status').find_by_tag('button').first + + if status == 'enabled' and share_btn['value'] == 'enable': + share_btn.click() + elif status == 'disabled' and share_btn['value'] == 'disable': + share_btn.click() def _write_to_share(share_type, as_guest=False): diff --git a/plinth/modules/samba/views.py b/plinth/modules/samba/views.py index a1b4786ed..cc177ae33 100644 --- a/plinth/modules/samba/views.py +++ b/plinth/modules/samba/views.py @@ -4,6 +4,7 @@ Views for samba module. """ import logging +import os import urllib.parse from collections import defaultdict @@ -20,12 +21,19 @@ logger = logging.getLogger(__name__) def get_share_mounts(): - """Return list of mount points.""" - ignore_points = ('/boot', '/boot/efi', '/boot/firmware', '/.snapshots') - return [ - mount for mount in storage.get_mounts() - if mount['mount_point'] not in ignore_points - ] + """Return list of shareable mounts.""" + ignore_mounts = ('/boot', '/boot/efi', '/boot/firmware', '/.snapshots') + mounts = [] + + for mount in storage.get_mounts(): + mount_point = mount['mount_point'] + if mount_point not in ignore_mounts: + basename = os.path.basename(mount_point) + mount['name'] = basename or _('FreedomBox OS disk') + mount['share_name_prefix'] = basename or 'disk' + mounts.append(mount) + + return sorted(mounts, key=lambda k: k['mount_point']) class SambaAppView(views.AppView): @@ -37,20 +45,27 @@ class SambaAppView(views.AppView): """Return template context data.""" context = super().get_context_data(*args, **kwargs) disks = get_share_mounts() - shares = samba.get_shares() - - for disk in disks: - disk['name'] = samba.disk_name(disk['mount_point']) context['disks'] = disks + shares = samba.get_shares() shared_mounts = defaultdict(list) for share in shares: shared_mounts[share['mount_point']].append(share['share_type']) context['shared_mounts'] = shared_mounts - context['share_types'] = [('open', _('Open Share')), - ('group', _('Group Share')), - ('home', _('Home Share'))] + context['share_types'] = [{ + 'id': 'open', + 'type': _('Open Share'), + 'share_name_suffix': '' + }, { + 'id': 'group', + 'type': _('Group Share'), + 'share_name_suffix': '_group' + }, { + 'id': 'home', + 'type': _('Home Share'), + 'share_name_suffix': '_home' + }] unavailable_shares = [] for share in shares: