Create a blocking system

Nice work.Thanks @sagesolutions .
It would be grateful if this blocking system integrated with Version 3.

Great work @sagesolutions!

I have tested out the blocking system for private_message. I can say it works for me as expected.

When User A blocked User B:

1. User A can view the private message with User B, and cannot send any messages
2. User B can view the private message with User A, and can send message as usual
3. When User B does not know whether they are not blocked, as they still can message User A as usual
4. When User B message the thread, User A cannot see whats new in the thread
5. When User B message the thread, User A will not get new notifications, this include email too
6. If User A remove User B in private message block, User A can see latest messages from User B in the thread
7. If User A remove User B in private message block, there is no new notifications if User B send message before the unblocking

I can say this is RTBC, as a base for blocking system, it works.

Is this possible to put also to Private Message version 3.x ? Now this is for 2.x

Hi,

Just wanted to ask is this intentional and why mentioned in #52:

When User A blocked User B:
2. User B can view the private message with User A, and can send message as usual"

Does this "send message as usual" mean that user B can send a message to user A?

I think it should not be possible because user B is blocked by user A, they should not have any messages anymore?

@jukka792 From your comment in #54, it depends if its set on passive or active mode. If its on passive, then user B doesn't know they're blocked and they can continue to send messages to user A. User A will not see these messages in their private messages.

However, if you set the mode to Active, then User B cannot send messages to user A since they are blocked.

I hope this clears things up!

In regards to #53, at the time of development version 3.x wasn't out yet. I don't have time for the next couple months to add it to 3.x. Hopefully someone else can do the upgrade path to 3.x if this is urgent.

Hey @sagesolutions that's absolutely amazing work, thank you for your efforts!

I'm trying to review your MR and I'd really appreciate if you can give more details why you decided to make block entities to be revisionable and use EntityPublishedTrait and EntityChangedTrait... Is that used somehow? From my point of view it is much more straightforward to just remove blocks and add them back isntead of creating the revisions but maybe I'm missing something.

Also all the new features are supposed to go to 3.x branch, as some of the planned updates are breaking changes and 2.x is for bug-fixes only.
Could you try to update the target branch to 3.x?

@artem_sylchuk great questions!

I agree, I don't think we need the EntityChangedTrait or EntityPublishedTrait on the PrivateMessageBlock entity. Also, I don't think we need revisions as well. This should really simplify the entity.

I've updated the target branch to 3.0.x, and tried to rebase but got the following error:

Rebase failed: Rebase locally, resolve all conflicts, then push the branch. Try again.

I will update the merge conflicts for the 3.0 branch soon, but no promises!

I've managed to rebase to version 3.0.x and removed the EntityChangedTrait and EntityPublishedTrait on the PrivateMessageBlock entity.

I kept the entity revision-able because when I took over from patch #30, a lot of work was done regarding revisions.

Please review!

I did a quick review and seen at least a few possible improvements:

• Default values for block_mode and block_message configuration values are not set. Defaults for them are handled in code but saving the configuration form at least once would set configuration entity properties so it's worth adding defaults to the installed config YML to avoid confusion.
• New configuration values aren't set in update hooks (i.e. there's no upgrade path for config).
• There's no way to disable blocks system; i.e. the behavior of existing installations would change on upgrade, which may be undesirable.
• At least some of the changes (at least in CSS) seems to be unnecessary (like reformatting code without other changes). Another example is adding StringTranslationTrait to the src/Service/PrivateMessageService.php file.
• The js/private_message_block_user.js file should be using let instead of var. Also, it may need once() in behavior attachment part.
• The block_page library may need to have a dependency on drupal.autocomplete.
• The database connection is being injected in src/Service/PrivateMessageService.php but is not used.
• At least some of functionality of src/Service/PrivateMessageBlockServiceInterface.php is duplicated in src/Service/PrivateMessageService.php. A dependency injection should be used instead to avoid duplication of logic and unnecessary code complexity growth.
• The private_message_block entity type shouldn't use revisions or translations.
• The access control for private_message_block is incomplete; users should be only able to iteract with their own blocking records (unless they have an admin permission which allows bypassing this).
• The private_message_block entity type should probably be simplified to and have a pair of 'who -> whom' IDs and no data table. Currently, there's a single entity for each user (created when the user blocks someone for the first time). Instead, there could be multiple entities for each user but they'd be much lightweight so we won't have to load the list of all blocked users each time. This would also simplify querying as there would always be just one table.
• The private_message_block entity type does not have its own entity view page (there's just no sense in it), so the template
• Tests changes appears to be unnecessary (but I might be mistaken).
• Finally, tests for new functionality are missing.

Remaining points:

• Default values for block_mode and block_message configuration values are not set. Defaults for them are handled in code but saving the configuration form at least once would set configuration entity properties so it's worth adding defaults to the installed config YML to avoid confusion.
• New configuration values aren't set in update hooks (i.e. there's no upgrade path for config).
• There's no way to disable blocks system; i.e. the behavior of existing installations would change on upgrade, which may be undesirable.
• The js/private_message_block_user.js file should be using let instead of var. Also, it may need once() in behavior attachment part.
• The block_page library may need to have a dependency on drupal.autocomplete.
• The access control for private_message_block is incomplete; users should be only able to iteract with their own blocking records (unless they have an admin permission which allows bypassing this).
• The private_message_block entity type does not have its own entity view page (there's just no sense in it), so the template
• Tests changes appears to be unnecessary (but I might be mistaken).
• Finally, tests for new functionality are missing.

Also I think the private_message_block entity type should be renamed to avoid using the 'block' word as it could misinterpreted as Drupal block (e.g. a thing that you place on a page and it displays something).

Hello there. I have fixed almost all the points mentioned in #64. The only thing left to do it to write tests for this functionality.

Hello,

Not sure why, I cannot test out the patch, most file patch failed. I am using private message version 3.0.0 (composer require 'drupal/private_message:^3.0')

Not sure if its help, the log is as attach

I have manually changes the code based on git 69 so that it can be use with the latest private_message ver 3.0.0. the patch is as attach if anywant wanted to test out

big thanks to @dinazaur, @abramm, and @sagesolutions!

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.

Automatically closed - issue fixed for 2 weeks with no activity.