@solideogloria

Given #309040: Add hook_requirements_alter() is in core since 9.5.x and up, seems the update_notifications_disable contrib could be ported to use that mechanism to suppress or alter the warning, not the problematic methodology it used per the summary here.

I’d strongly be in favor of closing this as “works as designed”

I disagree. The use of that contrib module is not the only use-case where the warning occurs. It also occurs if you actually disable the update module, for example if the server doesn't have access to the Internet, and the check for updates creates network connection errors in the logs every time cron runs. That's my use-case.

I would like a stable release. Are there any blockers?

Is this issue still relevant? The latest release says it added support for views.

If so, please create a Merge Request instead of using patch files.

The fixes are already in the branch now.

Looks like a lot of great code cleanup!

solideogloria → made their first commit to this issue’s fork.

Ah, I see. Thanks.

Thank you for saying so!

Also, the link above doesn't filter to only my issues, for some reason. Here's a working link:

https://www.drupal.org/project/issues/search?submitted=solideogloria&sta... →

Looks great!

My quick guess is that the IF statement needs to be entered for more cases now, in order to track the middle days during a multiday event, so that concurrent multiday events get properly "pushed down", rather than displaying on top of each other.

It wasn't the newly merged-in changes that added $day_no, it's been there for the last 9 years, however, it does need to be removed for the existing changes in this issue to work.

Personally, I don't see why it was added, as it makes it so that any multiday event on the month calendar that isn't on the first day would be skipped over by the IF statement.

Or, if it was needed from another issue and the rebase, then we need to dig further to determine the proper change.

After looking on a whim, the issue is the addition of && $day_no === 1. Please remove that change.

The changes seem to break normal multiday events, even without concurrent ones. I'm going to look and try to see why.

There's several phpcs changes that are needed to pass code review.

The changes are so complicated mostly because the module puts too much functionality into a single file and few functions. If it were broken up into smaller functional pieces with fewer massive loops, it would be easier to change.

(The massive loops make it so a single indentation change or added level causes all the lines to be affected in the patch, making it hard to read.)

The alternative is to find a different way to track whether each day has an event. Currently, the addition of "cell slots" and the completely new functionality for event tracking is what causes the most changes to the file, but it's necessary because the preexisting method doesn't do multiday events correctly.

Unfortunately, it can't be broken down. The functionality is broken unless all that is changed. The other lines are basically to keep the lines fitting on the screen after the changes and pass code review. The .gitignore changes can be left out if desired, but the only way to fix the functionality is to include the complete changes.

In addition, after creating a custom user cancellation handler, there's no way to select that method when using drush user:cancel. It'd be nice if there was a --method=[method] argument.

I agree. This functionality should be in Core.

I'm not on D11 yet. That's why I have to use 3.1 not 3.2

solideogloria → created an issue.

Looks good to me. I've been using the changes for quite a while.

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.

✨ Make Masquerade::getMasquerade public Active

Should we close the other as a duplicate? It's also using a patch file, when a MR should be used.

📌 Replace js message with webcomponents Active

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.