Montreal, Canada
Account created on 29 June 2010, almost 14 years ago
#

Merge Requests

Recent comments

🇨🇦Canada fengtan Montreal, Canada

fengtan changed the visibility of the branch 11.x to hidden.

🇨🇦Canada fengtan Montreal, Canada

We are working on this issue at DrupalCon Portland 2024

🇨🇦Canada fengtan Montreal, Canada

Fixed in 23ae4d8d. Thanks!

Somehow I did not seem to have permissions to push to the issue fork (search_api_fusion-3413577) so I had to push directly to 2.0.x.

🇨🇦Canada fengtan Montreal, Canada

Pushed 2c5f9721 along with 5d9c6851 which makes the module incompatible with drupal 9 since Error::logException() was only introduced in drupal 10 https://www.drupal.org/node/2932520

Made new RC https://www.drupal.org/project/search_api_fusion/releases/2.0.0-rc1

🇨🇦Canada fengtan Montreal, Canada

I have scanned components 8.x-2.4 with Upgrade Status . The report shows 4 errors, which are all addressed by MR !35:

Thanks! Attached is a static copy of MR !35 so it can be easily referenced in composer.json.

🇨🇦Canada fengtan Montreal, Canada

Patch does not apply anymore against 8.x-3.x -- re-rolled

🇨🇦Canada fengtan Montreal, Canada

Confirmed this patch brings supports for Drupal 10 and works as expected.

Any chance this can be rolled into a new stable release ? Drupal 9 is end of life since Nov 1st 2023 https://www.drupal.org/docs/understanding-drupal/drupal-9-release-date-a...

🇨🇦Canada fengtan Montreal, Canada

#3 works fine -- thanks!

With regards to setting the urlPrefix to https://x.com instead of https://www.twitter.com/ -- I believe this would prevent users from re-saving existing URLs (they would have to update the links from x.com to twitter.com upon saving the form), which may be annoying.

Also x.com currently redirects to twitter.com so people may have a hard time generating URLs with x.com.

An alternative is to create a new, distinct social network X on top of Twitter so end users would be able to choose whether they want their link to be rendered with the bird icon or with an X. The domain constraint would match the network they pick.

🇨🇦Canada fengtan Montreal, Canada

Okay, I have added a version check to both /admin/reports/status and /admin/reports/redis.

From what I can see, ACL's are supported by:

As far as I can tell ACLv2 brings server-specific enhancements (mainly selectors and key-based permissions), but does not introduce incompatibilities with regards to how the client connects to the server. In short this patch brings support to both ACLv1 and ACLv2.

🇨🇦Canada fengtan Montreal, Canada

Okay! Feel free to transfer ownership. Thanks!

🇨🇦Canada fengtan Montreal, Canada

I tested both patches (against 5.x and 6.x) and they both worked.

🇨🇦Canada fengtan Montreal, Canada

Embedding patches from merge requests !7 and !75 so they can be referenced in composer.json files.

🇨🇦Canada fengtan Montreal, Canada

I have added automated tests. Note that the full suite of tests is unlikely to pass as 1.0.x-dev already shows 6 failures without this patch:

🇨🇦Canada fengtan Montreal, Canada

Here is a proposed patch.

🇨🇦Canada fengtan Montreal, Canada

fengtan created an issue.

🇨🇦Canada fengtan Montreal, Canada

I ran into this as well and ran this script to set the missing labels:

    $bundles = \Drupal::service('entity_type.bundle.info')->getBundleInfo('site_setting_entity');
    foreach ($bundles as $bundle_machine_name => $bundle_value) {
      $this->database->update('site_setting_entity_field_data')
        ->fields(['name' => $bundle_value['label']])
        ->condition('type', $bundle_machine_name)
        ->isNull('name')
        ->execute();
    }
🇨🇦Canada fengtan Montreal, Canada

Thanks! Merged !1.

webform_submissionref.tokens.inc includes a similar case so I have pushed 1e3e77d1 as well.

🇨🇦Canada fengtan Montreal, Canada

Failpatch failed as expected -- now here is the full patch, let's see if it succeeds...

🇨🇦Canada fengtan Montreal, Canada

Thanks for the review @quietone -- I reached out to #migration in slack and came up with a failpatch that includes a migration test. Moving to NR to check if it fails...

🇨🇦Canada fengtan Montreal, Canada

Updated patch to accommodate automated tests.

🇨🇦Canada fengtan Montreal, Canada

Somehow the bot says a test about conditional access is failing. The changeset added in #10 only addresses a messenger deprecation so it is not clear how this is related.

Here is an attempt to resolve this, let's see what the bot says...

🇨🇦Canada fengtan Montreal, Canada

Patch #2 works fine on our side. Thanks!

There is one limitation though: the form shows no success or error message when submitted.

This is because the patch is using drupal_set_message() which is now deprecated https://www.drupal.org/node/2774931

Attached is a new patch that replaces drupal_set_message() with \Drupal\Core\Messenger\MessengerInterface::addMessage() as recommended by the change record above.

🇨🇦Canada fengtan Montreal, Canada

Thanks for reviewing! Here is a re-roll against 2.0.x...

🇨🇦Canada fengtan Montreal, Canada

+1 that would be handy !

🇨🇦Canada fengtan Montreal, Canada

Thanks !

🇨🇦Canada fengtan Montreal, Canada

Thanks for the quick reply !

🇨🇦Canada fengtan Montreal, Canada

Here is a proposed patch that works for us.

Production build 0.69.0 2024