🇺🇦Ukraine @HitchShock

Hi @acbramley
I understand your point, but:
- That ticket is still in progress, so it doesn't make sense to move the functions elsewhere for now, because it's still a big question which of the tasks will be solved first.
Since there will be a merge conflict here, the task that will merge last will fix it. So, we can ignore it for now.
- A similar opinion as above, since all class methods currently use db queries, it should be the same here. Their refactoring will be part of #3396062 or will happen here if that task is merged first.
Especially since we can't use entity queries to get these values. It will at least be select queries

Conclusion: Your comment cannot be a blocker for this ticket.

P.S. There is also a related bug described here 🐛 Skip changing 'revision translation affected' field if the old revision is syncing Needs work . Without that patch, a revision_translation_affected property might be changed, which can cause the issue when we see a revision of the other(unexpected) language in the revisions list.

Steps to reproduce:

• User A - created a node EN - revision 1
• User B - created a translation ES - revision 2
• User A - updated a translation EN - revsion 3
• Revision list:
  • EN - revisions 1 and 3
  • ES - revision 2
• Remove User B
• Revision list:
  • EN - revisions 1, 2, 3
  • ES - revision 2

Reroled and refactored the fix.
Updated tests.
Also, provided patch files for different versions,

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

+1
Files of the library have been updated several times, but the version hasn't been updated for a long time.
So, please, remove the library version or don't forget to update it every time the library files are updated.

I like the solution to remove the version.
So, RTBC.

+1 Caught the same issue
The problem is that I can't find any default features that will allow us to reproduce it with a simple core settings.
But if we will add a custom constraint to the field with Media library, then it won't be working, because of the reason mentioned in the description.

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

hitchshock → created an issue.

hitchshock → created an issue.

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

The last patch works for me and also fixes the critical conflict with entity_reference_revisions module.
+1 RTBC

@socialnicheguru functionality conflicts or code conflicts inside the *group.module*?
If it's just a code conflicts, then no extra moves are needed here since there are no conflicts with the dev branches

hitchshock → created an issue.

MR must be rebased from the dev branch

A fix will be provided in the next release

Thanks all for the help.
I reverted some changes to keep the module compatible with other packages and older versions of Drupal.
For example:
- I reverted "spatie/pdf-to-image": "*" cuz our module doesn't support version >=3.0
- We still need to use FileSystemInterface for older Drupal versions

A fix will be provided in the next release

@carolpettirossi Nice catch! Thanks!

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

Provided CI and fixed phpstan/phpcs/cspell errors.

hitchshock → created an issue.

Done!

@candelas Thanks for the issue.
I will use this ticket to clean up the outdated generator commands.
Updated the title and description.

@chrisscrumping Thanks for the patch.
Even though the bug was fixed in the context of 📌 Automated Drupal 11 compatibility fixes for migrate_file_to_media Needs review I would prefer to give you credit for your activities

Updated project requirements.
The minimal Drupal version is 10.3 from now.
Fixed phpstan errors on the project to make it compatible with D10/D11

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

Thanks @carolpettirossi for the patch.
I made a small change in the MR.
An update will be available in the next release.

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

hitchshock → created an issue.

hitchshock → created an issue.

Fixed caching

Implemented bulk operation

hitchshock → created an issue.

Implemented a new config

hitchshock → created an issue.

Installed CI

Added filters to the list page

hitchshock → created an issue.

hitchshock → created an issue.

hitchshock → created an issue.

hitchshock → created an issue.

hitchshock → created an issue.

Refactored the code to make it D11-compatible

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

Implemented a patch + added new fixes.

About the issue from bjorn, it was caused by an error during installation. That error prevented the creation of the entity table.

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

@lazzyvn As you wish, but this is a form for testing. I don't believe that someone uses it regularly and for many users. So, I don't understand how it changes the module design.
The form still works as expected, we can still send the message to everyone or selected users (separated by a comma). This is more than enough for testing.

hitchshock → created an issue.

HitchShock → created an issue.

The module social_swiftmail was already migrated to symfony mailer but swiftmail module still exists in composer.json requirements.
It must be removed from there ASAP, it breaks the composer audit and prevents a normal update process

I see that the simpler the work, the more it is scrutinized, and everyone sees something different and changes the same thing several times. And they pick on such trifles that are absolutely not critical and will not affect the overall functionality.
Instead of quickly implementing a simple change, everyone is engaged in bureaucratic ping-pong for no real reason.

In a month, someone else will come and say that they don't like some error text (which is generally displayed only when UI validation is disabled, which is almost never) or comment text.

I'm tired of fulfilling the whims of everyone who comes to check the task. So I'm stopping working on the issue.

Also added a patch

HitchShock → created an issue.

Solved the last PR issues

A solution #6 is pretty good for me.
I just updated it a bit:
- it's better to use regex patterns to search and replace values
- sometimes we need also to update config keys (not only values)
- it's not required to update IDs of the Relationship Types. We can keep them to prevent possible errors with IDs mentioned in #15

And of course, you need to update your custom code:
- group_content -> group_relationship
- group_content_type -> group_relationship_type

Also, I removed $max_length for the prepareText() function, cuz I don't believe that this is the correct solution. We need to use a fully indexed value. The indexed value is already truncated by the Embeddings data type plugin

HitchShock → created an issue.

Sorry @smustgrave, forgot to submit my comment there :)

Wrote an answer to the last comment in the MR

HitchShock → created an issue.

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

HitchShock → created an issue.

HitchShock → created an issue.

Hi @benjifisher
1. Fixed a small grammatic issue
2. About the empty email error message:

If a user enters an empt address and an invalid one, then currently they get the message about an empty address. They fix it and try again, and get the message about the invalid address. Let's list all the errors at once.

I really liked this idea and implemented it.
But I don't like the idea to make an "empty" message like this

The email address "" is not valid. Use the format user@example.com, and separate the addresses with a comma.

For me, this message is unclear/unreadable/not user-friendly. It is very difficult to correctly display an "empty" value. Using quotation marks for this purpose can only worsen the situation, as the user may start looking for something that is not there. A message like that really can confuse a user.
While the direct text that an empty email address value has been entered is clear and not ambiguous.

HitchShock → created an issue.

FYI. The function field_ui_form_field_ui_field_storage_add_form_alter() still exists in the code after ✨ Make field selection less overwhelming by introducing groups Fixed .
Atm this is a dead code cuz the behaviour/logic was changed.
Since this ticket is in fact a continuation of that epic, it is the best candidate to remove this useless function.

Updated MR.

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

HitchShock → created an issue.

The last patch looks well to me. RTBC