I've tested and reviewed the code and it does what it claims to do. Although I alone am not the community, if you're happy for a single review then I'll update the status.
This works well and is a valuable addition to the module. We use Views Data Export for a lot of our clients and being able to export the data for processing makes this even more useful.
@qpro, just add the URLs for all the environments, the field accepts multiple values.
I haven't made the exact changes in the MR, which were hard to follow, but I'm not seeing any errors.
This is in dev.
The iframe element will have width=""
and height=""
attributes. The width and height values returned from the oembed provider will allow you to work out the aspect ratio, but this would require more than just CSS to make use of.
That said, I tend to use FitVids → to solve this problem.
Ref: https://www.php.net/manual/en/function.realpath.php
Note:
The running script must have executable permissions on all directories in the hierarchy, otherwise realpath() will return false.
If a parent directory doesn't exist then realpath will return false, excluding it from this module's file_exists check and therefore not showing it in the "not on server report".
Never mind. Go to /admin/appearance/settings/civic_subtheme
, go to "Content provisioning" and click "Provision content".
Media Download Tracker → is an alternative I've written for tracking downloads not using entity_usage.
I'm seeing this error every no matter how I install the theme and subtheme so I'm not sure my "steps to reproduce" would actually help.
I've tried following:
- the project page:
https://www.drupal.org/project/civictheme →
- the docs: https://docs.civictheme.io/development/drupal-theme, https://docs.civictheme.io/development/drupal-theme/sub-theme
- the "Next steps" after installing civic theme subtheme dependencies:
npm install
npm run build
npm run storybook
Enable theme in Drupal:
drush theme:enable civictheme -y
drush config-set system.theme default civictheme
drush theme:enable dink_civic -y
drush config-set system.theme default dink_civic
I've tried via GUI and Drush for each step. Using Drush to install CivicTheme produced the following "warnings":
drush then civictheme
[warning] The "extra_field_block:node:civictheme_page:content_moderation_control" was not found
[warning] The "extra_field_block:node:civictheme_page:content_moderation_control" was not found
[warning] The "extra_field_block:node:civictheme_event:content_moderation_control" was not found
[warning] The "extra_field_block:node:civictheme_event:content_moderation_control" was not found
[warning] The "block_content:50c52459-c100-4e4d-92a4-9c4c77d8f8d0" was not found
[warning] The "block_content:7cf65f98-cbae-4e9d-b6ea-7d6d1c437c1c" was not found
[warning] The "block_content:d7098a8c-3ba3-48f7-bc0c-5787ebaa0427" was not found
[warning] The "block_content:5c0ad15f-3e9c-4eeb-8525-7a064b06fc58" was not found
[warning] The "block_content:b7f36176-620f-4178-aadd-9b448c610986" was not found
[warning] The "block_content:286ff750-a9a9-423a-8a4e-515f79fc0a8f" was not found
[warning] The "block_content:5508dc51-1cf8-4577-9038-b572fbd698ae" was not found
I came across this recently where installing the subtheme via the GUI timed out the first time then didn't seem to properly install and I got the error in 🐛 Installing civictheme via drush results in "This block is broken or missing." on main page Postponed: needs info . I uninstalled the subtheme and reinstalled it but still had the same error.
When I then set CivicTheme as the default it also had the missing block error, even though it was fine the first time I tested it. I then uninstalled the subtheme and CivicTheme and tried to reinstall CivicTheme and got the error: "Configuration (...) provided by civictheme already exist in active configuration".
This is with a fresh install of GovCMS so I can just reinstall it again but this may not be the case in all situations.
@sarwan_verma this module supports Drupal versions from 8.8 onwards. At the very least I'd like to support 9 and 10 as well as 11. This requires using the DeprecationHelper to maintain backward compatibility. This is described in the change record I linked to in #6.
I suspect the culprit might be this line in Drupal\auditfiles\Auditor\AuditFilesNotOnServer
:
$target = $this->fileSystem->realpath($result->uri);
This is converting a URI found in the file_managed
table to a realpath, but it appears to return false
if the file parent directory doesn't exist when it needs to return the full path to the missing file.
For context, here's the entire method:
public function getReferences(): \Generator {
$maximumRecords = $this->auditFilesConfig->getReportOptionsMaximumRecords();
$query = $this->connection->select('file_managed', 'fm');
$query
->orderBy('changed', 'DESC')
->range(0, $maximumRecords)
->fields('fm', ['fid', 'uri']);
/** @var array<object{uri: string, fid: string}> $results */
$results = $query->execute()->fetchAll();
foreach ($results as $result) {
$target = $this->fileSystem->realpath($result->uri);
if ($target !== FALSE && !file_exists($target)) {
yield FileEntityReference::create((int) $result->fid);
}
}
}
<img src="/path/to/image.png">
will also work.
@joseph.olstad phpmailer_smtp uses the PHPMailer library as much as possible rather than duplicating its functions. For example, you won't find a single preg_match in the module either, and all address parsing is done by the library.
To understand how the module works it helps to have some knowledge of the PHPMailer library.
Specifically, in the format()
method of Drupal\phpmailer_smtp\Plugin\Mail\PhpMailerSmtp
there is the following line:
$this->msgHTML($rendered, DRUPAL_ROOT, TRUE);
This sends the message HTML to the PHPMailer library for processing, including inline attachments. So rather than a bunch of if/then/else statement and calling the inline attachment functions directly, this leaves it up to PHPMailer to determine, including generating a cid.
Have a look at /vendor/phpmailer/phpmailer/src/PHPMailer.php
for more information.
I've changed this to a task so if the documentation is lacking in this area I'm happy to accept an MR.
This is already possible.
Including an image or other file reference within the HTML or template file, which can either be a path to the image or base64 encoded data within the <img>
tag, will result in the image being embedded and referenced by the cid.
I'll leave this open, let me know if you have a different scenario in mind.
I was following the existing pattern but yeah, I tend to agree.
It might be better to rewrite it to check for Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem
using is_a()
or similar. Or if it implements Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItemInterface
. But this might be tricky to do in annotations.
Applied this and the MR In 🐛 PluginNotFoundException: The "" plugin does not exist Needs review and can now use field states for select lists within paragraphs.
Another option would be to use the visibilitychange event which works more reliably on mobile platforms than beforeunload.
I just checked and the deprecated method will be removed in Drupal 12, not 11. I'll tag a Drupal 11 release and this can be fixed later.
I'd like to soon but I'm not sure the change proposed in the merge doesn't break compatibility with older Drupal versions.
@sarwan_verma can you review the change record I linked to.
@joseph.olstad yes this module doesn't need to be patched to make use of the Key module. Although including Key support directly can simplify the experience for the end user.
@liquidcms I checked one of the modules you mentioned and it optionally supports Key if it's installed. https://git.drupalcode.org/project/s3fs/-/blob/8.x-3.x/src/Form/Settings...
Added comment in MR.
Thanks @sarwan_verma. Will the module still work with versions of Drupal prior to 10.3 after this change? See the change record → for more information.
#5 see the response to your issue #3460838-3: Allow rows name property to be customizable. → (Views REST Serializer Extra ):
This module does allow changing the key names FYI:
@Sivaji_Ganesh_Jojodae, please fill in the section "Steps to reproduce" in the issue summary. It needs more information than just "Some of our views".
Composer or Ludwig → is required to install the module and its dependency, PHPMailer, as per the project page → .
One option is to set up a local development environment with composer to build the website then deploy it to your host.
Cache metadata may be required. Have a read through some previous issues about this: https://www.drupal.org/project/issues/webform_rest?text=ResourceResponse... →
@4kan, I missed that one.
Error in db log when going to Admin -> Config -> User interface -> Flipbook Settings
TypeError: Drupal\flipbook\Form\ChoosePdfStyleForm::__construct(): Argument #1 ($config) must be of type Drupal\Core\Config\ConfigFactoryInterface, Drupal\Core\Messenger\Messenger given, called in /web/modules/contrib/flipbook/src/Form/ChoosePdfStyleForm.php on line 52 in Drupal\flipbook\Form\ChoosePdfStyleForm->__construct() (line 39 of /web/modules/contrib/flipbook/src/Form/ChoosePdfStyleForm.php).
The parameters supplied from create()
are in a different order to what __construct()
expects. I've also re-ordered the assigmnents in __construct()
to match the order of the parameters.
This is a good feature request.
For others coming across this issue, a different hook is required when loading a previous submission by token. This also works when creating a new submission so may be preferrable to hook_ENTITY_TYPE_create()
.
/**
* Implements hook_ENTITY_TYPE_prepare_form().
*/
function CUSTOM_MODULE_webform_submission_prepare_form(\Drupal\webform\WebformSubmissionInterface $webform_submission, $operation, \Drupal\Core\Form\FormStateInterface $form_state) {
$current_page = \Drupal::request()->query->get('page');
if (empty($current_page)) {
return;
}
$pages = $webform_submission->getWebform()->getPages();
if (isset($pages[$current_page])) {
$form_state->set('current_page', $current_page);
}
}
Example request path:
/form/example-wizard?current_page=contact&token=PYNaCklH_mocS8aru_pdPJhE9Iicr9ibEKHNmxVU4UU
Create the simplest views possible to replicate the problem, export their config, and upload it here so we can test using exactly the same configuration.
As these are preventing the module from being used I've made a quick change to the MR and will merge it. Thanks.
OK I see the problem now. However the interface should be injected instead. Drupal\Component\Datetime\TimeInterface
NW for the above and the unrelated change mentioned in #5.
Thanks for the report. There's an unrelated change, but still valid, which should go in another issue:
- use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+ use Symfony\Component\HttpKernel\Event\RequestEvent;
That said, although I can replicate the problem, I'm not sure why it's happening. Drupal\Component\Datetime\TimeInterface
is what we want, and @datetime.time
is in the services.yml
, which is also correct. See: https://api.drupal.org/api/drupal/core%21core.services.yml/service/Drupa...
Try this first, from the project page → : In your "REST export nested" display, edit your fields settings and select "Raw output" for the JSON field.
If that doesn't resolve your issue, provide step by step instructions on how to reliably replicate the issue.
NW due to Key not being optional.
Have you tried overriding config for the password using the Key module as described in the documentation I linked to? Does this produce the desired outcome?
Thanks for the MR. If we were to integrate with the Key module then it should be an option, as you suggest, and then only if Key is installed.
That said, we're already using Key (+ Encrypt) with this module without supporting it directly. See Key's documentation → regarding overriding any configuration.
There's an issue with a patch for encrypting the value: ✨ Encrypt key value Needs work
A related issue which could resolve this issue has been committed to 10.3 and 11. I've added some screenshots of before and after for comparison. See #2966656-31: Negotiate max width/height of oEmbed assets more intelligently →
In a nutshell: oembed providers support maintaining aspect ratio at different sizes using query parameters maxwidth and maxheight. The above issue takes this into account so the width and height of the iframes will be set correctly.
Thanks @cleavinjosh, I'd liked to hear from @FiNeX as well if possible.
Has anyone else had a chance to review this? Does this solve the problem @FiNeX?
Thanks, committed.
Thanks, committed.
Symfony\Component\HttpKernel\Event\GetResponseEvent
has been replaced by Symfony\Component\HttpKernel\Event\RequestEvent
. See
#3094398: Update Symfony response and request events to new classes →
I'm unlikely to work on this any time soon.
Another module which works with PHPMailer SMTP → and OAuth2 Client → is Gmail OAuth2 Client.
The URL for the feed is https://www.codeenigma.com/rss.xml .
The fieldblock issue in #7 also adds support for third party settings which is used by Photoswipe Dynamic Caption.
I've added support for third party settings. To test, enable Photoswipe's submodule Photoswipe Dynamic Caption and configure.
I'm looking at adding support for third party settings. I think this only makes sense per instance so can probably be included here as well.
I've created a proof of concept which lists relevant fields separately per bundle as well as an "all bundles" option, which is the default and works the same as it does now without any changes to config.
Before I refine the code for sharing I'd like some feedback on the approach. This is how the field list would present.
Fieldblock might be the problem. In its file src/Plugin/Block/FieldBlock.php
there's a comment for getFieldDefinition()
:
* A FieldBlock works on an entity type across bundles, and thus only has
* access to field storage definitions. In order to be able to use formatters,
* we create a generic field definition out of that storage definition.
I think both the entity type and bundle are required to get additional config settings via entityFieldManager->getFieldDefinitions()
.
This works fine as a normal field formatter.
Using
Field block →
the target_bundles
is missing (and possibly other field config settings).
I can't tell which of the 3 modules is responsible.
imclean → changed the visibility of the branch 3422506-support-media-library-modify to hidden.
imclean → created an issue.
I'm also using this patch #2966656-34: Negotiate max width/height of oEmbed assets more intelligently → .
I'm not sure this patch is necessary. Add the oembed URL (e.g. https://www.example.com/media/oembed
) to "Additional video providers" to get fitvids to recognise it.
No feedback or response to the question in #10.
@pacproduct it looks like all settings need to be disabled at once.
Steps to reproduce:
- Enable both "Protect Enquiries webform with Honeypot" and "Add time restriction to Enquiries webform"
- Try to disable either setting individually
The workaround is to disable both then enable the one you want.
Thanks for the report. Does this fix it for you?
Thanks, committed to latest dev.
This is better.
OK see if this works for you.
Yes I know what your patch does, but I don't think we should add a new setting for this. The correct way to resolve the issue would be to set the content-type header, not provide a config option to get around problems with other modules.
Thanks for the detailed report and patch.
In the PhpMailerSmtp->mail function, there is a check to determine the charset to use for the message. It will look at the $headers['content-type'] value and use the charset, if found. If not, the charset keeps the default value of PHPMailer::CHARSET_ISO88591.
Yes, the place to set the charset is in the content-type header.
We checked our code and there were some modules there the 'Content-Type' value was simple 'text/html'. Others contains charset definitions like 'text/html; charset=UTF-8;'.
The Problem is now, that most of the modules expect the use of utf-8 but don't declare it at the point of sending a message.
That is indeed the problem. The sending modules which rely on a specific charset need to set the header correctly, which is the correct solution.
This module responsible for formatting and sending emails based on the parameters supplied. The intent is not to have config options for every possible SMTP and MIME settings so I'm not keen on adding this.
That said, I'd be open to setting a default of UTF-8 (as is the case with other headers) while honouring any content-type header. Would this resolve the issue for you?