Note to committer, please also add credit for danchadwick → , who posted code changes in the duplicate issue that came before this one.
Closing as duplicate of issue with a better description and a MR instead of a patch. Please review/test the change on that issue, and you should get issue credit there as well.
Should we close the other as a duplicate? It's also using a patch file, when a MR should be used.
I was talking about whether we have content entity types that allow non-numeric string ids like 'xyz'.
It might be doable in contrib. It depends on what the type is in the database for the ID. I'm pretty sure Core only uses integers.
If you look at EntityBase.php, it does currently rely on the implicit string type with mb_strlen($this->id()):
public function preSave(EntityStorageInterface $storage) {
// Check if this is an entity bundle.
if ($this->getEntityType()->getBundleOf()) {
// Throw an exception if the bundle ID is longer than 32 characters.
if (mb_strlen($this->id()) > EntityTypeInterface::BUNDLE_MAX_LENGTH) {
throw new ConfigEntityIdLengthException("Attempt to create a bundle with an ID longer than " . EntityTypeInterface::BUNDLE_MAX_LENGTH . " characters: $this->id().");
}
}
}
I wonder if this could throw an error, as it doesn't handle the case where the ID is NULL? Maybe that can't be the case on preSave. But it doesn't currently handle the integer case.
Not sure if there are content entity types that have string ids, or config entity types with int ids.
A lot of standard entities, such as nodes and taxonomy terms, return string IDs.
Looks good to me.
Speaking of the showcore route, there's this issue
For anyone updating to alpha6, the MR's .diff file does not apply, but the .patch file DOES apply.
The issue is marked Needs Review. Please test and review the changes in MR !18.
You can download the code changes as a patch/diff, and include the downloaded file in your Composer patches file.
Does a separate issue need to be opened for removing the submodule?
If you're still using this module, you should switch to a supported module.
@anybody How would we modify the form elements to allow relative dates?
$form['date_range']['max_date'] = [
'#type' => 'date',
'#title' => 'Enter Max date',
'#format' => 'm/d/Y',
'#description' => t('i.e. "09/06/2016" or "+1 year"'),
'#default_value' => $this->options['date_range']['max_date'],
'#attributes' => [
'min' => \Drupal::service('date.formatter')->format(\Drupal::time()->getRequestTime(), 'custom', 'Y-m-d'),
],
];
$form['date_range']['min_date'] = [
'#type' => 'date',
'#title' => 'Enter Min date',
'#format' => 'd/m/Y',
'#description' => t('i.e. "09/06/2016" or "-1 year"'),
'#default_value' => $this->options['date_range']['min_date'],
'#attributes' => [
'max' => \Drupal::service('date.formatter')->format(\Drupal::time()->getRequestTime(), 'custom', 'Y-m-d'),
],
];
Rebased
No changes are likely going to be made for Drupal 7, since that version of Drupal is no longer supported.
Me neither. Maybe it was fixed upstream in Masquerade?
Could you also look at and merge ✨ Support masquerade module versions later than 2.0.0-rc4 Active , so that the module is compatible with the most recent and secure version going forward?
Changes need to be applied to the Merge Request, rather than as a patch.
For some reason, "n/a" doesn't get displayed if there is no "most recent post". Not sure why. There must be some limitation on what Twig logic is allowed in views?
The main forum listing is done. If someone else wants to look at it and see if it looks good, this view could replace the current functionality. Then work can be done on the next piece if desired.
I used to use Mime Mail. It renders HTML tags even if given as plaintext HTML, I think, as it can guess the intention. However, the module's behavior ought to be consistent. The configuration isn't collecting or storing the template in a way that suggests HTML.
Also, I do think it would be helpful for Drupal Symfony Mailer to be recommended on the module page. The module is a also drop-in replacement for Swift Mailer, which is unsupported now due to SA-CONTRIB-2024-006 →
The patches works as intended (it fixes the issue for me).
See the related issue for supporting HTML.
solideogloria → created an issue.
This will break websites that use HTML in comment bodies of "Default mail text for sending out notifications to commenters" or "Default mail text for sending out the notifications to entity authors", which includes some of our websites.
As it was possible to put HTML there so far, I think it is reasonable to expect this to continue to work.
That's not true. I just installed the module and put in HTML tags, and they just show up as tags/text in the email. I'm using Drupal's recommended Drupal Symfony Mailer → module.
Use Vanilla JavaScript. For sessions, see these pages:
https://drupal.stackexchange.com/questions/278686/what-is-the-difference...
https://api.drupal.org/api/drupal/core%21core.api.php/group/session/11.x
solideogloria → created an issue.
I will also note that this functionality can be done with the Flag module. If you hover over Drupal.org's subscribe link, it's using a flag endpoint, such as https://www.drupal.org/flag/flag/project_issue_follow/3021537?destinatio... → ...
The patch/code needs to be rewritten. It's using really old code such as db_select() which doesn't exist anymore.
It also needs to be converted to a Merge Request.
You should avoid using jQuery, as Drupal Core is working to remove it.
I added patch #39 to the MR, then made some initial fixes and re-exported config.
None of the existing patches are against the contrib module.
#4 worked for me in Drupal 10.4.
The view does not show any results if the forums do not have any containers as parents. Containers are not required, so the Parent view relationship should not be required.
Looks good to me.
solideogloria → created an issue.
Ideally, you wouldn't need to do any setup. You should just be able to use DDEV to install the modules and spin up a site, then run ddev phpunit from https://github.com/ddev/ddev-drupal-contrib to run all the unit tests.
Please also add the related issue that is blocking this one as a "related issue"
Review the changes now. I added more tests for more use cases.
Same. I don't have that use case personally, so I don't really know either way, to know which is better.
I think that argument makes sense, though. The tests will also need to be updated. Specifically this part, since we would be changing it to not be affected if there were no query params given but the route restriction has query params specified:
$this->request->getMethod()->willReturn('GET');
$request_affected = $this->restrictIpService->requestAffected();
$this->assertEquals(TRUE, $request_affected, 'Request affected for method match and empty params.');
Did you try the patch from the MR? The MR uses if (!$params || !$query_string) {, which should also work.
Ah ok. So should the status be Postponed, since it's waiting on another MR?
Confirmed that it's broken as-is.
By the way, it'd be awesome if you could review 🐛 LogicException when logging out while masquerading Needs review
Confirmed that the functionality is broken, and that this MR fixes it. Thanks.
The module literally doesn't work without this fix.
The contrib module doesn't work with the most recent version of Masquerade right now.
The unit test is still failing.
solideogloria → created an issue.
solideogloria → created an issue.
All that was needed to rebase is to type /rebase in a comment on this page
https://git.drupalcode.org/project/webform/-/merge_requests/585
Also, credit has not yet been assigned to users for this issue itself and should be added.
FYI: The release notes are missing issue credit for this issue.
Your issue is that #25 is not the most recent patch. You have to use the merge request patch/diff
No, the patch is not already included. You might have a conflict with another patch you are using. I was able to successfully use the patch with 1.0.0-beta3 without any issues.
Look at the changes in the MR. It was broken in 1.0.0-beta3 for me, so the MR needs to be merged.
PHPCS needs to pass.
I discovered the change in 🐛 Typo in the calendar-month-col twig template Active broke my TS. Once I fixed that, I can test the changes here.
The changes work great, as far as I can test. My test does not cover the entirety of the functionality changes, but I'm happy with what I use and see.
I'm going to see if the changes work with my site. I have to adjust some custom TypeScript to work with the changes, I think, for me to test it.
Shorten title more
solideogloria → changed the visibility of the branch 8.x-1.x to hidden.
solideogloria → changed the visibility of the branch 3255924-multiday-rendering-fix to hidden.
The function is already fixed in 1.x, it seems.
@cmlara Very interesting article. I had no idea WordPress did that. I definitely agree that Drupal should only collect what it needs. I think that if users are given more granular control over which data is sent, they are more likely to allow it.
The changes need to be applied to the merge request.
It seems like it was a misunderstanding of what needs to change. While EntityInterface $source_entity = NULL is deprecated due to not having the nullable type, there is nothing wrong with ?EntityInterface $source_entity without a default value, so it should be left as-is.
PHPUnit failed in the pipeline. Is this the cause?
The function doc comments need to be updated to match the changes.
Looks good to me. It's a simple change/fix.
Ah, right you are. Thanks
Looks good to me.
Looks good to me.
Looks good to me.
The branches in the MR look incorrect. See the documentation for how to do this properly.
https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... →
They must be already fixed in the dev branch, then.
I looked at the diff: https://git.drupalcode.org/project/search_api/-/merge_requests/208/diffs
It doesn't have a single changed method signature to add the nullable to the type.