Problem/Motivation

According to Change Record: New parameter added to \Drupal\Core\Form\ConfigFormBase::__construct ConfigFormBase::__construct() now has a required second parameter. This module's config form throws an error currently on Drupal 11 because this second parameter is not provided.

CommentFileSizeAuthor
#2 3563519-configformbase-required-param-2.patch1.3 KBbaysaa

Issue fork citation_select-3563519

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

baysaa created an issue. See original summary.

baysaa’s picture

baysaa commented
StatusFileSize
new 3563519-configformbase-required-param-2.patch1.3 KB
baysaa’s picture

baysaa commented
Issue summary: View changes

santanu mondal made their first commit to this issue’s fork.

santanu mondal opened merge request !8

santanu mondal’s picture

santanu mondal commented
Status: Active » Needs review
aryan-r-patel’s picture

aryan-r-patel commented

I am curently on Drupal 10.6.2.
I applied the patch locally and when I cleared the cache, I receive the following error:

/var/www/drupal # drush cr
PHP Fatal error:  Type of Drupal\citation_select\Form\SettingsForm::$typedConfigManager must not be defined (as in class Drupal\Core\Form\ConfigFormBase) in /var/www/drupal/web/modules/contrib/citation_select/src/Form/SettingsForm.php on line 22
 [warning] Drush command terminated abnormally.

Removing the "protected" access modifier seems to solve this. Are you able to reproduce this on your end as well?

baysaa’s picture

baysaa commented

@aryan-r-patel this change is for Drupal 11+

aryan-r-patel’s picture

aryan-r-patel commented

@baysaa Thank you for your reply. However, we must ensure that the module remains both compatible for Drupal 10 and 11 for now, as most of our sites are on Drupal 10, and we cannot merge a patch that causes a Fatal Error.
Could you please update the patch if possible so that the code is compatible with both Drupal 10 and 11?
This will allow us to make this change for Drupal 11 without breaking on Drupal 10.

baysaa’s picture

baysaa commented

@aryan-r-patel I think I see the issue now sorry. I've removed "protected" from TypedConfigManagerInterface in the SettingsForm constructor can you try again. This should fix the issue

https://git.drupalcode.org/project/citation_select/-/merge_requests/8.diff

aryan-r-patel’s picture

aryan-r-patel commented

No worries!
Thank you for updating the Merge Request. It is working now on Drupal 10!

kylehuynh’s picture

kylehuynh commented
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.