]> git.mxchange.org Git - friendica.git/commitdiff
Apply suggestions from code review
authorPhilipp <admin+Github@philipp.info>
Sun, 26 Mar 2023 22:17:40 +0000 (00:17 +0200)
committerPhilipp <admin@philipp.info>
Mon, 27 Mar 2023 17:36:14 +0000 (19:36 +0200)
Switch to `isWritable`

Co-authored-by: Hypolite Petovan <hypolite@mrpetovan.com>
src/Core/Config/Capability/IManageConfigValues.php
src/Core/Config/Model/DatabaseConfig.php
src/Core/Config/Model/ReadOnlyFileConfig.php
src/Module/Admin/Logs/Settings.php
src/Module/Admin/Site.php
src/Module/Admin/Storage.php
tests/src/Core/Config/ConfigTest.php
view/templates/admin/storage.tpl
view/theme/frio/templates/admin/storage.tpl

index 93e0143b0a7da7ef99977ef8e4db18efa8eb26b7..964087ca9565763b52a5f5355e88c7443b4c950a 100644 (file)
@@ -66,7 +66,7 @@ interface IManageConfigValues
         *
         * @return bool true, if set is disabled
         */
-       public function isSetDisabled(string $cat, string $key): bool;
+       public function isWritable(string $cat, string $key): bool;
 
        /**
         * Sets a configuration value for system config
index 058c90359944c313cda98f73b8c2577ef98e3d68..7b34eda975753e70e363149ec25fb4355a47a169 100644 (file)
@@ -81,9 +81,9 @@ class DatabaseConfig implements IManageConfigValues
        }
 
        /** {@inheritDoc} */
-       public function isSetDisabled(string $cat, string $key): bool
+       public function isWritable(string $cat, string $key): bool
        {
-               return $this->cache->getSource($cat, $key) >= Cache::SOURCE_ENV;
+               return $this->cache->getSource($cat, $key) < Cache::SOURCE_ENV;
        }
 
        /** {@inheritDoc} */
index f0a25fcf401361650cd14a514842acdc3c965e30..cc84a6c0d35c884469800e908125e72808ec2fca 100644 (file)
@@ -69,9 +69,9 @@ class ReadOnlyFileConfig implements IManageConfigValues
        }
 
        /** {@inheritDoc} */
-       public function isSetDisabled(string $cat, string $key): bool
+       public function isWritable(string $cat, string $key): bool
        {
-               return $this->configCache->getSource($cat, $key) >= Cache::SOURCE_ENV;
+               return $this->configCache->getSource($cat, $key) < Cache::SOURCE_ENV;
        }
 
        /** {@inheritDoc} */
index 16120717625d9b38b3dd8eed3d99746c4866ffb1..54f929f090f0efef6edadc5a4b41714031c30f41 100644 (file)
@@ -48,13 +48,13 @@ class Settings extends BaseAdmin
                        return;
                }
 
-               if (!DI::config()->isSetDisabled('system', 'logfile')) {
+               if (DI::config()->isWritable('system', 'logfile')) {
                        DI::config()->set('system', 'logfile', $logfile);
                }
-               if (!DI::config()->isSetDisabled('system', 'debugging')) {
+               if (DI::config()->isWritable('system', 'debugging')) {
                        DI::config()->set('system', 'debugging', $debugging);
                }
-               if (!DI::config()->isSetDisabled('system', 'loglevel')) {
+               if (DI::config()->isWritable('system', 'loglevel')) {
                        DI::config()->set('system', 'loglevel', $loglevel);
                }
 
@@ -88,9 +88,9 @@ class Settings extends BaseAdmin
                        '$clear' => DI::l10n()->t('Clear'),
                        '$logname' => DI::config()->get('system', 'logfile'),
                        // see /help/smarty3-templates#1_1 on any Friendica node
-                       '$debugging' => ['debugging', DI::l10n()->t("Enable Debugging"), DI::config()->get('system', 'debugging'), DI::config()->isSetDisabled('system', 'debugging') ? DI::l10n()->t("<strong>Readonly</strong> because set by an environment variable") : '', DI::config()->isSetDisabled('system', 'debugging') ? 'disabled' : ''],
-                       '$logfile' => ['logfile', DI::l10n()->t("Log file"), DI::config()->get('system', 'logfile'), DI::l10n()->t("Must be writable by web server. Relative to your Friendica top-level directory.") . (DI::config()->isSetDisabled('system', 'logfile') ? '<br>' . DI::l10n()->t("<strong>Readonly</strong> because set by an environment variable") : ''), "", DI::config()->isSetDisabled('system', 'logfile') ? 'disabled' : ''],
-                       '$loglevel' => ['loglevel', DI::l10n()->t("Log level"), DI::config()->get('system', 'loglevel'), DI::config()->isSetDisabled('system', 'loglevel') ? DI::l10n()->t("<strong>Readonly</strong> because set by an environment variable") : '', $log_choices, DI::config()->isSetDisabled('system', 'loglevel') ? 'disabled' : ''],
+                       '$debugging' => ['debugging', DI::l10n()->t('Enable Debugging'), DI::config()->get('system', 'debugging'), !DI::config()->isWritable('system', 'debugging') ? DI::l10n()->t('<strong>Read-only</strong> because it is set by an environment variable') : '', !DI::config()->isWritable('system', 'debugging') ? 'disabled' : ''],
+                       '$logfile' => ['logfile', DI::l10n()->t('Log file'), DI::config()->get('system', 'logfile'), DI::l10n()->t('Must be writable by web server. Relative to your Friendica top-level directory.') . (!DI::config()->isWritable('system', 'logfile') ? '<br>' . DI::l10n()->t('<strong>Read-only</strong> because it is set by an environment variable') : ''), '', !DI::config()->isWritable('system', 'logfile') ? 'disabled' : ''],
+                       '$loglevel' => ['loglevel', DI::l10n()->t("Log level"), DI::config()->get('system', 'loglevel'), !DI::config()->isWritable('system', 'loglevel') ? DI::l10n()->t('<strong>Read-only</strong> because it is set by an environment variable') : '', $log_choices, !DI::config()->isWritable('system', 'loglevel') ? 'disabled' : ''],
                        '$form_security_token' => self::getFormSecurityToken("admin_logs"),
                        '$phpheader' => DI::l10n()->t("PHP logging"),
                        '$phphint' => DI::l10n()->t("To temporarily enable logging of PHP errors and warnings you can prepend the following to the index.php file of your installation. The filename set in the 'error_log' line is relative to the friendica top-level directory and must be writeable by the web server. The option '1' for 'log_errors' and 'display_errors' is to enable these options, set to '0' to disable them."),
index 07aa79422a03054a3d136f54a5b3866bca587e27..89f55d6f17c8a52df31cefcb43653efa69aab71e 100644 (file)
@@ -165,8 +165,8 @@ class Site extends BaseAdmin
                $transactionConfig->set('system', 'poco_discovery'        , $poco_discovery);
                $transactionConfig->set('system', 'poco_local_search'     , $poco_local_search);
                $transactionConfig->set('system', 'nodeinfo'              , $nodeinfo);
-               if (!DI::config()->isSetDisabled('config', 'sitename')) {
-                       $transactionConfig->set('config', 'sitename'          , $sitename);
+               if (DI::config()->isWritable('config', 'sitename')) {
+                       $transactionConfig->set('config', 'sitename', $sitename);
                }
                $transactionConfig->set('config', 'sender_email'          , $sender_email);
                $transactionConfig->set('system', 'suppress_tags'         , $suppress_tags);
@@ -190,7 +190,7 @@ class Site extends BaseAdmin
                } else {
                        $transactionConfig->set('config', 'info', $additional_info);
                }
-               if (!DI::config()->isSetDisabled('system', 'language')) {
+               if (DI::config()->isWritable('system', 'language')) {
                        $transactionConfig->set('system', 'language', $language);
                }
                $transactionConfig->set('system', 'theme', $theme);
@@ -417,7 +417,7 @@ class Site extends BaseAdmin
                        '$relocate_cmd'      => DI::l10n()->t('(Friendica directory)# bin/console relocate https://newdomain.com'),
 
                        // name, label, value, help string, extra data...
-                       '$sitename'         => ['sitename', DI::l10n()->t('Site name'), DI::config()->get('config', 'sitename'), DI::config()->isSetDisabled('config', 'sitename') ? DI::l10n()->t("<strong>Readonly</strong> because set by an environment variable") : '', '', DI::config()->isSetDisabled('config', 'sitename') ? 'disabled' : ''],
+                       '$sitename'         => ['sitename', DI::l10n()->t('Site name'), DI::config()->get('config', 'sitename'), !DI::config()->isWritable('config', 'sitename') ? DI::l10n()->t('<strong>Read-only</strong> because it is set by an environment variable') : '', '', !DI::config()->isWritable('config', 'sitename') ? 'disabled' : ''],
                        '$sender_email'     => ['sender_email', DI::l10n()->t('Sender Email'), DI::config()->get('config', 'sender_email'), DI::l10n()->t('The email address your server shall use to send notification emails from.'), '', '', 'email'],
                        '$system_actor_name' => ['system_actor_name', DI::l10n()->t('Name of the system actor'), User::getActorName(), DI::l10n()->t("Name of the internal system account that is used to perform ActivityPub requests. This must be an unused username. If set, this can't be changed again.")],
                        '$banner'           => ['banner', DI::l10n()->t('Banner/Logo'), $banner, ''],
@@ -425,7 +425,7 @@ class Site extends BaseAdmin
                        '$shortcut_icon'    => ['shortcut_icon', DI::l10n()->t('Shortcut icon'), DI::config()->get('system', 'shortcut_icon'), DI::l10n()->t('Link to an icon that will be used for browsers.')],
                        '$touch_icon'       => ['touch_icon', DI::l10n()->t('Touch icon'), DI::config()->get('system', 'touch_icon'), DI::l10n()->t('Link to an icon that will be used for tablets and mobiles.')],
                        '$additional_info'  => ['additional_info', DI::l10n()->t('Additional Info'), $additional_info, DI::l10n()->t('For public servers: you can add additional information here that will be listed at %s/servers.', Search::getGlobalDirectory())],
-                       '$language'         => ['language', DI::l10n()->t('System language'), DI::config()->get('system', 'language'), DI::config()->isSetDisabled('system', 'language') ? DI::l10n()->t("<strong>Readonly</strong> because set by an environment variable") : '', $lang_choices, DI::config()->isSetDisabled('system', 'language') ? 'disabled' : ''],
+                       '$language'         => ['language', DI::l10n()->t('System language'), DI::config()->get('system', 'language'), !DI::config()->isWritable('system', 'language') ? DI::l10n()->t('<strong>Read-only</strong> because it is set by an environment variable") : '', $lang_choices, !DI::config()->isWritable('system', 'language') ? 'disabled' : ''],
                        '$theme'            => ['theme', DI::l10n()->t('System theme'), DI::config()->get('system', 'theme'), DI::l10n()->t('Default system theme - may be over-ridden by user profiles - <a href="%s" id="cnftheme">Change default theme settings</a>', DI::baseUrl() . '/admin/themes'), $theme_choices],
                        '$theme_mobile'     => ['theme_mobile', DI::l10n()->t('Mobile system theme'), DI::config()->get('system', 'mobile-theme', '---'), DI::l10n()->t('Theme for mobile devices'), $theme_choices_mobile],
                        '$force_ssl'        => ['force_ssl', DI::l10n()->t('Force SSL'), DI::config()->get('system', 'force_ssl'), DI::l10n()->t('Force all Non-SSL requests to SSL - Attention: on some systems it could lead to endless loops.')],
index 1bb5bf9abb0982a40e001c291a7c2c517082b67e..04c200429a65303d7b4ac4e58f0e52bb6a252503 100644 (file)
@@ -76,7 +76,7 @@ class Storage extends BaseAdmin
                        }
                }
 
-               if (!empty($_POST['submit_save_set']) && !DI::config()->isSetDisabled('storage', 'name') ) {
+               if (!empty($_POST['submit_save_set']) && DI::config()->isWritable('storage', 'name') ) {
                        try {
                                $newstorage = DI::storageManager()->getWritableStorageByName($storagebackend);
 
@@ -129,7 +129,7 @@ class Storage extends BaseAdmin
                                'prefix' => $storage_form_prefix,
                                'form'   => $storage_form,
                                'active' => $current_storage_backend instanceof ICanWriteToStorage && $name === $current_storage_backend::getName(),
-                               'set_disabled' => DI::config()->isSetDisabled('storage', 'name'),
+                               'is_writable' => DI::config()->isWritable('storage', 'name'),
                        ];
                }
 
@@ -146,7 +146,7 @@ class Storage extends BaseAdmin
                        '$save_reload'           => DI::l10n()->t('Save & Reload'),
                        '$noconfig'              => DI::l10n()->t('This backend doesn\'t have custom settings'),
                        '$form_security_token'   => self::getFormSecurityToken("admin_storage"),
-                       '$storagebackend_ro_txt' => DI::config()->isSetDisabled('storage', 'name') ? DI::l10n()->t("Changing the current backend is prohibited because it is set by an environment variable") : '',
+                       '$storagebackend_ro_txt' => !DI::config()->isWritable('storage', 'name') ? DI::l10n()->t('Changing the current backend is prohibited because it is set by an environment variable') : '',
                        '$storagebackend'        => $current_storage_backend instanceof ICanWriteToStorage ? $current_storage_backend::getName() : DI::l10n()->t('Database (legacy)'),
                        '$availablestorageforms' => $available_storage_forms,
                ]);
index 0c9d671ea96c75eab97b75e48e7ce5bf1a3e57d1..800f41a7e9a97af3f3666d6cd44452879eac73e4 100644 (file)
@@ -629,9 +629,9 @@ class ConfigTest extends DatabaseTest
                foreach ($data as $category => $keyvalues) {
                        foreach ($keyvalues as $key => $value) {
                                if (!empty($assertDisabled[$category][$key])) {
-                                       static::assertTrue($config->isSetDisabled($category, $key), sprintf("%s.%s is not true", $category, $key));
+                                       static::assertTrue($config->isWritable($category, $key), sprintf('%s.%s is not true', $category, $key));
                                } else {
-                                       static::assertFalse($config->isSetDisabled($category, $key), sprintf("%s.%s is not false", $category, $key));
+                                       static::assertFalse($config->isWritable($category, $key), sprintf('%s.%s is not false', $category, $key));
                                }
                        }
                }
index f39e2aa969762bb89abd8a35763656d26a37d56c..787ec56045572ddbc5241b7d7a6f2f17366c2a8f 100644 (file)
 
                {{if $storage.form}}
                <input type="submit" name="submit_save" value="{{$save}}"/>
-        {{if ! $storage.set_disabled}}
+        {{if $is_writable}}
                                {{if $storage.active}}
                <input type="submit" name="submit_save_set" value="{{$save_reload}}"/>
                                {{else}}
                <input type="submit" name="submit_save_set" value="{{$save_use}}"/>
                                {{/if}}
                {{/if}}
-               {{elseif ! $storage.set_disabled}}
+               {{elseif $is_writable}}
                <br /><input type="submit" name="submit_save_set" {{if $storage.active}}disabled="disabled"{{/if}} value="{{$use}}"/>
                {{/if}}
        </form>
index cdbe7bc34afca1b29567a3d6910abb5242c10220..c1bfe371bc2aaa0d4c6cbb9f8ad6a5035d71322d 100644 (file)
@@ -6,7 +6,7 @@
        <div class="well well-lg">
                {{$label_current}}: <b>{{$storagebackend}}</b>
                        {{if $storagebackend_ro_txt}}
-                       <p><p><i>{{$storagebackend_ro_txt nofilter}}</i>
+                       <br><i>{{$storagebackend_ro_txt nofilter}}</i>
                        {{/if}}
        </div>
 
                                <div class="panel-footer">
                                        {{if $storage.form}}
                                        <input type="submit" name="submit_save" class="btn btn-primary" value="{{$save}}"/>
-                           {{if ! $storage.set_disabled}}
+                           {{if $is_writable}}
                                                        {{if $storage.active}}
                                        <input type="submit" name="submit_save_set" class="btn btn-primary" value="{{$save_reload}}"/>
                                                        {{else}}
                                        <input type="submit" name="submit_save_set" class="btn btn-primary" value="{{$save_use}}"/>
                                                        {{/if}}
                                                {{/if}}
-                                       {{elseif ! $storage.set_disabled }}
+                                       {{elseif $is_writable}}
                                        <input type="submit" name="submit_save_set" class="btn btn-primary" {{if $storage.active}}disabled="disabled"{{/if}} value="{{$use}}"/>
                                        {{/if}}
                                </div>