Ukraine
Account created on 20 November 2017, about 7 years ago
#

Merge Requests

More

Recent comments

🇺🇦Ukraine HitchShock Ukraine

@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

🇺🇦Ukraine HitchShock Ukraine

A fix will be provided in the next release

🇺🇦Ukraine HitchShock Ukraine

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

🇺🇦Ukraine HitchShock Ukraine

Provided CI and fixed phpstan/phpcs/cspell errors.

🇺🇦Ukraine HitchShock Ukraine

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

🇺🇦Ukraine HitchShock Ukraine

@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

🇺🇦Ukraine HitchShock Ukraine

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

🇺🇦Ukraine HitchShock Ukraine

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

🇺🇦Ukraine HitchShock Ukraine

Added filters to the list page

🇺🇦Ukraine HitchShock Ukraine

Refactored the code to make it D11-compatible

🇺🇦Ukraine HitchShock Ukraine

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.

🇺🇦Ukraine HitchShock Ukraine

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

🇺🇦Ukraine HitchShock Ukraine

@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.

🇺🇦Ukraine HitchShock Ukraine

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

🇺🇦Ukraine HitchShock Ukraine

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.

🇺🇦Ukraine HitchShock Ukraine

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

🇺🇦Ukraine HitchShock Ukraine

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

🇺🇦Ukraine HitchShock Ukraine

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

🇺🇦Ukraine HitchShock Ukraine

Wrote an answer to the last comment in the MR

🇺🇦Ukraine HitchShock Ukraine

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.

🇺🇦Ukraine HitchShock Ukraine

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.

🇺🇦Ukraine HitchShock Ukraine

The last patch looks well to me. RTBC

🇺🇦Ukraine HitchShock Ukraine

Refactored the last patch to make it Drupal 10 compatible.

🇺🇦Ukraine HitchShock Ukraine

It doesn't work for users because:
Entity type - user
Base field - users

Patch #1 Updated the first patch to fix the issue with users according to the core behavior (see EntityViewsData::getViewsData())

Patch #5 is a great example of how to add a filter for a computed field, but one type of field is not enough to say that the issue is resolved.
Also, I'm sure that it's not required for the current ticket. We can create a separate one for filters and sorts, where we will provide more cases, for more field types. But at the moment, it makes no sense to block the task because of this.

🇺🇦Ukraine HitchShock Ukraine

Hi @sourabhjain
Seems like php extension doesn't exist only on your local env, that's why you have such PHPStan errors.
According to
https://git.drupalcode.org/project/pdf_to_imagefield/-/jobs/947631
https://git.drupalcode.org/issue/pdf_to_imagefield-3425194/-/jobs/976016
there is only one PHPStan error

 ------ ------------------------------------------------------------------------------ 
  Line   src/Plugin/Field/FieldWidget/PDFToImageWidget.php                             
 ------ ------------------------------------------------------------------------------ 
  64     Unsafe usage of new static().                                                 
         💡 See:                                                                       
            https://phpstan.org/blog/solving-phpstan-error-unsafe-usage-of-new-static  
 ------ ------------------------------------------------------------------------------ 
🇺🇦Ukraine HitchShock Ukraine

php imagick extension is a requirement for this module.
When you install the module via composer you must receive an error like this
spatie/pdf-to-image 2.2.0 requires ext-imagick * -> it is missing from your system. Install or enable PHP's imagick extension.

So this is an expected behavior.

I created a separate ticket to add an extra check before installing the module - 📌 Add check installation requirements and do status reporting. Active

🇺🇦Ukraine HitchShock Ukraine

Well done @denist3r
Merged into the dev branch.

Production build 0.71.5 2024