Lutsk
Account created on 12 March 2013, over 11 years ago
#

Merge Requests

Recent comments

🇺🇦Ukraine artem_sylchuk Lutsk

Thanks for catching that, fix is merged.
However the code there is confusing, the thread entity seems to be stored in the formState several times with different names, I think I'll create another issue for cleaning that up.

🇺🇦Ukraine artem_sylchuk Lutsk

That should be enough for now I think, thanks!

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

Private message had a config to hide the format selection that was confusing people.
That setting is removed now as preferable way to handle this is to use Drupal core configuration: https://www.drupal.org/node/3318572
For core versions prior 10.1 the https://www.drupal.org/project/allowed_formats had to be used.

Using WYSIWYG editors and allowing different media for drupal default textare is a topic of a basic Drupal configuration and isn't specific for Private Message as module uses the generic field type for it.

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

Let's get this finally merged

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

Drupal 9 is no longer supported so I think we can safely move drush services deifinition to the general services.yml file.

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

Good catch, thanks for your massive work!

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

I hope that additional mesage on the module install will help to not miss the needed steps of the module configuration. The readme already contains mention of the needed steps.

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

Awesome work, thank you guys!

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

artem_sylchuk changed the visibility of the branch 8.x-2.x to hidden.

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

Its mergable with latest dev (at least gitlab says so), however it brings massive UX change and upgrade path is questionable so it is doubtable that I'll merge it the way it is now. However I think eventually having 2 separate private message types is the proper way to go.

🇺🇦Ukraine artem_sylchuk Lutsk

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

🇺🇦Ukraine artem_sylchuk Lutsk

artem_sylchuk changed the visibility of the branch 2942602-unable-to-create to hidden.

🇺🇦Ukraine artem_sylchuk Lutsk

Going to review this, however I see that at least at one place the context passed to once() is wrong

🇺🇦Ukraine artem_sylchuk Lutsk

I suppose there is no more to fix here. Except some documentation maybe

🇺🇦Ukraine artem_sylchuk Lutsk

Can't reproduce on the recent version of the module.

🇺🇦Ukraine artem_sylchuk Lutsk

Code referenced here is no longer present in the supported branch.

🇺🇦Ukraine artem_sylchuk Lutsk

That is doable with using some of the drupal apis (defining a route in a custom module + some preprocess functions), however I don't think it is still relevant after 5 years.

🇺🇦Ukraine artem_sylchuk Lutsk

No chance for this to happen anytime soon.

🇺🇦Ukraine artem_sylchuk Lutsk

Don't think it is still relevant after the 5+ years.

🇺🇦Ukraine artem_sylchuk Lutsk

1.x isn't going to receive any updates.

🇺🇦Ukraine artem_sylchuk Lutsk

Unfortunately 1.x is no longer supported and I'm not sure if anything of that can be easily used in 3.x. CLosing as outdated

🇺🇦Ukraine artem_sylchuk Lutsk

That might be a cool feature, but years passed since its creation and there is no comments here. Closing as 1.x branch is no longer supported.

🇺🇦Ukraine artem_sylchuk Lutsk

1.x is no longer supported, closing as outdated.

🇺🇦Ukraine artem_sylchuk Lutsk

1.x is no longer supported, closing as outdated.

🇺🇦Ukraine artem_sylchuk Lutsk

I think the main idea of the Message module is a bit different from exchanging the messages between users. They define the "templates" with token support to be used as some kind of events log. It perfectly fits the idea of system notifications but I think that it is not ideal when you need to exchange messages between several recepients.

In any case private_message has some kind of integration with message_notify module for notifications and it this hangs open for 5 years without any changes so don't think that anyone plans any work here.

🇺🇦Ukraine artem_sylchuk Lutsk

I'd love to see that as the separate module and probably the complexity of the proposed solution can be reduced, I see some ban entity permissions that probably could be omitted.
But as long as this patch is around for really long I suppose a lot of users has it applied and trying to implement it in a different way will result in the painful upgrade path for ones who used the patch before it was merged.

So merged it as is to 3.x, but probably we need a way to allow disabling this feature. Right now it is possible to hide the ban links, but access to the ban pages can't be restricted using permissions. Don't want to postpone this issue for another year or so because of that.

🇺🇦Ukraine artem_sylchuk Lutsk

Hi @earthangelconsulting,
The biggest breaking change was introduced in 8.x-2.x-beta17.
In 3.0.0-beta2 there was some refactor and the notification sub-system was moved to the separate module.
It can cause problems with private_message_update_8007. I wasn't able to reproduce the problem but please let me know if it happens to you.
(As a workaround you can try skipping that update entirely by placing the return; statement as the first line of private_message_update_8007)

If you already have the 8.x-2.x-beta17 version then it should be pretty safe to upgrade.
But obviously it is recommended to create the database dump before procceeding and running the update on the dev environment first.

*running the update.php or drush updb is thre required step to update*

If you have the version before beta17 then the process may be more complicated. The update hooks should do all the needed changes, however the time of update depends on your database size. You may need to check what goes in private_message_update_8005() before running the update.

Hope that helps, thanks.

Please feel free to re-open if you face any issues during the update.

🇺🇦Ukraine artem_sylchuk Lutsk

Oh, my bad.
Thanks for pointing that out @oadaeh!
Updated the patch.

🇺🇦Ukraine artem_sylchuk Lutsk

The old patch no longer applies, here is the re-roll attempt

🇺🇦Ukraine artem_sylchuk Lutsk

Attempted to re-roll the patch as it doesn't apply to the dev version anymore

🇺🇦Ukraine artem_sylchuk Lutsk

Patch from #21 applies clearly, however it includes the irrelevant part with automatted converting the ->link() to ->toLink()->toString() by rector.
Fixed that and attached the interdiff.

I tried to fix the MR, but it includes a lot of uneeded changes in commits history and it doesn't seem I can create a new branch for some reason.
So I hope patch is fine too.

🇺🇦Ukraine artem_sylchuk Lutsk

Hi @heddn,
I've just opened the issue queue going to finally deal with the D10 realease and found that you already did that.
Thank you very much!

Sorry everyone that it took so long.
I suppose this one should be closed too

Production build 0.69.0 2024