🇪🇸Spain @omarlopesino

I added a composer.json file to be able to require the module through composer

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

The MR !18 invokes the hook before converting the diff to JSON and saving the log, only when diffs are enabled of course.

Can you review it please? Thanks!

omarlopesino → created an issue.

Thanks both for the feedback!

About #1: Adding a hook will allow developers quickly sanitizing the values, meanwhile we prepare a definitive solution. It would also let developers add custom logic to the redaction, as maybe it needs some dynamic calculation, or a specific way to redact it.

This approach would help users less familiarized with module development / those who want to provide this redact configuration through recipes.

I will open a new issue for adding the hooks to alter diffs, independent on the purpose.

About #2: I agree as it will help to easily switch on and off the redaction. That can be saved into Drupal state, so it does not need deployments.

omarlopesino → created an issue. See original summary → .

omarlopesino → created an issue.

After reviewing this issue back, I agree that image_widget_crop is setting up "arbitrary properties" into the field value for convenience. That is the way the module is able to save the list of crops, to save it properly on entity save.

That is a specific behaviour that I think it won't be present on Drupal core, so I agree it belongs to contrib support.

However, I noticed something. This problem is happening because a contrib module is setting a property to a field value which won't be saved. In this case, that property won't be saved to database. I mean, if the entity is loaded, that `image_crop` property won't be present.

I will check if it is possible to solve this problem with other solution: loading the entity before saving the diff here, like it is done at entity update

omarlopesino → created an issue.

Released in 1.0.5. Thanks, and apologies again in case rejecting the issue have felt uncomfortable.

I have included to the MR a hook update that will update the view , replacing the configuration with the module's one. The module's main feature is the view, so it is needed to automatically refresh the view once is updated in the module.

Please note @eduardo, that the site where you are using this view will be changed. I think it won't suppose a problem, but if it does, let me know and we can think a solution if needed.

After talking with @Gedur, I have understood that filtering by date helps filtering out errors that may have been fixed in the past, and are not needed to be seen. You have tried to explain it to me but I didn't understand well.

Reopening and doing just some minimal adjustments:

• The 'relative dates' placeholder is removed, as the filter will be based on dates.
• The filter is now considering the greater or equal, and less or equal, instead greater and less.
• The labels have been renamed to 'Timestamp (from)' and 'Timestamp (to)'.
• The ids of the exposed filters have been renamed to timestamp_from and timestamp_to.

Thanks both for the contribution, changes will be consolidated and added into a new release.

Yesterday I've noticed that I can use lists selecting 'Custom Scalar + Raw'. So the module already implements what I've needed. CLosing.

I have this warning in my project. It happens when I create a bundle computed field and add I declare it into views. The problem happens because commerce promotion is trying to get the field information from entity field definitions, and not entity bundle field definitions.

I've added a 'Steps to reproduce' section at the issue.

This patch solves the issue for me. Please review, thanks!

I've checked this and it works fine. I prefer this solution rather than the previous one as it not rely anymore in the values returned by formatters, so results are more predictible. Thanks!

Moving to RTBC

?? only works when the returned value is null and ?: accepts any value considered empty (FALSE / 0...). For this case both should be good, but ?: would cover more unexpected returns. Let me know if you want me to do the change.

This has already been released, thanks!

omarlopesino → created an issue.

I confirm that Scala returns the context as it is , but the problem is that the returned json by the formatter is a plain text of the json, and not the json decoded.

Then, in the GraphQL view, instead of returning

"myJsonField": {
"foo": "bar"
}

it returns:

"myJsonField": "\<code>{\r\n  \"foo\": \"bar\"\r\n}\</co..>"

This MR allows recognizing this specific case and do the conversion.

The MR fixes the problem. it just fixes the parenthesis so each empty call is separated. Please review, thanks!

omarlopesino → created an issue.

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

All the warnings reported by static analysis have been fixed, which led to some small refactors. It should preserve the old behaviour. Please review, thanks!

I agree that would be a definitive solution. In the meanwhile, a patch generated from this MR can be used to be able to view status report in sites using xray audit but not using paragraphs.

Ups my bad, I didn't check the submodules. This is already supported by the graphql comments submodule, thanks for letting me know.

Would it be good idea supporting the rest of fields of the comment entity type? I mean, the entity_id, entity_type, field_name , and pid.

I've updated the MR updating the Comment GraphQLEntityType plugin with those fields.

The MR solves the problem for me, please consider reviewing and merging, thanks!

omarlopesino → created an issue.

MR created. Please review, thanks!

omarlopesino → created an issue.

Created MR. Please review, thanks!

omarlopesino → created an issue.

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

Created MR. PLease review, thanks!

omarlopesino → created an issue.

I've just added a commit that ensures there is a field that sets which are the allowed query tags, so if a query tag is passed dinamically, it does not add unwanted query tags.

Please review, if it looks good I can add tests.

Agree with #5. Closing this issue in favor of https://www.drupal.org/project/graphql/issues/3491736 🐛 Cache collision when multiple servers are using the same schema plugin Active , which already applies fixes detected at this issue.

Merge request re-rolled. Tests are passing now. Ready to review again.

I've just adapted the tests to the new functionality, ready to review again.

I find this issue pretty useful for my use case. I want to build an API and having generic mutations helps a lot to get all the job done.

Don't mind me adding to the MR some improvements:

- Retrieve uuid in generic entity queries.
- Allow reading graphql compose configuration to use the same field names than in the rest of graphql queries.

I have created a MR with the proposed resolution, please consider reviewing and merging it into the module. Thanks!

omarlopesino → created an issue.

Gitlab CI tests added. No phpcs/phpstan errors found :) just a minor cspell error that is just fixed in the MR.

Please review, thanks!

I've just seen in this commit the compatibility with Drupal 11 is added: https://git.drupalcode.org/issue/group_term-3507121/-/commit/8997937bb25...

Adding a .gitlab-ci.yml file so that it is possible confirm compatibility with Drupal 11 through tests / static analysis.

omarlopesino → created an issue.

About this issue, I am aware of this problem, and we have this important problem if we fix it: If we release a new version changing the module name, the sites using this module would break. It is true that uninstalling the site , updating the module, and reinstalling it would fix it, but having to reconfigure an entire module should be avoided. Any ideas to deal with this problem?

Patch from #2 is empty. Is it possible that you have renamed it but the patch haven't been generated correctly?

I suggest you following these instructions → or use Drupal GItalb repositories → to create a merge request.

I notices too late about this issue :/ and I don't know if my answer will be useful as possibly the root problem is fixed somehow. However, I will reply in case this is needed for future use cases.

The To field is the same field used at the standasrd Webform email handler, as you mention it must be filled with either the manual list of emails or a token.

To add subscribers dynamically, it is needed to code it as it is not possible to dynamically get them out of the box. There are two possible solutions that needs to be evaluated:

1. In a custom module, implement hook_token_info and hook_tokens to provide a token with the list of subscriber emails, separated by comma.
2. If the subscribers are saved through a contributed module that manage newsletter subscriptions, a submodule into webform entity email can be created.

This has been released releases ago, but forgot to move the issue to 'Fixed'.

LGTM. Added to merge train, and it will be released in alpha14

Currently we are using select queries → , which already supports database prefixing, as they are automatically added by Drupal API.

If your wordpress database has a database prefix, it should be declared at database definition at settings.php (or other settings file) .

Example:

$databases['legacy']['default'] = [
  'database' => 'legacy',
  'username' => 'db',
  'password' => 'db',
  'prefix' => 'elem_',
  'host' => $host,
  'port' => $port,
  'driver' => $driver,
];

This example adds the 'prefix' => 'elem_', property to let Drupal nows the database prefix on queries. This should fix the issue. Indeed, I had to do this on my project to make it works with a wordpress site that has database prefixes.

Let me know if that fixes the problem for you.

Marking task as Needs review in the meanwhile.

Released at 1.0.1

I've merged and there were no issues. Released at 1.0.1

Merged and released at 1.1.9. Now the module is fully compatible with Drupal 11.

The tests fails, I guess because Salesforce module was not yet compatible with 11, but now it is compatible.

Thanks for noticing this! This change will prevent false positives / negatives. Merging. It will be released at 1.0.1

Released at 2.0.2

Released at 2.0.2

Released at 2.0.2. Thanks!

After reviewing it looks good to me. For a moment I have thought on what happens with this requesst as it is paged. and I've ensured that is possible to add pager arguments through the query.

It will be released at 2.0.2

Released at 1.0.0-alpha13

Released at 1.1.1

Thanks, I've just commited this into the main branch and it will be released at 2.0.2

It looks good to me, thanks for fix the method so we get the correct WordPress term ids!

PHPunit tests fails after adapting the code to Drupal 11. It is needed reviewing what is happening and fix it.

After reviewing everything is enough okay. Version 2.0.0 has been released and now the module is stable.

Release candidate created. After reviewing a stable version will be created, probably on Monday.

1.2.1 is compatible with 11

Drupal 11 compatibility changes have been merged and released at 1.0.1

The MR build is failing.

This module has been refactored in 2.x version and currently there is no support for the development version.

THe version 2.x will support the latest Drupal version.

Most of the steps are done, the pending points are adding the status report , RC, review and create the stable version :)

Working on it.

omarlopesino → created an issue.

Sure! It is not possible contacting them through Drupal (they do not allow be contacted directly from drupal.org). I've sent a direct message through drupal.org Slack. Hopefully they respond.

I have tested it on my site and I confirm the ajax block form can be properly configured in Drupal 11. Thanks!

omarlopesino → created an issue.

I wonder if this module should use plugins to support the different purger types, so this escalates better.

This issue is really interesting, thanks for the contribution @kevinb623 . Supporting queues may drastically improve the performance when there are too many URLs to purge.

Also many thanks for helping reviewing @dpi, it saved me time.

The most priority changes needed to merge this are:

1. Make form labels translatable
2. Hook install to configure existing sites.
3. Coding standards must pass, Currently, this module is integrated with Gitlab CI to help on this purpose.
4. Ideally, applying every suggestion mentioned in the feedback.