Enh #6335: Pending approvals: possibility to send a message #6365

marc-farre · 2023-05-31T09:29:03Z

Enh #6335: Pending approvals: possibility to send a message
#6335

What kind of change does this PR introduce? (check at least one)

Does this PR introduce a breaking change? (check one)

Yes
No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

It's submitted to the develop branch, not the master branch if no hotfix
When resolving a specific issue, it's referenced in the PR's description (e.g. Fix #xxx[,#xxx], where "xxx" is the Github issue number)
All tests are passing
New/updated tests are included
Changelog was modified

If adding a new feature, the PR's description includes:

A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

luke- · 2023-05-31T12:21:58Z

@funkycram Thanks!
@yurabakhtin Can you please review?

protected/humhub/modules/user/tests/codeception/unit/ApproveUserFormTest.php

marc-farre · 2023-06-01T06:52:38Z

@yurabakhtin Do you know why checks are not running again now?

yurabakhtin · 2023-06-01T07:02:50Z

@yurabakhtin Do you know why checks are not running again now?

@funkycram Maybe because conflicts should be fixed. Can you do this or do you need a help from me?

marc-farre · 2023-06-01T07:46:03Z

Thanks. Yes.
The only way I found to resolve the conflict was removing the added line about this issue. I wait for checks to pass, and then I'll add again the line - Enh #6335: Pending approvals: possibility to send a message.

marc-farre · 2023-06-01T08:46:36Z

@yurabakhtin I think checks are OK now (except the ApproveUserFormTest::testSendMessageIsSentInUserLanguage() because we first need german translation for it to work), but I have "Error: The operation was canceled." on https://github.com/humhub/humhub/actions/runs/5141925904/jobs/9255010872?pr=6365

No I've pushed the change on CHANGELOG_DEV.md and I have again the conflict: https://github.com/humhub/humhub/pull/6365/conflicts

Thanks for your help! Tests are new for me, I need to take some time to learn this part, sorry for that.

marc-farre · 2023-06-02T07:06:58Z

@luke- I think checks are OK now, except the ApproveUserFormTest::testSendMessageIsSentInUserLanguage() because we first need german translation for it to work.
The conflict is due to the README_DEV.md, I don't really know why.
Maybe we could merge into develop and if there is something more to be done I'll create a new PR?

yurabakhtin · 2023-06-02T07:27:47Z

@funkycram

The conflict is due to the README_DEV.md, I don't really know why.
Maybe we could merge into develop and if there is something more to be done I'll create a new PR?

I solve similar issue with this way:

marc-farre · 2023-06-02T07:29:58Z

I solve similar issue with this way

Thanks @yurabakhtin ! I'll know it for next time!

marc-farre · 2023-06-02T09:27:41Z

@yurabakhtin Test are failing for the same reason, because we first need to have german translation:
ApproveUserFormTest: Send message is sent in user language
Test ../../user/tests/codeception/unit/ApproveUserFormTest.php:testSendMessageIsSentInUserLanguage
Failed asserting that two strings are equal.

But it seems ok for the rest.

luke-

@Semir1212 Can you please review the new text messages?

protected/humhub/modules/admin/controllers/ApprovalController.php

protected/humhub/modules/admin/models/forms/ApproveUserForm.php

protected/humhub/modules/admin/models/forms/AuthenticationSettingsForm.php

marc-farre · 2023-08-09T15:35:17Z

@Semir1212 when you have time to these texts (see above). Thanks!

…the same time

marc-farre · 2023-12-13T08:55:18Z

@yurabakhtin I think everything is resolved here. Do you approve this PR? Thanks.

yurabakhtin · 2023-12-13T09:39:11Z

@yurabakhtin I think everything is resolved here. Do you approve this PR? Thanks.

@marc-farre yes, I already approved this PR.

what-the-diff · 2023-12-13T12:02:03Z

PR Summary

New Messaging Feature
- This update comes with a new feature for sending messages to users who are awaiting approval. This feature also allows admins and group managers to send messages to these unapproved users individually or in bulk.
- A new action column has been added to the grid to display buttons for these custom actions.
- You can set default message content from the settings, and this default content can also be altered before sending out the message. As an additional feature, one can keep track of the number of messages sent to individual users.
Improvements to Interface
- The interface has also been updated with a new view file and a form related to message sending.
- The changes also include a new button in the approval index view, which can be used to send messages to all selected users. New javascript code has also been added for handling the bulk action buttons in the approval index view.
Code Enhancements
- Several new methods, attributes, and constants have been added to different models including approval, settings, and form models. These are primarily related to the actions of sending a message and maintaining the default message content.
Increased Testing and Validation
- The PR introduces new test cases, ensuring that the default send message functionality is tested thoroughly. Additional tests have also been included to confirm that administrators and group managers are able to send messages to unapproved users as expected.
- A new method has also been added to assert that a message is sent when required, increasing the robustness of the code.
Minor Code Improvements
- The PR also brings some minor code improvements and refactoring which makes the code more efficient and easier to maintain.

marc-farre · 2023-12-20T08:13:13Z

@yurabakhtin can you help me to resolve these 2 problems?

1/ This pull request will merge automatically when all requirements are met.

If I click here:

Everything is resolved.

2/ Some checks were not successful

PHP 7.4: I don't see why we check this PHP version as it's not supported anymore since 1.15

PHP 8.1: https://github.com/humhub/humhub/actions/runs/7266895756/job/19799580071?pr=6365

The problem seams to be Command failed: cd /home/runner/work/humhub/humhub/protected/humhub/tests ...
An idea?

Thanks for your help!

yurabakhtin · 2023-12-20T10:57:33Z

@marc-farre

1/ This pull request will merge automatically when all requirements are met.

If I click here:

Everything is resolved.

I think @luke- should approve the changes.

PHP 7.4: I don't see why we check this PHP version as it's not supported anymore since 1.15

You are right, you can remove this line https://github.com/humhub/humhub/blob/master/.github/workflows/php-test.yml#L39:

                    - "7.4"

PHP 8.1: https://github.com/humhub/humhub/actions/runs/7266895756/job/19799580071?pr=6365

The problem seams to be Command failed: cd /home/runner/work/humhub/humhub/protected/humhub/tests ... An idea?

I don't see there the error, I see only these:

I think here some problem with switching to user's language.
and 3. I am not sure but it seems new strings were replaced there so in tests we have old strings.

Can you fix the tests, or do need my help there?

…humhub/humhub into enh/6335-pending-approvals-send-message

marc-farre · 2023-12-21T15:19:27Z

Thanks @yurabakhtin
So the remaining problem is the test that tries to compare translated text.
But translations are not yet available as they will be done later on https://translate.humhub.org/

The problem is here:

humhub/protected/humhub/modules/user/tests/codeception/unit/ApproveUserFormTest.php

Lines 48 to 49 in d1e7162

    
           Die Erstellung Ihres Kontos wird derzeit überprüft. 
        
           Können Sie uns die Motivation hinter Ihrer Anmeldung nennen?

So I tried to manually add the translation here: 112048b#diff-6671c06c2f3ff265f9083ff45a2a8e3b158ca99492618f2cba2c53b51879710eR126-R127

But the problem remains.
An idea? Thanks!

yurabakhtin · 2023-12-22T10:27:30Z

So I tried to manually add the translation here: 112048b#diff-6671c06c2f3ff265f9083ff45a2a8e3b158ca99492618f2cba2c53b51879710eR126-R127

But the problem remains. An idea? Thanks!

@marc-farre I have compared the string and I see your translated and original strings have no \n\n in the last string {AdminName}, so you should either add this there or remove from here https://github.com/humhub/humhub/pull/6365/files#diff-50786b2517fe78c0e6d6e2da1e1a20acb43afe83066ab033fd535e02eaa235a7R387 - "{AdminName}\n\n",.

I have tested locally with deleted \r\n in the method getDefaultSendMessageMailContent() and after that the test testSendMessageIsSentInUserLanguage becomes green.

marc-farre · 2023-12-22T10:51:21Z

@yurabakhtin Thanks, indeed. I've added the 2 lines, and now checks seams to be OK.

I think @luke- should approve the changes.

@luke- Could you approve the changes? They are all resolved. Thanks!

luke-

@marc-farre Thanks looks file. Can you please try to remove the de message file from commit?

@Semir1212 Can you please take a look into the messages?

luke- · 2023-12-22T14:16:54Z

protected/humhub/modules/admin/messages/de/user.php

@@ -1,89 +1,89 @@
 <?php


Can you please revert changes to this file? Could cause problems with the Translation Site

luke- · 2023-12-22T14:17:08Z

protected/humhub/modules/admin/controllers/ApprovalController.php

+        $model->setSendMessageDefaults();
+        if ($model->load(Yii::$app->request->post())) {
+            if ($model->sendMessage()) {
+                $this->view->success(Yii::t('AdminModule.user', 'The message has been sent by email.'));


@Semir1212 Can you please check the message here?

luke- · 2023-12-22T14:17:12Z

protected/humhub/modules/admin/controllers/ApprovalController.php

+                $this->view->success(Yii::t('AdminModule.user', 'The message has been sent by email.'));
+                return $this->redirect(['index']);
+            }
+            $this->view->error(Yii::t('AdminModule.user', 'Could not send the message to the user!'));


@Semir1212 Can you please check the message here?

luke- · 2023-12-22T14:17:18Z

protected/humhub/modules/admin/controllers/ApprovalController.php

@@ -194,15 +219,22 @@ public function actionBulkActions()

        $model = new ApproveUserForm($usersId);

+        if ($action === self::ACTION_SEND_MESSAGE && $model->bulkSendMessage()) {
+            $this->view->success(Yii::t('AdminModule.user', 'The users were notified by email.'));


@Semir1212 Can you please check the message here?

marc-farre · 2023-12-22T14:21:45Z

Can you please try to remove the de message file from commit?

@luke- If I remove it, then tests fails, because of

humhub/protected/humhub/modules/user/tests/codeception/unit/ApproveUserFormTest.php

Lines 46 to 52 in d1e7162

    
                   $this->assertEquals("Hallo UnApproved User, 
        
           Die Erstellung Ihres Kontos wird derzeit überprüft. 
        
           Können Sie uns die Motivation hinter Ihrer Anmeldung nennen? 
        
           Mit freundlichen Grüßen 
        
           Admin Tester", $form->message);

luke- · 2023-12-22T16:01:51Z

Can you please try to remove the de message file from commit?

@luke- If I remove it, then tests fails, because of

humhub/protected/humhub/modules/user/tests/codeception/unit/ApproveUserFormTest.php

Lines 46 to 52 in d1e7162

$this->assertEquals("Hallo UnApproved User,

Die Erstellung Ihres Kontos wird derzeit überprüft.

Können Sie uns die Motivation hinter Ihrer Anmeldung nennen?

Mit freundlichen Grüßen

Admin Tester", $form->message);

Ok then I'll try to merge

marc-farre added 3 commits May 31, 2023 11:27

Enh #6335: Pending approvals: possibility to send a message

27a5e86

Enh #6335: Pending approvals: possibility to send a message

33760ea

Enh #6335: Pending approvals: possibility to send a message

bf37690

marc-farre mentioned this pull request May 31, 2023

Pending approvals: possibility to send a message #6335

Closed

marc-farre added 3 commits May 31, 2023 12:11

Added nb msg sent

c43f450

Added nb msg sent

f31fc2a

fixed tests

ac8e816

luke- requested a review from yurabakhtin May 31, 2023 12:22

yurabakhtin requested changes May 31, 2023

View reviewed changes

protected/humhub/modules/user/tests/codeception/unit/ApproveUserFormTest.php Outdated Show resolved Hide resolved

marc-farre added 3 commits June 1, 2023 08:41

fix test (removal of assertNotNull)

2be1f79

merged updated changelog

1fd3959

merged updated changelog

988c03f

marc-farre added 2 commits June 1, 2023 09:41

merged updated changelog

1092caf

merged updated changelog

9df5ea4

marc-farre added 2 commits June 1, 2023 10:10

fix testNonGroupManagerCannotSendMessageUnapprovedUser

afdd18f

merged updated changelog

1fb4d4c

Merge branch 'develop' into enh/6335-pending-approvals-send-message

5302beb

yurabakhtin approved these changes Jun 2, 2023

View reviewed changes

luke- requested changes Jun 30, 2023

View reviewed changes

luke- assigned Semir1212 Jun 30, 2023

allowing to send a bulk message to several pending approval users at …

6d998f1

…the same time

luke- assigned yurabakhtin and unassigned Semir1212 Oct 11, 2023

yurabakhtin and others added 2 commits October 16, 2023 08:08

Merge branch 'develop' into enh/6335-pending-approvals-send-message

fa29b01

Merge branch 'develop' into enh/6335-pending-approvals-send-message

37d4d3c

marc-farre mentioned this pull request Dec 12, 2023

Enh/6486 add module not available option for containers #6499

Merged

14 tasks

yurabakhtin approved these changes Dec 13, 2023

View reviewed changes

Merge branch 'develop' into enh/6335-pending-approvals-send-message

53875d6

luke- enabled auto-merge December 13, 2023 12:01

Merge branch 'develop' into enh/6335-pending-approvals-send-message

8953673

marc-farre and others added 4 commits December 21, 2023 15:31

Fixed test translations

bd36716

Merge branch 'enh/6335-pending-approvals-send-message' of github.com:…

733830b

…humhub/humhub into enh/6335-pending-approvals-send-message

Merge branch 'develop' into enh/6335-pending-approvals-send-message

d1e7162

Fixed test translations

112048b

Fix test translations

3fa501f

Merge branch 'develop' into enh/6335-pending-approvals-send-message

cc93c29

luke- requested changes Dec 22, 2023

View reviewed changes

Merge branch 'develop' into enh/6335-pending-approvals-send-message

ee11915

luke- disabled auto-merge April 4, 2024 12:23

luke- merged commit 30a6b78 into develop Apr 4, 2024
2 of 6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enh #6335: Pending approvals: possibility to send a message #6365

Enh #6335: Pending approvals: possibility to send a message #6365

marc-farre commented May 31, 2023

luke- commented May 31, 2023

marc-farre commented Jun 1, 2023

yurabakhtin commented Jun 1, 2023

marc-farre commented Jun 1, 2023

marc-farre commented Jun 1, 2023

marc-farre commented Jun 2, 2023

yurabakhtin commented Jun 2, 2023

marc-farre commented Jun 2, 2023

marc-farre commented Jun 2, 2023

luke- left a comment

marc-farre commented Aug 9, 2023

marc-farre commented Dec 13, 2023

yurabakhtin commented Dec 13, 2023

what-the-diff bot commented Dec 13, 2023

marc-farre commented Dec 20, 2023 •

edited

yurabakhtin commented Dec 20, 2023

marc-farre commented Dec 21, 2023 •

edited

yurabakhtin commented Dec 22, 2023

marc-farre commented Dec 22, 2023

luke- left a comment

luke- Dec 22, 2023

luke- Dec 22, 2023

luke- Dec 22, 2023

luke- Dec 22, 2023

marc-farre commented Dec 22, 2023

luke- commented Dec 22, 2023

Enh #6335: Pending approvals: possibility to send a message #6365

Enh #6335: Pending approvals: possibility to send a message #6365

Conversation

marc-farre commented May 31, 2023

luke- commented May 31, 2023

marc-farre commented Jun 1, 2023

yurabakhtin commented Jun 1, 2023

marc-farre commented Jun 1, 2023

marc-farre commented Jun 1, 2023

marc-farre commented Jun 2, 2023

yurabakhtin commented Jun 2, 2023

marc-farre commented Jun 2, 2023

marc-farre commented Jun 2, 2023

luke- left a comment

Choose a reason for hiding this comment

marc-farre commented Aug 9, 2023

marc-farre commented Dec 13, 2023

yurabakhtin commented Dec 13, 2023

what-the-diff bot commented Dec 13, 2023

PR Summary

marc-farre commented Dec 20, 2023 • edited

yurabakhtin commented Dec 20, 2023

marc-farre commented Dec 21, 2023 • edited

yurabakhtin commented Dec 22, 2023

marc-farre commented Dec 22, 2023

luke- left a comment

Choose a reason for hiding this comment

luke- Dec 22, 2023

Choose a reason for hiding this comment

luke- Dec 22, 2023

Choose a reason for hiding this comment

luke- Dec 22, 2023

Choose a reason for hiding this comment

luke- Dec 22, 2023

Choose a reason for hiding this comment

marc-farre commented Dec 22, 2023

luke- commented Dec 22, 2023

marc-farre commented Dec 20, 2023 •

edited

marc-farre commented Dec 21, 2023 •

edited