Account created on 12 November 2008, about 16 years ago
#

Merge Requests

More

Recent comments

🇦🇺Australia geoffreyr

Thanks Lee! Given you prefer frequent releases, I might look at doing a 2.0.0-beta3 for the issues that are already committed to 2.x, and do additional ones for individual features as you suggested. The CI/CD one would be a good one to get in so we can start testing all changes yet to come.

🇦🇺Australia geoffreyr

Added a Composer requirement for drupal/core matching the info.yml file.

🇦🇺Australia geoffreyr

geoffreyr made their first commit to this issue’s fork.

🇦🇺Australia geoffreyr

MR ready. If you want to set 3.0.1 as being for 10.3 and above you won't want to merge this, but anyone else who finds this issue can use the plain diff source to download it and use it in their sites.

🇦🇺Australia geoffreyr

I've been trying this out with an implementation that uses IEF Table View Mode and so far it's working well. Much more concise and reliable than patching IEF to remove the sort elements. I'm not going to RTBC it until we test and deploy it, but this looks really promising.

🇦🇺Australia geoffreyr

I've tried rerolling #18 against the latest 2.x. It seems to work but I reckon it needs a bit of a look. If anyone else wants to check the branch out and make changes that's all good.

🇦🇺Australia geoffreyr

I've got a case where I have a CSV with a lot of line breaks in the cells that I'd like to convert to HTML <br /> markup (nl2br style). This feature might allow for what I want to achieve, so I'll see if it's possible to render out the cells with a text filter set.

🇦🇺Australia geoffreyr

I've uploaded a patch to let Linkchecker ignore non-numeric content entity IDs when deleting. This is a hack at best so I'm not putting it through as a proper merge request, it's more to let people who use Entityqueue or modules that do similar things with entities use Linkchecker alongside it.

I recommend that we identify and implement a proper solution.

🇦🇺Australia geoffreyr

@vaish Thanks, will do.

🇦🇺Australia geoffreyr

We've been using this one in PRODUCTION for the past two months, so moving to RTBC.

🇦🇺Australia geoffreyr

We've been using #11 in PRODUCTION for some time so I'm also +1 for RTBC.

🇦🇺Australia geoffreyr

MR is in. I should review the existing tests to see if the behaviour is already covered or not.

🇦🇺Australia geoffreyr

We've been observing this same issue. The original MRs no longer apply, and they appear to have no extant use since we're seeing position: fixed applied via JS, presumably from a completely different part of the theme. We'll have to dig into this in a bit more depth to see how the dropbutton styles are applied at runtime.

🇦🇺Australia geoffreyr

We've finally got this implementation into PRODUCTION, so I might look at finishing off the tests so it's ready for review.

🇦🇺Australia geoffreyr

This might be a little more complicated than I'd hoped because there's currently no way to enable or disable specific entity types from being managed by this module. It's all or nothing, which is probably why the admin form is crashing hard all the time.

I might take a look at https://www.drupal.org/project/form_mode_control/issues/3431354 🐛 Config schema is incorrect Active which aims to fix some of the config mapping issues.

🇦🇺Australia geoffreyr

@Mingsong Good catch on the data typing, thanks for making the change!

🇦🇺Australia geoffreyr

Amended issue title accordingly. MR to come shortly.

🇦🇺Australia geoffreyr

@Mingsong No worries, happy to shed some light on the intricacies of this issue. It's not really about the number of matches returned to the autocomplete; it's about the autocomplete text input field itself being hard limited to 1024 chars long. Here's a discussion covering some one-off fixes for the issue. Since EntityReferenceTreeWidget is a child class of EntityReferenceAutocompleteWidget, we'd be overriding the limitation present in EntityReferenceAutocompleteWidget::formElement.

Instead of doing a one-off fix, we figured making the widget more flexible would be a better solution. A patch to the widget would be a good start, and I'm currently working on this.

🇦🇺Australia geoffreyr

We're using 3.1.0 in PROD now and everything looks great. Thanks again for version 3.1.0! Closing as fixed.

🇦🇺Australia geoffreyr

Thank you very much! Very kind of you, it should do the job perfectly. We'll get this version into production as soon as we can.

🇦🇺Australia geoffreyr

Ok, looks like I've fixed the exception throwing.

🇦🇺Australia geoffreyr

Doesn't handle boolean results properly. Need to fix.

🇦🇺Australia geoffreyr

Adding #3441681 as related; I'd like to get this implemented so we can test our changes.

🇦🇺Australia geoffreyr

I think I've worked through most of the code quality issues. I can take a look at some of the other patches that I've been using to work out what to change. There aren't any changes to templates, so most people who've been theming Funnelback pages, blocks and results shouldn't be affected.

🇦🇺Australia geoffreyr

Looks like I've got a lot of code quality fixes to sort out before we can proceed. This will probably affect a lot of other pending patches and MRs unfortunately; might be a big job.

🇦🇺Australia geoffreyr

I'm going to mark this as needing review, as we need this functionality too, and I'd like to see how it goes in our production environment.

🇦🇺Australia geoffreyr

We're using the MR with cache context in production right now. Works really well and haven't seen any issues with cached search queries.

🇦🇺Australia geoffreyr

@larowlan Thanks, I'll give it a go.

🇦🇺Australia geoffreyr

We've been using this patch, but it's had the interesting side-effect of showing other users' unrelated search queries in cached search blocks. Need to dig into this a bit; maybe a custom cache context would help.

🇦🇺Australia geoffreyr

@larowlan Sounds fair enough. My MR is above but I expect it'll need more documentation, a changelog update, etc. Probably depends on when we want to cut a new release.

🇦🇺Australia geoffreyr

A rough first pass indicates that Funnelback module really doesn't depend on Drupal core search at all. No hooks, no config, no permissions, nothing. I'm putting together a patch for myself which I'll soon contribute back here, but we may want to add an update hook to warn module end users that it no longer needs core Search and they should disable it for themselves if they like.

🇦🇺Australia geoffreyr

We're actively looking at using the patch from #52 in production so we've created a MR for it. It also incorporates some minor fixes for D10 compatibility; there may be more adjustments to come. Will look at 2472941 when I have the chance.

🇦🇺Australia geoffreyr

geoffreyr made their first commit to this issue’s fork.

🇦🇺Australia geoffreyr

@abhishek_gupta1 Thanks for your patch. I've submitted a MR which does the same thing but is a little bit more terse.

🇦🇺Australia geoffreyr

I've worked out that we can go to the Advanced tab and set, under Custom Options, something like timeout: 10. This works as intended, albeit that if that timeout is ever exceeded, it throws a fatal error that Webform module can't recover from. It looks like Guzzle is throwing a ConnectException on timeout, which isn't classed as a RequestException. This is also a problem with Webform's own RemotePostWebformHandler, but I don't see a reason why we shouldn't fix this here.

🇦🇺Australia geoffreyr

@sarwan_verma Thanks for submitting your patch. I've done up an MR with this change and also fixed a few uses of this method throughout the module.

🇦🇺Australia geoffreyr

We've been using #18 to adjust the CSP to allow for some rich legacy experiences that take over the page. We're taking configuration from Composer files in separate repositories to adjust the CSP rules for particular page requests -- highly custom but very effective. This patch has been of great benefit to us; and given that the patch provides new tests for this case, and that they're all passing, I'm willing to RTBC this.

🇦🇺Australia geoffreyr

I think this should pass muster, casting to an int appears to round floats appropriately. In this case it doesn't really matter which direction it rounds, but (int) (37 / 2) rounds down to 18. Suggest we do it as a merge request so it can go through all the pipelines.

🇦🇺Australia geoffreyr

The patch for this issue doesn't apply on 10.2.5, apparently because #2825860 🐛 Notice: Undefined index: value in Drupal\views\Plugin\views\filter\NumericFilter->acceptExposedInput() Needs work got merged and directly conflicts with MR 5391. It looks like it should do the same thing albeit expressed differently. Can anyone else confirm?

🇦🇺Australia geoffreyr

Added a change to the WebformMatcher so it implements EntityMatcher::buildPath. This allows us to use the webform's configured alias if it's available.

🇦🇺Australia geoffreyr

geoffreyr made their first commit to this issue’s fork.

🇦🇺Australia geoffreyr

Just pushed an update to the merge request adding an update hook to add the `id` column if it's not already there. This will help users with existing installations.

Sorry about the implementation, it's hacky, but I couldn't find a better way. The issue https://www.drupal.org/project/drupal/issues/3264915 💬 Can't update database: Syntax error or access violation: 1075 Incorrect table definition Closed: duplicate describes a similar problem.
We might need to check other database types (SQLite, Postgres) to see if this method is valid or not.

🇦🇺Australia geoffreyr

We should probably add a hook_update implementation to add the primary key for cases where it does not already exist.

🇦🇺Australia geoffreyr

I had a bit of a go at porting this to D9/10. Not sure that we should call the parameter max_nodes_per_cron anymore, maybe it should be renamed to reference entities in general. publish and unpublish methods take the limit as the command, but default to 0 so the original invocation should continue to work (albeit with no limit).
Will iterate on this when I have the opportunity.

🇦🇺Australia geoffreyr

geoffreyr made their first commit to this issue’s fork.

🇦🇺Australia geoffreyr

MR created. I might have to amend it to update Drupal core version constraints.

🇦🇺Australia geoffreyr

For D10 compatibility this change will need to be compatible with Sendgrid API 2.0. I know that there is a 2.0 version of this module available, but we're using the patch at https://www.drupal.org/project/webform_sendgrid/issues/3369994 🐛 Subscription doesn't accept tokenised values for numeric fields Needs review and won't be able to update that to 2.x until after the upgrade. Will open an issue fork with the patch and work on it a bit.

🇦🇺Australia geoffreyr

I've made a few changes to the Merge Request as follows:

  1. The form save operation didn't vary the bundle key while performing the EntityQuery, ensuring that entity types that didn't use bundle key type would throw an exception. This has been changed to use the bundle key provided by the entity.
  2. Checks to make sure entities only get added to the snapshot queue once.
  3. Ensures that entity update/delete hooks take the new config into account, thereby skipping entities that aren't set to be included in snapshots.
🇦🇺Australia geoffreyr

I think I've finally got the Gitlab CI going and got the module polished up a bit.

🇦🇺Australia geoffreyr

Sorry for the long wait, but have got an initial dev release pushed out. Will work on some test automation next.

🇦🇺Australia geoffreyr

Actually implementing this is probably going to be a bit tricky. I've had an initial look at it and it's complicated by the library only being available in ESM form, both in Svelte and Vanilla forms. An initial pass with Webpack didn't get anywhere in being able to use it as an external lib. You could do it by baking the library in, but that'd blow out the size of the module.

🇦🇺Australia geoffreyr

Digging deeper into this. I'm seeing the following fired off during an install of the Ableplayer module.

$form = \Drupal::entityTypeManager()->getStorage('entity_form_display')->load('media.' . $bundle->get('id') . '.default');

Causes the exception Missing required properties for an EntityDisplay entity. to be thrown. The contents of the values it is sent are as follows:

Array (
  [content] => Array
  (
    [field_ableplayer_media_caption] => Array
    (
      [type] => media_library_widget
    )
  )
)

EntityDisplayBase expects a targetEntityType and bundle property to be present as well. But I'm not sure where it's supposed to be coming from; and this is during a load method call to boot. It's not cached the old config, and ableplayer's formatter isn't even present in the config table. Very odd.

🇦🇺Australia geoffreyr

Having a little look at this now. Starting by adding a fallback to t('Weight') that applies even when the module can't find label config for the field.

🇦🇺Australia geoffreyr

@theMusician We already had an extensive code and config base for the site we were adding Ableplayer to, so this will probably be hard to replicate. Thanks for testing the patch out, it is odd that nothing got logged even when you specifically removed the form displays. I'll see if I can find time to put together an example.

🇦🇺Australia geoffreyr

I've checked your patch and there aren't any further instances that need updating.

The reason that I needed to add UTC string to the 1st param of DrupalDateTime constructor is because I found that the dates used to index $exceptions wouldn't match those for the DrupalDateTime unless I specifically added it; there was always an offset equivalent to the difference between my timezone and UTC.

🇦🇺Australia geoffreyr

Yes, the View that I'm using this on is set to display only the current day.

I've just pushed an update that adds a replace_exceptions key in exceptions config that determines whether to run the exceptions replacement logic. Hopefully this helps, let me know if there's anything else you'd like me to take a look at.

🇦🇺Australia geoffreyr

@johnv Thank you for your patch! I've set up an issue fork at https://git.drupalcode.org/issue/office_hours-3370722/-/tree/3370722-rep... that applies the patch, fixes some typing issues I found, and adjusts some of the iterator logic as it wasn't replacing the first default slot of days with an exception.

My use case is displaying today's opening hours for a list of venues, so it's pretty narrow compared to the needs of everyone that uses this module. I reckon that we'll want to make replacements optional so it applies to some formatters and not others. Either way, we'll need some more eyes on it to see if the behaviour that we're seeing is what we want. Also, tests. I need to write some tests.

Production build 0.71.5 2024