Account created on 20 August 2008, over 16 years ago
#

Merge Requests

More

Recent comments

🇬🇧United Kingdom welly

We should also allow global tokens to be used in the email body rather than the module specific tokens.

🇬🇧United Kingdom welly

This will need rerolling for the 3.x branch

🇬🇧United Kingdom welly

This is fixed in the 3.0.x-dev branch (and 3.0.0-alpha1 release)

🇬🇧United Kingdom welly

This is fixed in the 3.0.x-dev branch (and 3.0.0-alpha1 release)

🇬🇧United Kingdom welly

Work is being done on a new 3.x branch to bring the module up to date.

🇬🇧United Kingdom welly

Thanks @smustgrave - I've reformatted the issue summary to give a bit more structure/context. Hopefully this will be more suitable for review!

🇬🇧United Kingdom welly

"I stand by the point that the phrase is unnecessarily sexualized; the rationale for naming the commands that way doesn't make sense other than it kinda matches Drush's shortening of "config-export", and there's no reason to not rename them."

Where do you stand on the county names Essex, Sussex? How about the navigation instrument - sextant?

With all due respect, if you find an acronym "sexualised" then that's on you not the rest of us.

But that's besides the point - your pearl clutching has caused build pipelines to break. There was no forewarning of this when the module was updated, nothing on the module homepage and nothing in the README.md file.

That is the problem here not your personal embarrassment at an acronym with four letters in it.

🇬🇧United Kingdom welly

Is this real? Sexualised phrase? Please tell me this is some kind of April Fool's joke.

Congratulations on breaking my build pipeline without any forewarning.

🇬🇧United Kingdom welly

@atomi there are no "facts" or "truth" in your comments. It is nothing more or less than opinion. Please don't use those words if you don't understand the meaning of them.

Your earlier comment was rude. Even if you absolutely hate the new design, you didn't provide any constructive criticism - you were rude and unhelpful. If you hate the new design, then say why rather than just post nasty remarks.

🇬🇧United Kingdom welly

Overall, I like the site refresh. It's much more modern so well done to all that was involved in this.

A few comments that have possibly been already made previously but I'll add mine anyway.

The three dots on the big blue block - I was expecting those to action something but clicking them furiously left me unsure if something was broken as they didn't seem to do anything. I'm not sure what they're supposed to represent if not buttons. I guess some kind of design motif?

The striped background when I first saw it, I think I recoiled a bit. Perhaps a bit more subtle - I'm not sure I like it personally (but other people may well do) but I suppose it's not completely offensive :) I'm just not that keen, personal opinion I suppose.

Drupal CMS and Drupal Core descriptions in the menu - while I know what these are, the description doesn't really make it clear what they are. I think something a little more descriptive than the current, somewhat cryptic, words. It just doesn't say what each thing is. Perhaps a bit less marketing and a bit more clarity on those words.

The Drupal 11 link doesn't have any hover text - not sure if this might have some impact on accessibility.

Those are just a few points that immediately jump out at me, but otherwise a great refresh. Must have been a lot of work for a bunch of people to then come in and whinge about background images and so on :)

🇬🇧United Kingdom welly

There is a related issue here https://www.drupal.org/project/storybook/issues/3442952 The story json generation should skip symlinks. Active

🇬🇧United Kingdom welly

I'm seeing a similar problem - I'm not sure if it's because I'm looking at this on a local dev environment or if something else is amiss. While I don't want to index Acquia solr from my local environment, it would good if I could at least connect to it so I can configure the dev/stage/prod environments.

I'm seeing this error in my local ddev environment:

Could not find a Solr core corresponding to your website and environment. Your subscription contains these cores: ACGI-XXXXXX.stage.XXXXXXX

and I see this when viewing the Acquia search API server:

Solr server URI https://XXXXXXXX.ddev.site:443/solr//
Solr core URI https://XXXXXXXX.ddev.site:443/solr//

🇬🇧United Kingdom welly

I wonder if this is because we're using symlinks to build the docroot file system?

🇬🇧United Kingdom welly

Hi @msn5158,

If you can give me a little bit more information, I'd be happy to help but this module is specifically for views contextual filters. I suppose it's might not entirely be out of scope of the module but this was designed for handling views contextual filters. Can you show a slightly more detailed example and I'll see if it's something we can implement.

Thanks!

🇬🇧United Kingdom welly

I'm not sure you've asked this in the correct project? This module doesn't provide a Replicate Settings screen and just provides an API. I think you are referring to Replicate UI as that does provide a settings form. So you'd probably have to ask your question there:

https://www.drupal.org/project/issues/replicate_ui?status=All&categories...

Otherwise, if I think this could be done using the cloneEntity() method and setting the uid for the cloned entity to the current user before saving the entity. But I think this is one for replicate_ui module.

🇬🇧United Kingdom welly

If this can be reviewed, it'd be much appreciated! Or at least my approach discussed. We'll probably have to revisit https://www.drupal.org/project/coc_forms_auto_export/issues/3259009 🐛 PHP Warnings/Notices on Download Page Fixed and https://www.drupal.org/project/coc_forms_auto_export/issues/3286562 📌 Automated Drupal 10 compatibility fixes Fixed but it'll get the dev branch in a working state.

🇬🇧United Kingdom welly

Not sure if it's directly related to the patch or something else but when I apply the patch and go from "Which file will be the one the others are merged into?" to the confirmation step, I get an error reporting:

The website encountered an unexpected error. Try again later.InvalidArgumentException: Field field_name is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() (line 616 of core/lib/Drupal/Core/Entity/ContentEntityBase.php). Drupal\Core\Entity\ContentEntityBase->get('field_name') (Line: 657)
Drupal\Core\Entity\ContentEntityBase->set('field_name', Array) (Line: 184)
Drupal\auditfiles\Auditor\AuditFilesMergeFileReferences->auditfilesMergeFileReferencesUpdateFileFields(1, 2) (Line: 133)
Drupal\auditfiles\Auditor\AuditFilesMergeFileReferences->listenerMergeFiles(Object, 'Drupal\auditfiles\Event\AuditFilesMergeFilesEvent', Object)
call_user_func(Array, Object, 'Drupal\auditfiles\Event\AuditFilesMergeFilesEvent', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object) (Line: 54)
Drupal\auditfiles\Batch\AuditFilesMergeFileReferencesBatchProcess->dispatch(Array) (Line: 43)
Drupal\auditfiles\Batch\AuditFilesMergeFileReferencesBatchProcess::create(Object, Object, Array) (Line: 296)

Moving back to "Needs work", unless this is a new/different issue.

🇬🇧United Kingdom welly

Please ignore this for now. Once https://www.drupal.org/project/coc_forms_auto_export/issues/3472520 🐛 Dev branch broken Needs review is reviewed/merged then I'll restart the branch as there are too many merge conflicts.

🇬🇧United Kingdom welly

Feel free to ignore this but added a backport of the patch for the 8.x-2.1release.

🇬🇧United Kingdom welly

This MR reverts the 3 merged branches that have caused the dev branch to be broken, and so reverts the module back to the last known good version (pre-D10). https://www.drupal.org/project/coc_forms_auto_export/issues/3472520 🐛 Dev branch broken Needs review should fix the branch and so that issue should probably be reviewed first and merged in before this one is looked at.

🇬🇧United Kingdom welly

I've had to revert a few of the commits so known, working patches could be reapplied and so I think this is probably a good starting place for ongoing development.

🇬🇧United Kingdom welly

welly changed the visibility of the branch 3472585-add-gitlab-ci to active.

🇬🇧United Kingdom welly

welly changed the visibility of the branch 3472585-add-gitlab-ci to hidden.

🇬🇧United Kingdom welly

Updated MR with coding standards fixes

🇬🇧United Kingdom welly

Have created an initial merge request that looks like it's working (although there are other issues that need investigation which can be done in a different issue). I'll need to merge in the coding standards work and check that doesn't break anything.

🇬🇧United Kingdom welly

I think something has gone awry here since some of these changes have been merged in. I wonder if there was a merge conflict when merging in the recent changes?

When enabling the dev version of the module, we get several errors including:

ParseError: Unmatched '}' in Drupal\Core\Extension\Extension->load() (line 633 of modules/custom/coc_forms_auto_export/coc_forms_auto_export.module).

And when you remove that addition curly brace, it causes the next following functions to break the code. I think this needs reworking. I'm going to create a new issue and a merge request.

🇬🇧United Kingdom welly

This was a long time coming! I remember starting work on this issue 8 years ago :) Glad to see it finally get over the line!

🇬🇧United Kingdom welly

I've attached a config export from the above simplytest.me environment so we can re-build the demo site if needs be.

🇬🇧United Kingdom welly

I've created an example of this behaviour here:

https://master-763xidikw6xdqu8pd9mzr8fbjg3acetr.tugboatqa.com/

You should be able to login with the usual simplytest.me credentials (admin/admin) and if you edit a page:

https://master-763xidikw6xdqu8pd9mzr8fbjg3acetr.tugboatqa.com/node/1/edit

you will see one field with a select widget excludes the current node, the second field using autocomplete does not return any nodes.

This shows broken behaviour to me.

🇬🇧United Kingdom welly

My concern with "works as designed" is using different field widgets exhibits different behaviour.

For example, if you create the entity reference view display, add a contextual filter (Content from URL) and attach it to a field and set the field widget to select or checkboxes or similar then this works as expected - the contextual filter behaves exactly as you'd expect and excludes the current node from the field widget.

However, if you then change this to Autocomplete the behaviour changes - no nodes are returned. I think this needs some consideration as to whether this is broken - to me it is.

While the patch/sandbox module does provide a fix for this, I think this should be the responsibility of core for the behaviour to be consistent across different field widgets.

🇬🇧United Kingdom welly

@Mingsong looks good to me for an initial test

🇬🇧United Kingdom welly

Looks good to me. Have tested this and it works as expected. Thanks for this @Mingsong! Sorry took a while to get round to checking.

🇬🇧United Kingdom welly

Ready for an initial review

🇬🇧United Kingdom welly

I'll create a new project issue with my changes to make it D9/10 compatible as a starting point.

🇬🇧United Kingdom welly

Just to note I'm creating these patches so we can apply them as standard in our composer workflow :) Let me know how we can get this issue over the line and merged. This is a pretty important bit of work for a project! Cheers!

🇬🇧United Kingdom welly

$options['update'] should be an int not a bool.

🇬🇧United Kingdom welly

Previous patch didn't apply for some reason. Let's try this one

🇬🇧United Kingdom welly

Having tested this recently, the changes made in the merge request work well and I'm able to expose additional config in the migrate form (I'm currently also overriding the MigrateSourceUi form class).

I've created a backport patch for RC1 (as that's the version I'm currently using) but happy to help out moving this ticket along. Is there anything specific that needs doing?

🇬🇧United Kingdom welly

Running tests..

🇬🇧United Kingdom welly

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

🇬🇧United Kingdom welly

Thanks for the contribution. Have merged that and will look at your other tickets and hopefully create a new release for them as soon as possible.

🇬🇧United Kingdom welly

Yeah, this is a great start! There were a couple of QA issues that needed cleaning up (coding standards, spelling) which I've done but happy to merge this.

Thanks @Mingsong!

🇬🇧United Kingdom welly

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

🇬🇧United Kingdom welly

I don't think this is necessary any longer looking at the changes made from the D10 compatibility fixes. Let's close this.

🇬🇧United Kingdom welly

> Curious why you didn't use trim((string) $var) instead?

Yeah, looking at that post, this seems like a tidier approach. I'll update the merge request and include the changes in the patch, will test it against Drupal 10.

🇬🇧United Kingdom welly

Have been experiencing redirect loops on a site we're using the patch with so back to "needs work" and I'll try and have a look at this myself.

🇬🇧United Kingdom welly

Right, I think this is ready to be looked at. There were some additional changes that needed making. Would be keen on any feedback on the changes made (this included enabling gitlab-ci for the module and then making updates to pass phpstan tests)

🇬🇧United Kingdom welly

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

🇬🇧United Kingdom welly

@Steven Jones, that was the patch in #4 I tested and have been using. Cheers!

🇬🇧United Kingdom welly

Trying again to change status - it didn't change before!

🇬🇧United Kingdom welly

I'm going to put this into RTBC because I'm not experiencing any redirect loops and we've fairly thoroughly tested the patch in a staging environment with no experience of redirect loops. And in fact, trying to create a redirect to itself the module flags this and gives a error message.

🇬🇧United Kingdom welly

I think this should be working pretty well now. Will need some testing.

Production build 0.71.5 2024