🇨🇦Canada @dtarc

Here's the patch.

dtarc → created an issue.

Here's a rerolled patch for thos still on 1.0.x.

Hi, we were setting defaults in the permissions by term field when creating new users and this broke our code.

In the user form, splitting up `$form['access']['terms']` by vocabulary was probably an unnecessary change that didn't need really need to happen.

Just making a comment here so that others might find something when looking. It's a hard problem to find if you don't know what's going on.

Anyway this was a breaking change.

dtarc → created an issue.

I don't think this is works as designed, I think it really does require a patch doing what aws outlined in #2

We're extending the class, but we are not overriding the constructor. We're extending the class in order to get more functionality out of it. I think one of the big reasons that Drupal moved to an OO structure is to allow extending classes like this.

I can submit patches to add that functionality to MessageController if you're friendly to that. I've already added one such patch in #3403215.

Is there a good reason to declare these classes final? I don't understand the logic behind that.

We've been using this funcationality for a while now by extending MessageController class. With 8.x-1.1 release, there is an API-breaking change with setting MessageController to be final, ie. not extendable. If this class is going to remain final then we really need this change to go in, which should not affect site builders. Thanks.

Hi,

We are extending MessageController class to get extra functionality. The recent update from 8.x-1.0 to 8.x-1.1 is causing code to break because all the classes have been declared final.

Code changed from

`class MessageController extends ControllerBase implements ContainerInjectionInterface {`

to

`final class MessageController extends ControllerBase implements ContainerInjectionInterface {`

Is there a reason for this API-breaking change? It seems unnecessarily strict to prevent these classes from being overridden.

Hi,

We ran into an error on updating from 8.x-1.0 to 8.x-1.1 because the constructor for MessageController changed.

It went from:

public function __construct() {

to:

public function __construct(EntityAccessControlHandlerInterface $access_handler, FormBuilderInterface $form_builder) {

Is there any reason for this API breaking change in a minor point release? Thanks.

Updated patch with updated @param values in method annotation.

Here's the patch.

dtarc → created an issue.

dtarc → created an issue.

Here's a patch.

Ugh, one more time.

I missed a line in patch in #2, here's the new version.

Here's the patch.

dtarc → created an issue.

Here's the patch.

$conditions is not a useless param -- it's passed into bat_event_get_states() where it is used. Please commit this patch.

We had a similar issue with this module "just not working" with no error message or anything else.

The issue for us was that is the number is invalid, ie. the area code doesn't exist, then the formatting doesn't happen, and no error message is output anywhere. This makes it very confusing. We sanitize our database for dev work, and our sanitization process just randomizes the phone numbers, which led to invalid phone numbers.

I don't know if this was the same issue the OP had.

If the number isn't valid then an exception is thrown and then caught here in this funcion:

  public function viewElements(FieldItemListInterface $items, $langcode) {
    $element = [];
    foreach ($items as $delta => $item) {
      try {
        if ($this->getSetting('link')) {
          $element[$delta] = $this->viewLinkValue($item);
        }
        else {
          $element[$delta] = $this->viewFormattedValue($item);
        }
      }
      catch (\Exception $e) {
        $element[$delta] = $this->viewPlainValue($item);
      }
    }

    return $element;
  }

A watchdog message should be logged if the exception is thrown.

Here's the patch.

dtarc → created an issue.

Here's.a patch with the fix.

bat_event_get_states can take an optional 2nd parameter so it seemed to make sense to pass $conditions along to it.

dtarc → created an issue.

I'm re-opening this issue because the fix has caused bat events showing on full calendar to lose their color.

The problem with the fix is that when the bat event is loaded, if it has a calendar label then `$state_info` is never refreshed from the loaded event.

Here's a small patch that fixes this by making sure `bat_event_load_state()` is called again with the loaded bat event.

OP = original post (or original poster)

dtarc → created an issue.

That Documentation should be moved to drupal.org.

Yes this module needs some maintenance. Please allow afagioli to co-maintain.

The patch in #2 works for Drupal 10 compatibility. Please see Upgrade Status screenshot.

Please commit.

The OP is listing the requirements from the documentation at https://roomify.github.io/docs/bat/drupal/requirements.html (see screenshot).

It seems like the OP was expecting to see these modules found in a composer.json file, maybe as requirements or suggestions.

These Drupal 8.x requirements need some work though. Drupal 8 uses Facets, not Facet API. Views Megarow does not have a Drupal. 8 release.

So course of action would be:

1. In Documentation, update 8.x Requirements:
- Remove Views Megarow
- Replace Facet API with Facets
- Determine whether any other changes need to be made

2. Consider putting these requirements in composer.json under "require". Patch is needed.

bbrala, it's possible to make a patch directly from a MR by appending ".patch" to the url.

So in this case the url is https://git.drupalcode.org/project/redirect_after_login/-/merge_requests...

This Drupal 8 / 9 / 10 module can be included in composer by adding the following to composer.json in repositories:

        {
            "type": "vcs",
            "url": "https://git.drupalcode.org/issue/ejectorseat-2410423.git"
        },

And then run:

composer require drupal/ejectorseat:dev-2410423-ejector-seat-drupal

I have tested Merge Request !1 from #5 and it works great! Also it's compatible with Drupal 10.

The only problem is that the MR is against 7.x-1.x, and instead we'll need a new branch for 8.x-1.x.

Please commit this work to a new branch and release a Drupal 8 / 9 / 10 version.

I tested the Merge Request !4 from #12 using the method from #18 to update composer. It works great! Please commit & release.

The patch from #7 works when applied directly but it does not work when added to composer. This is because there is a `version` property in `redirect_after_login.yml` that should not be there, and it gets commented out by the drupal packaging script. So then the patch will not apply from composer.

The merge request is ready and does the job, please commit and release for Drupal 10 compatibility.

Here's a new patch with all the work from #2 re-rolled.

Also here's a screenshot from the Upgrade Status module with this patch applied.

dtarc → created an issue.

Here's what we've been doing so far.

dtarc → created an issue.

That merge request would put the changes into 7.x branch, I think we'd need a new branch for 8.x or 1.0.0.

Updating the patch from #1 to replace `core: 8.x` to `core_version_requirement: ^8.9 || ^9 || ^10` in `ejectorseat.info.yml`.

This is a pretty useful module with high usage for 7.x. I don't know if there's any alternative for D9 but it would be great to get a D9 branch up.

Here's a patch for the change in #2. The patch in #5 doesn't match.