From 6e699d330f219af7d2aeadb7c3edbaca3a6dd696 Mon Sep 17 00:00:00 2001
From: Bernd Oliver Suenderhauf <bos@suenderhauf.de>
Date: Tue, 26 Mar 2019 17:04:37 +0100
Subject: [PATCH] Issue #3040903 by Pancho: Offer all available file schemes,
but only those, preselecting the site default
---
config/install/fillpdf.settings.yml | 2 -
fillpdf.services.yml | 2 +-
src/Entity/FillPdfForm.php | 30 ++++++-
src/Form/FillPdfSettingsForm.php | 47 +++++++++-
src/InputHelper.php | 1 -
src/Service/FillPdfAdminFormHelper.php | 65 ++++++++++++--
.../Functional/FillPdfSettingsFormTest.php | 86 +++++++++++++++++++
tests/src/Traits/TestFillPdfTrait.php | 12 ++-
8 files changed, 223 insertions(+), 22 deletions(-)
create mode 100644 tests/src/Functional/FillPdfSettingsFormTest.php
diff --git a/config/install/fillpdf.settings.yml b/config/install/fillpdf.settings.yml
index dce5b8d..22e2228 100644
--- a/config/install/fillpdf.settings.yml
+++ b/config/install/fillpdf.settings.yml
@@ -2,5 +2,3 @@ remote_protocol: https
# Should not contain a protocol. That's what the above is for.
remote_endpoint: fillpdf.io/xmlrpc.php
-
-scheme: public
diff --git a/fillpdf.services.yml b/fillpdf.services.yml
index 091d7ab..591210b 100644
--- a/fillpdf.services.yml
+++ b/fillpdf.services.yml
@@ -8,7 +8,7 @@ services:
fillpdf.admin_form_helper:
class: Drupal\fillpdf\Service\FillPdfAdminFormHelper
- arguments: ['@module_handler', '@config.factory']
+ arguments: ['@module_handler', '@config.factory', '@stream_wrapper_manager', '@database']
# I don't like the name of this, but it is what it does...it translates
# the context provided by a FillPDF Link into loaded entities (or serializes)
diff --git a/src/Entity/FillPdfForm.php b/src/Entity/FillPdfForm.php
index 8db0819..9a30119 100644
--- a/src/Entity/FillPdfForm.php
+++ b/src/Entity/FillPdfForm.php
@@ -120,15 +120,13 @@ class FillPdfForm extends ContentEntityBase implements FillPdfFormInterface {
->setLabel('Storage system for generated PDFs')
->setDescription(t('This setting is used as the storage/download method for generated PDFs. The use of public files is more efficient, but does not provide any access control. Changing this setting will require you to migrate associated files and data yourself and is not recommended after you have uploaded a template.'))
->setSettings([
- 'allowed_values' => FillPdfAdminFormHelper::schemeOptions(),
+ 'allowed_values_function' => [get_called_class(), 'getSchemeAllowedValues'],
])
+ ->setDefaultValueCallback(get_called_class() . '::getSchemeDefaultValue')
->setRequired(TRUE)
->setDisplayOptions('form', [
'type' => 'options_buttons',
'weight' => 25,
- 'settings' => [
- 'options' => FillPdfAdminFormHelper::schemeOptions(),
- ],
]);
$fields['destination_redirect'] = BaseFieldDefinition::create('boolean')
@@ -154,4 +152,28 @@ class FillPdfForm extends ContentEntityBase implements FillPdfFormInterface {
return $fields;
}
+ /**
+ * Allowed values callback for 'scheme' base field definition.
+ *
+ * @see ::baseFieldDefinitions()
+ *
+ * @return array
+ * Stream wrapper descriptions, keyed by scheme.
+ */
+ public static function getSchemeAllowedValues() {
+ return \Drupal::service('fillpdf.admin_form_helper')->schemeOptions();
+ }
+
+ /**
+ * Default value callback for 'scheme' base field definition.
+ *
+ * @see ::baseFieldDefinitions()
+ *
+ * @return string
+ * The system's default scheme.
+ */
+ public static function getSchemeDefaultValue() {
+ return \Drupal::config('system.file')->get('default_scheme');
+ }
+
}
diff --git a/src/Form/FillPdfSettingsForm.php b/src/Form/FillPdfSettingsForm.php
index 02f0f0b..88b56de 100644
--- a/src/Form/FillPdfSettingsForm.php
+++ b/src/Form/FillPdfSettingsForm.php
@@ -2,14 +2,20 @@
namespace Drupal\fillpdf\Form;
+use Drupal\Core\Render\Markup;
+use Drupal\Component\Render\FormattableMarkup;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Form\ConfigFormBase;
use Drupal\Core\Form\FormStateInterface;
+use Drupal\Core\StreamWrapper\StreamWrapperInterface;
+use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface;
use Drupal\Core\Url;
use Drupal\fillpdf\Component\Utility\FillPdf;
use Drupal\fillpdf\Service\FillPdfAdminFormHelper;
use GuzzleHttp\Client;
use Symfony\Component\DependencyInjection\ContainerInterface;
+use Drupal\Core\Link;
+use Drupal\fillpdf\Entity\FillPdfForm;
/**
* Configure FillPDF settings form.
@@ -72,13 +78,48 @@ class FillPdfSettingsForm extends ConfigFormBase {
$config = $this->config('fillpdf.settings');
- $scheme_options = FillPdfAdminFormHelper::schemeOptions();
+ $config_scheme = $config->get('scheme');
+ $schemes = $this->adminFormHelper->schemeOptions();
+
+ // Set an error if the previously configured scheme doesn't exist anymore.
+ if ($config_scheme && !array_key_exists($config_scheme, $schemes)) {
+ $error_message = $this->t('Your previously used file scheme %previous_scheme is no longer available on this Drupal site, see the %system_settings. Please reset your default to an existing file scheme.', [
+ '%previous_scheme' => $config_scheme . '://',
+ '%system_settings' => Link::createFromRoute($this->t('File system settings'), 'system.file_system_settings')->toString(),
+ ]);
+
+ // @todo: It would be helpful if we could use EntityQuery instead, see
+ // https://www.drupal.org/project/fillpdf/issues/3043508.
+ $map = $this->adminFormHelper->getFormsByTemplateScheme($config_scheme);
+ if ($count = count($map)) {
+ $forms = FillPdfForm::loadMultiple(array_keys($map));
+ $items = [];
+ foreach ($map as $form_id => $file_uri) {
+ $fillpdf_form = $forms[$form_id];
+ $admin_title = current($fillpdf_form->get('admin_title')->getValue());
+ // @todo: We can simpify this once an admin_title is #required,
+ // see https://www.drupal.org/project/fillpdf/issues/3040776.
+ $link = Link::fromTextAndUrl($admin_title ?: "FillPDF form {$fillpdf_form->id()}", $fillpdf_form->toUrl());
+ $items[$form_id] = new FormattableMarkup("@fillpdf_form: {$file_uri}", ['@fillpdf_form' => $link->toString()]);
+ }
+ $list = [
+ '#theme' => 'item_list',
+ '#items' => $items,
+ ];
+ $error_message .= '<br />' . $this->t('Nevertheless, the following FillPDF forms will not work until their respective PDF templates have been moved to an existing file scheme:<br />@list', [
+ '@list' => \Drupal::service('renderer')->renderPlain($list),
+ ]);
+ }
+
+ $this->messenger()->addError(new FormattableMarkup($error_message, []));
+ $this->logger('fillpdf')->critical('File scheme %previous_scheme is no longer available.' . $count ? " $count FillPDF forms are defunct." : '');
+ }
$form['scheme'] = [
'#type' => 'radios',
'#title' => $this->t('Template download method'),
- '#default_value' => $config->get('scheme'),
- '#options' => $scheme_options,
+ '#default_value' => array_key_exists($config_scheme, $schemes) ? $config_scheme : $this->config('system.file')->get('default_scheme'),
+ '#options' => $schemes,
'#description' => $this->t('This setting is used as the download method for uploaded templates. The use of public files is more efficient, but does not provide any access control. Changing this setting will require you to migrate associated files and data yourself and is not recommended after you have uploaded a template.'),
];
diff --git a/src/InputHelper.php b/src/InputHelper.php
index a518281..d3931c0 100644
--- a/src/InputHelper.php
+++ b/src/InputHelper.php
@@ -43,7 +43,6 @@ class InputHelper implements InputHelperInterface {
$fillpdf_form = FillPdfForm::create([
'file' => $file,
'title' => $file->filename,
- 'scheme' => $this->config('fillpdf.settings')->get('scheme'),
]);
}
diff --git a/src/Service/FillPdfAdminFormHelper.php b/src/Service/FillPdfAdminFormHelper.php
index 15a3875..d7c4935 100644
--- a/src/Service/FillPdfAdminFormHelper.php
+++ b/src/Service/FillPdfAdminFormHelper.php
@@ -3,8 +3,12 @@
namespace Drupal\fillpdf\Service;
use Drupal\Core\Config\ConfigFactoryInterface;
+use Drupal\Core\Database\Connection;
use Drupal\Core\Extension\ModuleHandlerInterface;
+use Drupal\Core\StreamWrapper\StreamWrapperInterface;
+use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface;
use Drupal\fillpdf\FillPdfAdminFormHelperInterface;
+use Drupal\Core\StringTranslation\TranslatableMarkup;
class FillPdfAdminFormHelper implements FillPdfAdminFormHelperInterface {
@@ -14,9 +18,25 @@ class FillPdfAdminFormHelper implements FillPdfAdminFormHelperInterface {
/** @var \Drupal\Core\Config\ConfigFactoryInterface */
protected $configFactory;
- public function __construct(ModuleHandlerInterface $module_handler, ConfigFactoryInterface $config_factory) {
+ /**
+ * The stream wrapper manager.
+ *
+ * @var \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface
+ */
+ protected $streamWrapperManager;
+
+ /**
+ * The database connection.
+ *
+ * @var \Drupal\Core\Database\Connection
+ */
+ protected $connection;
+
+ public function __construct(ModuleHandlerInterface $module_handler, ConfigFactoryInterface $config_factory, StreamWrapperManagerInterface $stream_wrapper_manager, Connection $connection) {
$this->moduleHandler = $module_handler;
$this->configFactory = $config_factory;
+ $this->streamWrapperManager = $stream_wrapper_manager;
+ $this->connection = $connection;
}
/**
@@ -31,17 +51,46 @@ class FillPdfAdminFormHelper implements FillPdfAdminFormHelperInterface {
}
/**
- * Returns acceptable file scheme options.
+ * Returns acceptable file scheme options for use with FAPI radio buttons.
*
- * Suitable for use with FAPI radio buttons.
+ * Any visible, writeable wrapper can potentially be used.
*
* @return array
+ * Stream wrapper descriptions, keyed by scheme.
*/
- public static function schemeOptions() {
- return [
- 'private' => t('Private files'),
- 'public' => t('Public files'),
- ];
+ public function schemeOptions() {
+ $site_default = $this->configFactory->get('system.file')->get('default_scheme');
+ $streams = $this->streamWrapperManager;
+
+ $options = [];
+ foreach (array_keys($streams->getWrappers(StreamWrapperInterface::WRITE_VISIBLE)) as $scheme) {
+ $name = $streams->getViaScheme($scheme)->getName();
+ $description = $streams->getViaScheme($scheme)->getDescription();
+ $string = '<strong>' . $name . '</strong>';
+ if ($scheme == $site_default) {
+ $string .= ' (' . new TranslatableMarkup('site default') . ')';
+ }
+ $string .= ': ' . $description;
+ $options[$scheme] = $string;
+ }
+
+ return $options;
+ }
+
+ /**
+ * Returns all FillPdfForms with template PDFs stored in a particular scheme.
+ *
+ * @return string $scheme
+ * Scheme of the templates PDFs.
+ */
+ public function getFormsByTemplateScheme($scheme) {
+ $query = $this->connection->query("SELECT u.id AS form_id, f.uri AS file_uri
+ FROM {file_usage} u INNER JOIN {file_managed} f ON u.fid = f.fid
+ WHERE (type = :type) AND (uri LIKE :scheme)", [
+ ':type' => 'fillpdf_form',
+ ':scheme' => "{$scheme}://%"
+ ]);
+ return $query->fetchAllKeyed();
}
public static function getReplacementsDescription() {
diff --git a/tests/src/Functional/FillPdfSettingsFormTest.php b/tests/src/Functional/FillPdfSettingsFormTest.php
new file mode 100644
index 0000000..e6d10dd
--- /dev/null
+++ b/tests/src/Functional/FillPdfSettingsFormTest.php
@@ -0,0 +1,86 @@
+<?php
+
+namespace Drupal\Tests\fillpdf\Functional;
+
+use Drupal\Core\Url;
+use Drupal\Tests\BrowserTestBase;
+use Drupal\Tests\fillpdf\Traits\TestFillPdfTrait;
+
+/**
+ * @coversDefaultClass \Drupal\fillpdf\Form\FillPdfSettingsForm
+ * @group fillpdf
+ */
+class FillPdfSettingsFormTest extends BrowserTestBase {
+ public static $modules = ['fillpdf_test', 'file_test'];
+
+ use TestFillPdfTrait;
+
+ protected function setUp() {
+ parent::setUp();
+
+ $this->initializeUser();
+ }
+
+ /**
+ * Tests the settings form.
+ */
+ public function testSettingsForm() {
+ // FillPDF is not yet configured. Scheme should be initialized with the site
+ // default. Backend should be set to 'fillpdf_service'.
+ $this->drupalGet(Url::fromRoute('fillpdf.settings'));
+ $this->assertSession()->pageTextContains('Public files (site default)');
+ $this->assertSession()->checkboxChecked('edit-scheme-public');
+ $this->assertSession()->checkboxChecked('edit-backend-fillpdf-service');
+
+ // Now set the site default to 'private'.
+ $config = $this->container->get('config.factory')
+ ->getEditable('system.file')
+ ->set('default_scheme', 'private');
+ $config->save();
+
+ // Scheme should now be initialized with the new site default. Backend
+ // should still be 'fillpdf_service'.
+ $this->drupalGet(Url::fromRoute('fillpdf.settings'));
+ $this->assertSession()->pageTextContains('Private files (site default)');
+ $this->assertSession()->checkboxChecked('edit-scheme-private');
+ $this->assertSession()->checkboxChecked('edit-backend-fillpdf-service');
+
+ // Select 'dummy_remote://' as new default scheme.
+ $this->configureBackend('dummy_remote');
+
+ // Verify that the default 'dummy_remote' stream wrapper is present.
+ $this->drupalGet(Url::fromRoute('fillpdf.settings'));
+ $this->assertSession()->elementExists('css', '#edit-scheme-dummy-remote');
+
+ // Now uninstall the test module with the dummy stream wrappers.
+ $this->assertTrue(\Drupal::service('module_installer')->uninstall(['file_test']), "Module 'file_test' has been uninstalled.");
+ $this->assertFalse(\Drupal::moduleHandler()->moduleExists('file_test'), "Module 'file_test' is no longer present.");
+
+ // Reload the page and verify that 'dummy_remote' is gone.
+ $this->drupalGet(Url::fromRoute('fillpdf.settings'));
+ $this->assertSession()->elementNotExists('css', '#edit-scheme-dummy-remote');
+ $this->assertSession()->pageTextContains('Your previously used file scheme dummy_remote:// is no longer available');
+
+ // Select 'private://' as new default scheme.
+ // @todo Once https://www.drupal.org/project/fillpdf/issues/3040901 is in,
+ // this should be set via the UI:
+ // $this->drupalPostForm(NULL, ['scheme' => 'private'], 'Save configuration');
+ // $this->assertSession()->pageTextNotContains('An illegal choice has been detected.');
+ $this->configureBackend('private');
+
+ // Now remove the private path and rebuild the container.
+ $settings['settings']['file_private_path'] = (object) [
+ 'value' => '',
+ 'required' => TRUE,
+ ];
+ $this->writeSettings($settings);
+ $this->rebuildContainer();
+
+ // Reload the page and and verify that 'private' is gone, too.
+ $this->drupalGet(Url::fromRoute('fillpdf.settings'));
+ $this->assertSession()->elementNotExists('css', '#edit-scheme-private');
+ $this->assertSession()->pageTextContains('Your previously used file scheme private:// is no longer available');
+
+ }
+
+}
diff --git a/tests/src/Traits/TestFillPdfTrait.php b/tests/src/Traits/TestFillPdfTrait.php
index c2356e7..874f4a0 100644
--- a/tests/src/Traits/TestFillPdfTrait.php
+++ b/tests/src/Traits/TestFillPdfTrait.php
@@ -11,14 +11,20 @@ trait TestFillPdfTrait {
/**
* Configures the FillPdf test backend.
+ *
+ * @param string $scheme
+ * (optional) The file system scheme to use for PDF templates. Defaults
+ * to 'public'.
+ * @param string $backend
+ * (optional) The backend to use. Defaults to 'test'.
*/
- protected function configureBackend() {
+ protected function configureBackend($scheme = 'public', $backend = 'test') {
// FillPDF needs to be configured.
/** @var \Drupal\Core\Config\Config $fillpdf_settings */
$fillpdf_settings = $this->container->get('config.factory')
->getEditable('fillpdf.settings')
- ->set('scheme', 'public')
- ->set('backend', 'test');
+ ->set('scheme', $scheme)
+ ->set('backend', $backend);
$fillpdf_settings->save();
}
--
GitLab