Thanks
I've been playing with the latest patch for some time now, my general felling is the approach of making provider_id = '' an optional parameter is very dangerous and complicates a lot the code.
I think instead the provider_id should be a required parameter, and provide a hook_update_n to create a default one based on the current config. This will of course be a breaking compatibility change for custom code out there, but in the other hand tools like phpstan will allow to detect how to fix this properly.
The same should apply to auth manager.
I miss dreditor to review this .patch format, but I will do my best:
+++ b/modules/salesforce_mapping/src/Entity/MappedObject.php
@@ -409,7 +409,8 @@ class MappedObject extends RevisionableContentEntityBase implements MappedObject
$result = $this->client()->objectUpdate(
$mapping->getSalesforceObjectType(),
$this->sfid(),
- $params->getParams()
+ $params->getParams(),
+ $mapping->provider_id
Use $mapping->getProviderId() instead.
@@ -203,14 +203,17 @@ class RestClient implements RestClientInterface {
/**
* {@inheritdoc}
*/
- public function isInit() {
- if (!$this->authProvider || !$this->authManager) {
+ public function isInit($provider_id = '') {
+ if (!$this->authProvider || !$this->authManager->getProvider($provider_id)) {
return FALSE;
}
// If authToken is not set, try refreshing it before failing init.
if (!$this->authToken) {
try {
$this->authToken = $this->authManager->refreshToken();
+ if (!empty($provider_id)) {
+ $this->authToken = $this->authManager->refreshToken($provider_id);
+ }
return isset($this->authToken);
}
catch (\Exception $e) {
@@ -224,20 +227,26 @@ class RestClient implements RestClientInterface {
/**
* {@inheritdoc}
*/
- public function apiCall($path, $params = [], $method = 'GET', $returnObject = FALSE, array $headers = []) {
- if (!$this->isInit()) {
+ public function apiCall($path, $params = [], $method = 'GET', $returnObject = FALSE, array $headers = [], $provider_id = '') {
I think the provider Id should be required when initializing the rest client, and the provider id should not be set after the client is initialized, this allows to remove provider_id from all the methods of this class.
Thanks, looks good to me as well.
The method addFilterToQuery(Request $request, SelectInterface &$query) needs some documentation updates. The return type is not properly documented.
51 errors to go
------ -----------------------------------------------------------------------------------------------------------
Line Entity/OgMembership.php
------ -----------------------------------------------------------------------------------------------------------
322 Parameter #2 $callback of function array_filter expects (callable(Drupal\og\OgRoleInterface): bool)|null,
Closure(Drupal\og\Entity\OgRole): bool given.
325 Parameter #1 $callback of function array_map expects (callable(Drupal\og\OgRoleInterface): mixed)|null,
Closure(Drupal\og\Entity\OgRole): (int|string|null) given.
401 Parameter #2 $callback of function array_filter expects (callable(Drupal\og\OgRoleInterface): bool)|null,
Closure(Drupal\og\Entity\OgRole): bool given.
616 Call to an undefined method Drupal\Core\Entity\EntityInterface::getGroup().
------ -----------------------------------------------------------------------------------------------------------
------ ------------------------------------------------------------------------------------------------
Line MembershipManager.php
------ ------------------------------------------------------------------------------------------------
55 Interface Drupal\Core\Entity\EntityTypeManagerInterface referenced with incorrect case:
Drupal\core\Entity\EntityTypeManagerInterface.
308 Parameter #1 $group of method Drupal\og\OgMembershipInterface::setGroup() expects
Drupal\Core\Entity\ContentEntityInterface, Drupal\Core\Entity\EntityInterface given.
355 Call to an undefined method Drupal\Core\Entity\EntityInterface::get().
364 Call to an undefined method Drupal\Core\Entity\EntityInterface::get().
472 Instanceof between int and Drupal\Core\Session\AccountInterface will always evaluate to false.
💡 Because the type is coming from a PHPDoc, you can turn off this check by setting
treatPhpDocTypesAsCertain: false in your phpstan.neon.
486 Instanceof between int and Drupal\Core\Session\AccountInterface will always evaluate to false.
💡 Because the type is coming from a PHPDoc, you can turn off this check by setting
treatPhpDocTypesAsCertain: false in your phpstan.neon.
498 Instanceof between int and Drupal\Core\Session\AccountInterface will always evaluate to false.
💡 Because the type is coming from a PHPDoc, you can turn off this check by setting
treatPhpDocTypesAsCertain: false in your phpstan.neon.
553 Method Drupal\og\MembershipManager::loadGroups() should return
array<array<Drupal\Core\Entity\ContentEntityInterface>> but returns
array<array<Drupal\Core\Entity\EntityInterface>>.
------ ------------------------------------------------------------------------------------------------
------ -----------------------------------------------------------------------------------------------------
Line Og.php
------ -----------------------------------------------------------------------------------------------------
126 Method Drupal\og\Og::createField() should return Drupal\Core\Field\FieldConfigInterface but returns
Drupal\field\FieldConfigInterface.
185 Call to an undefined method Drupal\og\MembershipManagerInterface::getGroupMemberships().
205 Parameter #2 $user of method Drupal\og\MembershipManagerInterface::createMembership() expects
Drupal\user\UserInterface, Drupal\Core\Session\AccountInterface given.
------ -----------------------------------------------------------------------------------------------------
------ -------------------------------------------------------------------------------------------------------------
Line OgAccess.php
------ -------------------------------------------------------------------------------------------------------------
197 If condition is always true.
💡 Because the type is coming from a PHPDoc, you can turn off this check by setting
treatPhpDocTypesAsCertain: false in your phpstan.neon.
249 Call to an undefined method Drupal\Core\Access\AccessResultInterface::addCacheTags().
295 Call to method addCacheTags() on an unknown class Drupal\Core\Access\RefinableCacheableDependencyInterface.
💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
295 PHPDoc tag @var for variable $result contains unknown class
Drupal\Core\Access\RefinableCacheableDependencyInterface.
💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
319 Parameter #2 $group of class Drupal\og\Event\GroupContentEntityOperationAccessEvent constructor expects
Drupal\Core\Entity\ContentEntityInterface, Drupal\Core\Entity\EntityInterface given.
319 Parameter #3 $groupContent of class Drupal\og\Event\GroupContentEntityOperationAccessEvent constructor
expects Drupal\Core\Entity\ContentEntityInterface, Drupal\Core\Entity\EntityInterface given.
------ -------------------------------------------------------------------------------------------------------------
------ ---------------------------------------------------------------------------------------------------------------
Line OgRoleManager.php
------ ---------------------------------------------------------------------------------------------------------------
125 Method Drupal\og\OgRoleManager::getRequiredDefaultRoles() should return array<Drupal\og\Entity\OgRole> but
returns array<string, Drupal\Core\Entity\EntityInterface>.
136 Method Drupal\og\OgRoleManager::getRolesByBundle() should return array<Drupal\og\OgRoleInterface> but returns
array<Drupal\Core\Entity\EntityInterface>.
165 Method Drupal\og\OgRoleManager::getRolesByPermissions() should return array<Drupal\og\OgRoleInterface> but
returns array<Drupal\Core\Entity\EntityInterface>.
188 Negated boolean expression is always false.
💡 Because the type is coming from a PHPDoc, you can turn off this check by setting
treatPhpDocTypesAsCertain: false in your phpstan.neon.
------ ---------------------------------------------------------------------------------------------------------------
------ ----------------------------------------------------------------------------------------------------
Line PermissionManager.php
------ ----------------------------------------------------------------------------------------------------
61 Method Drupal\og\PermissionManager::getDefaultGroupPermissions() should return
array<Drupal\og\GroupPermission> but returns array<Drupal\og\PermissionInterface>.
81 Method Drupal\og\PermissionManager::getDefaultEntityOperationPermissions() should return
array<Drupal\og\GroupContentOperationPermission> but returns array<Drupal\og\PermissionInterface>.
------ ----------------------------------------------------------------------------------------------------
------ -------------------------------------------------------------------------------------
Line Plugin/EntityReferenceSelection/OgSelection.php
------ -------------------------------------------------------------------------------------
121 Call to an undefined method
Drupal\Core\Entity\EntityReferenceSelection\SelectionInterface::buildEntityQuery().
150 If condition is always true.
159 If condition is always true.
------ -------------------------------------------------------------------------------------
------ ---------------------------------------------------------------------------------------------------------------
Line Plugin/Field/FieldFormatter/GroupSubscribeFormatter.php
------ ---------------------------------------------------------------------------------------------------------------
158 If condition is always true.
💡 Because the type is coming from a PHPDoc, you can turn off this check by setting
treatPhpDocTypesAsCertain: false in your phpstan.neon.
193 Left side of && is always true.
💡 Because the type is coming from a PHPDoc, you can turn off this check by setting
treatPhpDocTypesAsCertain: false in your phpstan.neon.
193 Variable $access in PHPDoc tag @var does not exist.
198 Left side of && is always true.
💡 Remove remaining cases below this one and this error will disappear too.
218 Offset 'title' on array{title: Drupal\Core\StringTranslation\TranslatableMarkup, url: Drupal\Core\Url, class:
array{'unsubscribe'}} in empty() always exists and is not falsy.
------ ---------------------------------------------------------------------------------------------------------------
------ --------------------------------------------------------------------------------------------------------
Line Plugin/Field/FieldWidget/OgComplex.php
------ --------------------------------------------------------------------------------------------------------
138 Call to an undefined method Drupal\Core\TypedData\TypedDataInterface::get().
175 Access to an undefined property Drupal\Core\TypedData\TypedDataInterface::$_weight.
💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
285 Parameter #3 $weight_delta of method Drupal\og\Plugin\Field\FieldWidget\OgComplex::otherGroupsSingle()
expects int, float given.
305 Parameter #3 $weight_delta of method Drupal\og\Plugin\Field\FieldWidget\OgComplex::otherGroupsSingle()
expects int, float given.
------ --------------------------------------------------------------------------------------------------------
------ ---------------------------------------------------------------------------------------------------------------
Line Plugin/Validation/Constraint/UniqueOgMembershipConstraintValidator.php
------ ---------------------------------------------------------------------------------------------------------------
56 Method Drupal\og\Plugin\Validation\Constraint\UniqueOgMembershipConstraintValidator::validate() has parameter
$value with no type specified.
58 Variable $value in isset() always exists and is not nullable.
89 Access to an undefined property Symfony\Component\Validator\Constraint::$notUniqueMembership.
💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
------ ---------------------------------------------------------------------------------------------------------------
------ ---------------------------------------------------------------------------------------------------------------
Line Plugin/Validation/Constraint/ValidOgRoleConstraintValidator.php
------ ---------------------------------------------------------------------------------------------------------------
18 Method Drupal\og\Plugin\Validation\Constraint\ValidOgRoleConstraintValidator::validate() has parameter $value
with no type specified.
20 Variable $value in isset() always exists and is not nullable.
25 Negated boolean expression is always false.
💡 Because the type is coming from a PHPDoc, you can turn off this check by setting
treatPhpDocTypesAsCertain: false in your phpstan.neon.
31 Call to an undefined method Drupal\Core\Entity\FieldableEntityInterface::getGroup().
32 Call to an undefined method Drupal\Core\Entity\FieldableEntityInterface::getGroup().
34 Call to an undefined method Drupal\Core\Field\FieldItemInterface::referencedEntities().
36 Access to an undefined property Symfony\Component\Validator\Constraint::$notValidRole.
💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
------ ---------------------------------------------------------------------------------------------------------------
------ -----------------------------------------------------------------------------------------------------------
Line Plugin/views/argument_default/Group.php
------ -----------------------------------------------------------------------------------------------------------
77 Method Drupal\og\Plugin\views\argument_default\Group::buildOptionsForm() has parameter $form with no type
specified.
126 Method Drupal\og\Plugin\views\argument_default\Group::getGroup() should return
Drupal\Core\Entity\ContentEntityInterface|null but return statement is missing.
------ -----------------------------------------------------------------------------------------------------------
[ERROR] Found 51 errors
We replicated the problem in one of our sites. A QA analyst verified the patch of Merge request 33 fixes the problem. I reviewed the code and looks good, concerns in #17 are addressed by #19 and #20.
Found it. 🌱 [PLAN] New Toolbar Roadmap: Path to Beta & Stable Active
@ckrina thanks good to know :) So the navigation module will allow a color change and the environment indicator will have to interact with this new API? No CSS changes are required from this module if I understand this correctly? Any issue in core to check this?
Added ✨ Allow kernel tests to fail or expect logged errors Needs work to the issue summary to, to proper fix the problem with tests. Thanks @joachim
dagmar → changed the visibility of the branch 2401463-dblog-entities to hidden.
dagmar → changed the visibility of the branch 2401463-dblog-entities-D10 to hidden.
Isn't it possible that it will get too complicated for normal use? I know that uninstalling this module is not something, that is done on a daily basis, but even so.. We can argue that it is needed for all modules using content enties, but I still consider dblog entries as something different like a real content (for me these are just logs).
@poker10 I found a way 📌 Make dblog entities Postponed to not need this to implement 📌 Make dblog entities Postponed
I will keep this issue open in case someone else want to work on this, but the original problem described in the issue is not a problem anymore.
I found a way to not have to pause the creation of logs and remove the dblog module transparently as usual.
The approach is not elegant, but it works. When dblog module is asked if has data from the ContentUninstallValidator it replies no... And the module can be uninstalled. Logs are deleted anyways by the dblog_module_preuninstall hook. There is no other way to implement that I'm aware of as the ModuleInstaller::uninstall method does not offer any other opportunity to modify the validations.
This means this issue is not longer postponed on ✨ Provide a way to disable creation of database log entries Needs work
$config['environment_indicator.indicator']['bg_color'] = '#00a073';
$config['environment_indicator.indicator']['fg_color'] = '#ffffff';
With this configuration and this patch I see this.
Patch #3 does not work for me:
Fixed in 1.0.3
I think solution C proposed in #61 🐛 Dependency on config storage causes circular reference in service container Needs review is totally aligned with what I suggested in #4 🐛 Dependency on config storage causes circular reference in service container Needs review . Thanks! @mfb
Another interesting approach to analyze is the UI provided by the paragraph module. As it allows you to delete the content "on the fly" when deleting a paragraph type.
@poker10 I think you pointed out some valid inputs. I had the same thoughts as well and this is why I tried to avoid this in the first place while coding 📌 Make dblog entities Postponed
Unfortunately I don't see an easy way to avoid this, at least without not touching a lot of content related stuff involved with content deletion.
The way I see it, dblog entries will be the same than Comments now. If you have a non public site, deleting the logs will be enough to not need to pause them.
I'm thinking maybe we can avoid this and improve the UX at the same time to allow you to block temporally log entries while you are deleting them.
I think we can also play with ?redirect=to in the url to make this as simple as possible.
Eventually most developers will use drush to uninstall dblog, so I think we can provide a patch to make this in one go if we need to.
Thanks @marvil07
Great to see a new validation! However I think the proper validation should be a number 0 or greater. We don't know if someone is altering this value with custom code and the only thing we need to ensure to have dblog working properly is row_limit an unsigned integer.
I think this will require a change record as it is changing the constructor signature.
Thanks @mfb. I think the service is a good idea. But I won't be able to work on this for some time, so #47 for now is a good option as well.
Random test failure.
One more fix. Before, clicking form descriptions was re-sending any form.
Random test failure.
I've been trying this today. It seems the approach to alter the service is not actually necessary. Dblog can provide the service using by DblogServiceProvider::register and provide the regular logger only when the config is not available.
The only example in core I found doing something kind of similar is the Drupal\language\LanguageServiceProvider.
The logic then would be something like this:
namespace Drupal\dblog;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\DependencyInjection\ServiceProviderBase;
class DblogServiceProvider extends ServiceProviderBase {
public function register(ContainerBuilder $container) {
if (\Drupal::configFactory()->getEditable('dblog.settings')->get('logging_paused')) {
return;
}
$container->register('dblog.logger', 'Drupal\dblog\Logger\DbLog')
// ...
}
}
\Drupal::configFactory() wont work at this register phase I'm afraid. But again LanguageServiceProvider provides some clues how what to do.
In the formatMessage() method, the variables are unserialized and the backtrace markup object is created for use in the message. It would be redundant to repeat these two steps again in the eventDetails()
@mfb I see. Thanks!
Are we re-introducing #2371141: XSS vulnerability when displaying exception backtrace → because we are not sanitizing the output? I would love to see a test case for this new addition.
Ok, one more round, this patch is bigger than #10 because I went over all the occurrences of once.
@mfb Any particular reason why
// Save a reference so the backtrace can be displayed separately.
if (!str_contains($row->message, '@backtrace_string')) {
$row->backtrace = $variables['@backtrace_string'];
}
Can be calculated inside the eventDetails method? The logic here seems a bit odd to follow. Also with change from #52 ::formatMessage would do more than just formatting the message.
Once more once required in js/form-toggle-description.js
Sorry for reopening this, but the commit made here https://www.drupal.org/project/rollbar/issues/3289422#comment-15183543 📌 Drupal 10 compatibility fixes Fixed is not the right fix.
As I learned more about D10 upgrades I think the right fix here is:
Have one release to be D9 compatible only, and have another one to be D10 compatible only, and then advice to use this composer version:
"drupal/rollbar: "3 || 4",
Assuming v4 will be only D10 compatible.
[#3197514]
I ran upgrade_status to check what are the missing manual fixes to apply. This patch seems to cover them all.
@intrafusion I created issue #3379795 with the intention of not committing it. If you apply my fix the module won't be compatible with D10. This is why I mentioned it is something hard to fix, you can't be compatible with the 2 PSR3 signatures at the same time.
I'm leaving the patch here. This is only required if you plan to upgrade rollbar to the latest version while you are still using Drupal 9 (but plan to upgrade to D10 soon).
So the problem here is with the update but not with the fresh install? We ran all the tests for Sqlite, Mysql and Postgres https://www.drupal.org/project/drupal/issues/3081144#comment-15012792 🐛 Database primary keys can exceed maximum integer storeable (has actually occurred, in watchdog) Fixed is this a lack of tests?
Looks good, and fixes a problem that otherwise it quite complicated to recover from (see the original issue description). However as is this patch will not apply to PHP 7.4 as Stringable is only PHP 8 available.
What about adding the SVG content instead of the path to an icon as part of the json?
[
{
"title": "Link",
"icon": '<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24"><path d="M8 16h8v2H8zm0-4h8v2H8zm6-10H6c-1.1 0-2 .9-2 2v16c0 1.1.89 2 1.99 2H18c1.1 0 2-.9 2-2V8l-6-6zm4 18H6V4h7v5h5v11z"/></svg>
',
"description": "Insert a link to the Drupal CKEditor5 project site.",
"html": "<p>Do you know this cool <a href='https://www.drupal.org/project/ckeditor5' target='_blank'>editor</a>?</p>"
},
{
"title": "Simple Table",
"icon": '<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24"><path d="M8 16h8v2H8zm0-4h8v2H8zm6-10H6c-1.1 0-2 .9-2 2v16c0 1.1.89 2 1.99 2H18c1.1 0 2-.9 2-2V8l-6-6zm4 18H6V4h7v5h5v11z"/></svg>
',
"description": "Insert a simple Table.",
"html": "<table border='1'><tr><th>Header 1</th><th>Header 2</th><th>Header 3</th></tr><tr><td>Row 1, Column 1</td><td>Row 1, Column 2</td><td>Row 1, Column 3</td></tr><tr><td>Row 2, Column 1</td><td>Row 2, Column 2</td><td>Row 2, Column 3</td></tr><tr><td>Row 3, Column 1</td><td>Row 3, Column 2</td><td>Row 3, Column 3</td></tr></table>"
}
]
A minimalist approach if you don't want to deal with user input json in the ui is the use the getDynamicPluginConfig to retrieve the path of the template, and use $this->moduleHandler->alter() to allow other modules to alter the path passing the $editor as argument. Not sure how this gets cached though.
@dbielke1986 thanks for the quick response. I'm not fully familiar with CkEditor plugins yet, but I've been inspecting the code and checking different alternative approaches.
I think my proposal of services is not necessary, we can achieve the same result using CKEditor Plugins. The most similar plugin I found for this task is the core/modules/ckeditor5/src/Plugin/CKEditor5Plugin/SourceEditing.php plugin. It provides a text area that could be use to define the json of the expected templates there. And exposes this config inside via getDynamicPluginConfig.
Inside the Ckeditor JS plugin you can access this settings using: this.editor.plugins.get('name_of_your_config_key');
You can find another full example in web/core/modules/ckeditor5/src/Plugin/CKEditor5Plugin/Media.php and how it loads the config in core/modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalmediaui.js
This approach will save a template per filter format. While it is not fully flexible at least allows to have multiple configs instead of just one.
And eventually a moduleHandler->alter() could be added to allow other modules to alter the template before injecting it into the js.
Create a possibility to specify the configuration file which should be used.
I would like to suggest to define this from a service. Even if the default service loads the value from a config form.
The reason to do this is to allow more complex workflows where different templates can be loaded based on the current user or the current wysiwyg/filter format.
Thanks!. Fixed in release 1.0.2.
Thanks! @Nikolas Haliotis
There are a few coding standards that needs to be fixed, but those are irrelevant now. The only thing that is not 100% to me right now is how to make the DblogServiceProvider::alter code run as soon the Dblog Form is submitted.
I'm not sure if \Drupal::service('kernel')->rebuildContainer(); is the way to go, or if there is something less aggressive. I was not able to find something in core that does this already. All the calls to rebuildContainer I found are part of tests.
Awesome @petiar! Many thanks.
@sboden Thanks for the bug report. I think this is a duplicate of #2952091: Show translatable db log message on the main view →
I tried to replicate this with Drupal 9.5.x and 10.x. In both scenarios, the default value for 'identity' is set to drupal. And this has been the case since 2012 according to git log.
If you submit the form UI for syslog with no identity, it fallbacks to an empty string.
As @mfb said https://www.drupal.org/project/drupal/issues/3333215#comment-14868399 🐛 Return early if syslog configs are NULL to avoid openlog deprecation Fixed the only way this can happen is by either, editing the identity using drush cedit, or by manually removing the identity value from the config file and running drush config sync.
This seems more a won't fix to me than forcing the value of openlog.
The patch looks good, but in my opinion the change record needs to be a bit more verbose on other ways the watchdog_exception needs to be replaced:
Only the 2 first parameters of this function are covered:
function watchdog_exception($type, Exception $exception, $message = NULL, $variables = [], $severity = RfcLogLevel::ERROR, $link = NULL)
There is no example of how to use $link and other errors than RfcLogLevel::ERROR.
Tagging this with Novice → as the instructions to proceed are well defined. I'm here to review and provide guidance if needed.
I had more time to think about item 1. I think we should not make an exception for dblog entries. After researching a bit why the content validator was created it seems a bit dangerous to bypass the checks.
Concerns about disabling the dblog module in a production environment are still valid though. I created this to take care of this functionality: ✨ Provide a way to disable creation of database log entries Needs work
I went over all the pending open threads of the merge request and closed most of them. I left a few open to make sure the provided answer is enough for the original reviewers.
I also checked some concerns I had related caching. It seems entities are not cached but routes are (example route:[language]=en:/admin/reports/dblog/event/10) in cache_data table. This is also happening for the current non entity implementation so I will not attempt to fix this as part of this issue. Maybe is not even an issue.
Most of the concerns regarding performance, indexing and future compatibility with 🐛 Database primary keys can exceed maximum integer storeable (has actually occurred, in watchdog) Fixed are addressed as the underlying system stills uses the same database scheme and insert queries.
My general feeling about the code (as the current dblog maintainer) is: this is pretty much ready to be in. There is some room of improvement in the DblogEntryViewBuilder, but probably qualifies for a follow up. And comparing this with the current code of the dblog module I would say this is a big improvement in code quality and alignment of Drupal standards. And will also allow to close several postponed issues for more than a few years now.
The remaining steps in my opinion are:
- Reviewing the latest changes to ContentUninstallValidator::validate
- Review if all annotations defined in DblogEntry are correct, or if something is missing or redundant.
- Get some last code review round paying specially attention to the high level picture of this. I already did this a few times, but I wrote most of the code, and doesn't feel right to assume it is right.
as this is calling a new API method: ContentEntityTypeInterface;:requireDeleteContentBeforeUninstall which needs some grammar and common sense checking.
I updated the issue summary based on the latest reviews assuming the proposed method names are ok. Removing needs profiling as I did the profiling tests in #61.
I updated the issue summary to describe the proposed solution.
Upgraded to work with Drupal 10.1.x. The lines from patch #49 regarding core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php now seems included in another file core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php.
I think I also addressed comment .3 from #48 🐛 Database primary keys can exceed maximum integer storeable (has actually occurred, in watchdog) Fixed
@smustgrave This is what I have in mind. Something similar to what I mentioned in #8.
Using this approach, the signature of the Logger is not affected.
There is one test that is failing for me locally, I'm not sure how to fix it. Maybe @mfb can figure out why.
1) Drupal\Tests\syslog\Functional\SyslogTest::testSettings
Symfony\Component\DependencyInjection\Exception\RuntimeException: Unable to dump a service container if a parameter is an object without _serviceId.
/var/www/html/core/lib/Drupal/Component/DependencyInjection/Dumper/OptimizedPhpArrayDumper.php:436
/var/www/html/core/lib/Drupal/Component/DependencyInjection/Dumper/OptimizedPhpArrayDumper.php:316
/var/www/html/core/lib/Drupal/Component/DependencyInjection/Dumper/OptimizedPhpArrayDumper.php:230
/var/www/html/core/lib/Drupal/Component/DependencyInjection/Dumper/OptimizedPhpArrayDumper.php:134
/var/www/html/core/lib/Drupal/Component/DependencyInjection/Dumper/OptimizedPhpArrayDumper.php:75
/var/www/html/core/lib/Drupal/Core/DrupalKernel.php:935
/var/www/html/core/lib/Drupal/Core/DrupalKernel.php:810
/var/www/html/core/lib/Drupal/Core/Extension/ModuleInstaller.php:600
/var/www/html/core/lib/Drupal/Core/Extension/ModuleInstaller.php:236
/var/www/html/core/lib/Drupal/Core/ProxyClass/Extension/ModuleInstaller.php:83
/var/www/html/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:465
/var/www/html/core/tests/Drupal/Tests/BrowserTestBase.php:545
/var/www/html/core/tests/Drupal/Tests/BrowserTestBase.php:364
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
I updated the issue summary to answer @catch concerns regarding indexes. I ran a `DESCRIBE watchdog;` command before and after installing the site using this patch, and the table structure remains the same.
I guess this also is valid for the issue 📌 Make dblog entities Postponed , if we fix this in the dblog.install module, the changes will also apply to dblog as entities.
I also addressed the comments about dblog exception in ContentUninstallValidator. Based on my current understanding of the module_installer logic, there is no way for dblog to by pass this content validator exception, so I had to modify it any way. I tried to make it as simple and clear possible, and not only for dblog.
🐛 Schema::changeField() has bug when changing regular serial field to big serial field Fixed was merged for D10 and I just provided the backport for D9.5 I think this is now unblocked.
Rerolled for Drupal 9.5.x.
core/tests/Drupal/KernelTests/Core/Database/DriverSpecificSchemaTestBase.php has a lot of new lines in D10 than D9.5. I hope this doesn't break anything...
I'm back to work on this.
I faced this issue today. I seems to be a problem when installing and removing submodules which provide libraries. This issue is not automatically fixed by clearing the cache. I'm also not using Layout builder, so the workaround described in #2 🐛 Can't add node that has chart field on it, get WSOD Closed: outdated does not work for me.
In my case I installed the charts module. Then I created a chart, then I installed charts_google, and when I visited the old chart I got the described fatal error.
Fixed in version 1.0.1
dagmar → created an issue.
I would like to review this as well. A bit concerned about the backward compatibility.
Fixed in release 1.0.0
@smustgraveI think what @mfb is trying to say is, send the 4xx code as part of the $context array. Because it is hidden behind the generic client error.
Thanks
Thanks I had to rollback some untended changed but the issue is fixed now. Will be part of beta4.