Skip to content

Commit

Permalink
Simplify and fix (?) prefs handling for the Advanced Browse Modes
Browse files Browse the repository at this point in the history 
* fix variable naming: `$serverPrefs` isn't a good name for client specific preferences - and was overwritten in some places by the actual server prefs
* move client specific prefs saving to its own method
* remove custom browse mode configuration from the client prefs page: configure server wide, enable/disable per client

@darrel-k - please test the custom role stuff. Your commit actually caused to show the poor choice of variable names on my end: it broke the settings page (my fault!). The naming confusion should hopefully be fixed now.
  • Loading branch information
@michaelherger
michaelherger committed Aug 15, 2024
1 parent 2c9d40f commit 6661014
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 36 deletions.
14 changes: 8 additions & 6 deletions .../Plugin/ExtendedBrowseModes/HTML/EN/plugins/ExtendedBrowseModes/settings/browsemodes.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,26 @@
[% ELSE %]
<th></th>
<th width="20%">[% "NAME" | string %]</th>
[% END %]
<th>[% "PLUGIN_EXTENDED_BROWSEMODES_APPLY_TO" | string %]</th>
<th>[% "PLUGIN_EXTENDED_BROWSEMODES_BROWSE_BY" | string %]</th>
<th width="20%">[% "PLUGIN_EXTENDED_BROWSEMODES_ROLES" | string %]</th>
<th width="20%">[% "PLUGIN_EXTENDED_BROWSEMODES_RELEASE_TYPE" | string %]</th>
<th width="20%">[% "GENRES" | string %][% "COLON" | string %]</th>
<th>[% IF menu.id != '_new_' %][% "DELETE" | string %][% "COLON" | string %][% END %]</th>
[% END %]
</tr>
[% seen = 1; END %]
<tr>
<td style="text-align:center;">
[% IF needsClient && menu.name %]<input type="checkbox" name="enabled[% menu_no %]" id="enabled[% menu_no %]" [% IF menu.enabled %]checked="checked"[% END %] value="1" class="stdedit" />[% END %]
<input type="hidden" name="id[% menu_no %]" value="[% menu.id | html %]" />
</td>
[% IF !menu.feed && menu.id != '_new_' %]
[% IF (needsClient || !menu.feed) && menu.id != '_new_' %]
<td colspan="6"><label for="enabled[% menu_no %]">[% menu.name | getstring %]</label></td>
[% ELSE %]
[% ELSIF !needsClient %]
<td><input type="text" name="name[% menu_no %]" value="[% menu.name | html %]" style="width: 98%;" class="stdedit" /></td>
<td>
<select class="stdedit" name="libraryid[% menu_no %]" style="margin-top:0">
<select class="stdedit" name="libraryid[% menu_no %]" style="margin-top:0; max-width:200px">
<option value="">[% "SUB_LIBRARY" | string %]</option>
<option value="-1" [% IF menu.params.library_id == -1 %]selected[% END %]>[% "ALL_LIBRARY" | string %]</option>
[% FOREACH library = libraries %]
Expand All @@ -61,13 +61,15 @@
[% END %]
</tr>
[% END %]
[% IF !needsClient %]
<tr><td>&nbsp;</td><td colspan="5">[% "PLUGIN_EXTENDED_BROWSEMODES_CONTRIBUTOR_HINT" | string %] [% roles.join(', ') %]</td></tr>
<tr><td>&nbsp;</td><td colspan="5">[% "PLUGIN_EXTENDED_BROWSEMODES_RELEASE_TYPE_HINT" | string %] [% release_types.join(', ') %]</td></tr>
<tr><td>&nbsp;</td><td colspan="5">[% "GENRES" | string %][% "COLON" | string %] [% genre_list.join(', ') %]</td></tr>
[% END %]
</table>
[% END %]

[% WRAPPER setting title="PLUGIN_EXTENDED_BROWSEMODES_TAGS" desc="PLUGIN_EXTENDED_BROWSEMODES_TAGS_DESC" %]
[% IF !needsClient; WRAPPER setting title="PLUGIN_EXTENDED_BROWSEMODES_TAGS" desc="PLUGIN_EXTENDED_BROWSEMODES_TAGS_DESC" %]
<table>
<tr>
<th>[% "PLUGIN_EXTENDED_BROWSEMODES_ROLE" | string %]</th>
Expand All @@ -84,7 +86,7 @@
<td><input type="text" name="new_name" value="" class="stdedit" style="width: 200px;"/></td>
</tr>
</table>
[% END %]
[% END; END %]

[% IF needsClient %]
<input type="hidden" name="pref_enableLosslessPreferred" value="[% prefs.pref_enableLosslessPreferred || 0 %]" />
Expand Down
25 changes: 22 additions & 3 deletions Slim/Plugin/ExtendedBrowseModes/PlayerSettings.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ use base qw(Slim::Plugin::ExtendedBrowseModes::Settings);

use Slim::Utils::Prefs;

my $prefs = preferences('plugin.extendedbrowsemodes');
my $serverPrefs = preferences('server');

sub name {
return Slim::Web::HTTP::CSRF->protectName('PLUGIN_EXTENDED_BROWSEMODES');
}
Expand All @@ -21,9 +24,25 @@ sub page {

sub needsClient { 1 }

sub getServerPrefs {
my ($class, $client) = @_;
return preferences('server')->client($client);
sub handler {
my ($class, $client, $params) = @_;

my $clientPrefs = $serverPrefs->client($client);

if ($params->{'saveSettings'}) {
my $menus = $prefs->get('additionalMenuItems');

for (my $i = 1; defined $params->{"id$i"}; $i++) {
my ($menu) = $params->{"id$i"} eq '_new_' ? {} : grep { $_->{id} eq $params->{"id$i"} } @$menus;

if ($clientPrefs) {
$clientPrefs->set('disabled_' . $params->{"id$i"}, $params->{"enabled$i"} ? 0 : 1);
delete $menu->{enabled};
}
}
}

$class->SUPER::handler($client, $params);
}

1;
Expand Down
40 changes: 13 additions & 27 deletions Slim/Plugin/ExtendedBrowseModes/Settings.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use constant AUDIOBOOKS_MENUS => [{
}];

my $prefs = preferences('plugin.extendedbrowsemodes');
my $serverPrefs = preferences('server');

sub name {
return Slim::Web::HTTP::CSRF->protectName('PLUGIN_EXTENDED_BROWSEMODES');
Expand All @@ -50,15 +51,14 @@ sub page {
sub handler {
my ($class, $client, $params) = @_;

my $serverPrefs = $class->getServerPrefs($client);

if ($params->{'saveSettings'}) {

my $serverPrefs = preferences('server');
# if we're called from the client prefs, we've already saved the client prefs
if ($params->{'saveSettings'} && !$class->needsClient) {
# custom role handling
my $currentRoles = $serverPrefs->get('userDefinedRoles');
my $customTags = {};
my $id = 21;
my $changed = 0;

foreach my $pref (keys %{$params}) {
if ($pref =~ /(.*)_tag$/) {
my $key = $1;
Expand All @@ -80,6 +80,7 @@ sub handler {
}
}
}

foreach my $old (keys %{$currentRoles}) {
$changed = 1 if !$customTags->{$old};
}
Expand All @@ -88,13 +89,13 @@ sub handler {
$serverPrefs->set('userDefinedRoles', $customTags);
}

# browse menu handling
my $menus = $prefs->get('additionalMenuItems');

for (my $i = 1; defined $params->{"id$i"}; $i++) {

if ( $params->{"delete$i"} ) {
Slim::Menu::BrowseLibrary->deregisterNode($params->{"id$i"});
my $serverPrefs = preferences('server');

# remove prefs related to this menu item
foreach my $clientPref ( $serverPrefs->allClients ) {
Expand All @@ -108,12 +109,6 @@ sub handler {

my ($menu) = $params->{"id$i"} eq '_new_' ? {} : grep { $_->{id} eq $params->{"id$i"} } @$menus;

if ( $class->needsClient && $serverPrefs ) {
$serverPrefs->set('disabled_' . $params->{"id$i"}, $params->{"enabled$i"} ? 0 : 1);
}

delete $menu->{enabled} if $serverPrefs;

next unless $params->{"name$i"} && $params->{"feed$i"} && ($params->{"roleid$i"} || $params->{"releasetype$i"} || $params->{"genreid$i"} || $params->{"libraryid$i"});

if ( $params->{"id$i"} eq '_new_' ) {
Expand Down Expand Up @@ -151,9 +146,6 @@ sub handler {
$menu->{weight} = "15.$ts" * 1;
}

# need to migrate the enabled flag
my $serverPrefs = preferences('server');

# remove prefs related to this menu item
foreach my $clientPref ( $serverPrefs->allClients ) {
my $oldPref = $clientPref->get('disabled_' . $oldId);
Expand Down Expand Up @@ -214,22 +206,16 @@ sub handler {
$prefs->set('additionalMenuItems', $menus);
}

$params->{genre_list} = [ sort map { $_->name } Slim::Schema->search('Genre')->all ];
$params->{roles} = [ Slim::Schema::Contributor->contributorRoles ];
$params->{release_types} = Slim::Schema::Album->releaseTypes;

$class->SUPER::handler($client, $params);
}

sub getServerPrefs {}


sub beforeRender {
my ($class, $params, $client) = @_;

my $serverPrefs = preferences('server');
$params->{genre_list} = [ sort map { $_->name } Slim::Schema->search('Genre')->all ];
$params->{roles} = [ Slim::Schema::Contributor->contributorRoles ];
$params->{release_types} = Slim::Schema::Album->releaseTypes;
$params->{customTags} = $serverPrefs->get('userDefinedRoles');

$params->{libraries} = {};

if ($params->{'needsAudioBookUpdate'}) {
Expand All @@ -247,19 +233,19 @@ sub beforeRender {
$prefs->set('additionalMenuItems', $menus);
}

my $serverPrefs = $class->getServerPrefs($client);
my $clientPrefs = $serverPrefs->client($client) if $class->needsClient;

my %ids;
$params->{menu_items} = [ map {
$ids{$_->{id}}++;
$_->{enabled} = $serverPrefs ? ($serverPrefs->get('disabled_' . $_->{id}) ? 0 : 1) : 1;
$_->{enabled} = $clientPrefs ? ($clientPrefs->get('disabled_' . $_->{id}) ? 0 : 1) : 1;
$_;
} @{Storable::dclone($prefs->get('additionalMenuItems'))}, { id => '_new_' } ];

unshift @{$params->{menu_items}}, map { {
name => $_->{name},
id => $_->{id},
enabled => $serverPrefs ? ($serverPrefs->get('disabled_' . $_->{id}) ? 0 : 1) : 1,
enabled => $clientPrefs ? ($clientPrefs->get('disabled_' . $_->{id}) ? 0 : 1) : 1,
} } sort {
$a->{weight} <=> $b->{weight}
# don't allow to disable some select browse menus
Expand Down

0 comments on commit 6661014

Please sign in to comment.