Confirming that enabling the file_delete module fixes an issue.
Faced the same issue, the patch from #9 worked well.
Moving to RTBC.
Created the MR with D11 support fix.
Like with
💬
Drupal 10 Support
Needs review
, there's no need for other fixes except the info.yml file.
Opened the MR to fix this.
abramm → changed the visibility of the branch 3535811-drupal-11-support to active.
abramm → changed the visibility of the branch 3535811-drupal-11-support to hidden.
Created the MR.
Turned out the library is already correctly attached in the block code:
$pcp_markup = [
...
'#attached' => [
'library' => ['pcp/pcp.block'],
],
];
return $pcp_markup;
The library name in the info.yml file is incorrect (pcp/pcp-block instead of pcp/pcp.block), which explains why grepping didn't show anything for me.
So it's just two lines in the info.yml file which should be removed.
abramm → changed the visibility of the branch 3535809-incorrect-library-attachment to active.
abramm → changed the visibility of the branch 3535809-incorrect-library-attachment to hidden.
Made the MR with a fix.
Follow-up issues:
-
🐛
Tombstones URLs are only created for English paths
Active
-
📌
Use dependency injection
Active
Created the MR.
The fix should be simple, but it's worth waiting for 🐛 Missing closing div element in node--tombstone.html.twig Active to be merged to avoid merge conflicts.
Created the MR.
Created the MR.
Like to try a solution that doesn't require checking core version if possible.
That's exactly what's done in #12.
Do you want me to create the MR?
There is no assurance that patch files do always exist on d.o
Well, in that case, I just hope it will live long enough until the release 😅.
That's a fair point, though.
@abramm Do you know that this is bad practice and you should never grab patches from d.o with composer? You should always download the patches locally and refer to them there.
I'm not seeing any problem with patches from Drupal.org. It's more a question of preference; in any case, someone could download the patch and commit it locally if they prefer.
(Using patches from d.org MRs is definitely a bad practice, though.)
Here's a patch made from both commits fixing the issue.
Posting it in case someone needs to apply it via composer (and doesn't want to switch to dev version).
While the decision to hide facet sources for default displays is reasonable (especially taking into account exposed views facets), I found this caused an issue with a website heavily using default displays.
Attaching a patch in case someone needs to re-add default displays temporarily (until all Views/facets are converted to not use default displays).
I've faced the same issue, but I've come to a different solution before I've seen the MR.
Instead of trying to replicate the core behavior, my patch simply extracts the class name produced by core.
Attaching a patch with a workaround.
#25 is missing one use statement, here's the right one.
Re-roll of #24 for 11.1.x.
Hi @chi,
Yes, that could be another workaround. The usage may be limited, though.
Submitted the MR, attaching a patch for composer.
Attaching a patch made from MR !154 (second try approach) for applying via composer.json.
Here's a reroll of #14 for those of us upgrading to D11.
Hi ahmed.raza,
Can you please add an issue credit?
Thanks!
The auto-generated patch doesn't apply cleanly on top of the 1.3.0 release version since the Drupal.org packaging script adds a few extra lines at the end of the info file. That's expected.
Attaching a patch that applies over the packaged 1.3.0 release.
Pushed the fix to the 3431055-Drupal-11-Support branch (MR !10). I can confirm the settings form works as expected now.
Attaching a patch to ease applying via Composer.
I can confirm the issue with the latest state of the 3431055-Drupal-11-Support branch. The $typedConfigManager property of the class shouldn't be overridden or set as it's already defined and set in the parent ConfigFormBase class constructor.
D10: https://git.drupalcode.org/project/drupal/-/blob/10.4.x/core/lib/Drupal/...
D11: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...
Patch for applying via composer.json.
Here's a diff file from @srjosh's MR #5 for applying via Composer.
Retrieved from https://git.drupalcode.org/project/kaltura_media/-/merge_requests/5.diff
Patch for applying via Composer.
Hi, not sure what you mean by 'For some reason' - can you please explain why the change is needed?
Hi @mably,
Welcome aboard! I've added you to the maintainers list.
The module is not commonly used nowadays since the domain module itself now provides an ability to switch the theme, however, I think it may be still useful to make a D11 release for those who are migrating older sites.
LGTM.
Keeping assigned to @mably for the release.
Yes, that's what I mean; no one has expressed an interest in this module since 2022.
Actually, I'd say it's no longer needed as we developed it years ago when Commerce didn't have BOGO.
We're currently not looking for a co-maintainer.
As for D10/D11 support, there's an automatic patch, but no one seems to have checked it - probably just no one is interested in that.
I don't think this is an issue.
The module assumes that import and export operations are done on the same site or between sites with identical configurations.
I've pushed a fix for the issue outlined in #3 (hope @ras-ben doesn't mind; otherwise, feel free to remove/squash my commit),
Attaching a patch to apply via composer.json.
The MR doesn't correctly account included and excluded dates; that's likely because include/exclude logic is handled in the recurring_events_event_instances_pre_create hook implementation (recurring_events_recurring_events_event_instances_pre_create_alter). This causes an issue with the removal of instances that are no longer in the range.
The proposed code change calls this hook only for events instances to be created while originally it was called for *all* instances.
Steps to reproduce:
1) Create the weekly event series, starting from e.g. the beginning of the year and ending at the end of the year.
2) Observe 51 instances (or whatever the number of weeks is in range) created.
3) Change the included date range to include only one month from the range, save the series.
Expected: all instances except those in the included range are deleted.
Actual: none of the instances are deleted.
Note: I'm talking about the Excluded dates and Included dates fields, which are below the normal range selection.
The change looks reasonable, although it'd be great if there would be a test and require-dev (as mentioned) before the merge.
Also, the patch should be changed to MR to allow tests run.
Here's an MR changing default behavior.
The proposed MR allows logging in with email stored in the username.
I checked the MR, added a minor suggestion.
Well, if the field type comes from something that's published on Drupal.org and is maintained, we could easily merge in the patch. Everybody wins.
Hi cnfl,
Plugin derivatives are not a single_content_sync specific feature; they are a Drupal way to use the same class for multiple plugins. You can learn more about derivatives in Drupal development docs. Due to this reason, it's not documented in the module.
We're using derivatives to use the same class for multiple field types plugins; e.g. it would be stupid to have separate classes, say, for string and number field types if their code would be 100% identical.
You don't necessarily need to use derivatives for your plugin; that depends on your the field type(s) you're working on. Is it a custom field type or something from Drupal.org ?
Hi @koosvdkolk,
You're right, the warning message is outdated; it references the very old hooks logic before we had plugins.
Feel free to submit the MR with a fix/README update.
abramm → changed the visibility of the branch 3202631-add-textarea-option to hidden.
Added two MRs (test only and test + code change), let's see if tests still passes.
The patch in #34 doesn't apply to 10.3, so re-rolled it against both 10.3.x (for those of us who need this patch to be applied to their sites) and 11.x branches.
Also, removed the unrelated change and split patches to test and test+fix.
Attaching a quick fix patch (also pushed to the MR branch).
Just spotted the edge case: the proposed RegExp solution will likely fail if there's a matching pattern inside the path (which is weird and I can't imagine anyone doing this, but technically it's still a valid URL).
const regex = new RegExp('(?<=//).*@(?=(?:.*.)+[^.]+)');
"https://valid:url@drupal.org/somewhere/".replace(regex, '');
// returns 'https://drupal.org/somewhere/' which is fine
"https://drupal.org/some//really:weird@url/".replace(regex, '');
// returns 'https://url/' which is not correctMaybe an extra negative lookahead could handle this?
This change has caused a regression in our application.
I believe this is an edge case that's specific to our project (and a result of bad route argument naming), but I'm noting it here in case it would be helpful to anyone else.
We have a custom form controller and a route defined like this:
my_path:
path: "/my-url/{some_param}"
defaults:
_controller: '\Drupal\my_module\Controller\CustomFormController::build'
form_id: 'Drupal\my_module\Form\MyModuleFormClass'
Due to the change in this commit, the form_id may be now fetched from the request attributes (\Symfony\Component\HttpFoundation\Request::get), which caused the wrong form_id value to be fetched from the route attributes rather than from the request GET/POST parameters.
We've fixed this by changing the form_id argument to a different name (e.g. my_form_class) to avoid naming interference.
Having multiple exposed forms on the page, drupalSettings would be overwritten by the one rendering last; so I'd stick with the value from the data attribute.
Also, it'd be great if you could raise separate issues for two extra field types you add as they're not related.
Hi jannakha,
Thank you for your contribution!
Can you please also add tests? Please refer to an example here:
✨
Support geofield field type
Fixed
.
I don't think this is a module issue.
As an alternative, try using a development environment that correctly handles the installation of self-signed certificates, like ddev.
Hi Boobaa,
Thank you for your effort!
A few thoughts here.
1) We definitely don't want to have accessCheck(FALSE). The current user may not have permissions to view all revisions; this code will give them read access. This is a security/data disclosure issue.
2) The core version requirement should be core_version_requirement: ^9.3 || ^10, same as in root info.yml.
3) The following line will produce a warning if there are no revisions in YML file:
if ($content['revisions']) {
4) The revision creation time should be import/exported the same way we do for created/changed timestamps.
5) We'd likely need a test for this; also, please create the merge request to run automated tests.
Thanks!
The test is here, merged to 1.4.x.
I've added support for the address_country field type, it already merged to 1.4.x.
As for address_zone, there's an open MR adding support and it works, but needs tests before we could merge it.
Committed to 1.4.x.
Committed to 1.4.x.
Hi @ankondrat4,
Seems like we both pushed a similar code; I'll credit you as well.