Feature Language Change Conflicting Files Updates Requested PR-5.2-dev Pending

User tests: Successful: Unsuccessful:

avatar roland-d
roland-d
22 May 2022

Pull Request for Issue #37745 #37446 .

Summary of Changes

This change adds the option to select email as a notification type. Users can send notifications to:

  • Private messages (old and default behavior)
  • Email

image

Testing Instructions

First you need to apply this PR. Since there are database changes you will need to run those as well.

For MySQL users run this on your database:

INSERT IGNORE INTO `#__mail_templates` (`template_id`, `extension`, `language`, `subject`, `body`, `htmlbody`, `attachments`, `params`) VALUES
('plg_workflow_notification.mail', 'plg_workflow_notification', '', 'PLG_WORKFLOW_NOTIFICATION_EMAIL_ON_TRANSITION_SUBJECT', 'PLG_WORKFLOW_NOTIFICATION_EMAIL_ON_TRANSITION_MSG', '', '', '{"tags":["siteurl","title","performer","transitionName","toStage","extraText"]}');

For Postgres users run this on your database:

INSERT INTO "#__mail_templates" ("template_id", "extension", "language", "subject", "body", "htmlbody", "attachments", "params") VALUES
('plg_workflow_notification.mail', 'plg_workflow_notification', '', 'PLG_WORKFLOW_NOTIFICATION_EMAIL_ON_TRANSITION_SUBJECT', 'PLG_WORKFLOW_NOTIFICATION_EMAIL_ON_TRANSITION_MSG', '', '', '{"tags":["siteurl","title","performer","transitionName","toStage","extraText"]}')
ON CONFLICT DO NOTHING;
  1. Enable Workflows by going to Content -> Articles -> Options -> Integration -> Enable Workflow and set it to Yes
  2. Save the changes
  3. Go to Content -> Workflows
  4. Click on the number 7 (or another number) under Transitions
  5. Click on the Unpublish transition
  6. Click on Notification
  7. Notice there is now the option to select a notification type
  8. The Private Messages is the old and default way of sending notifications. Clicking on the dropdown now also let's you select Email as an option. Multiple options are possible as well
  9. Select a usergroup and/or a user where you want to send the message to
  10. Save the transition
  11. Go got Content -> Articles
  12. Edit an article
  13. Set the transition to Unpublished
  14. Save the article
  15. Check if the message has arrived

Mail templates

The sending of the emails uses the mail templates in Joomla. The mail template can be customized via these steps:

  1. Go to System -> Mail Templates
  2. Filter on workflow
  3. You will see 1 template.
    image
  4. Click on the template to edit it
  5. Modify the template to your needs

Feedback requested

The following questions I have:

  1. The name of the person doing the transition has a placeholder named performer Any suggestions for a better name?
  2. Are there more placeholders wanted for the mail template?

@brianteeman Could you please check the language files?

Final words

Thanks to @bembelimen and @softforge for the idea to add this. Thanks to @brianteeman to point out to use mail templates.

I know this isn't perfect code so looking for feedback to get this into Joomla to make transitions more useful as they are quite neat.

Actual result BEFORE applying this Pull Request

Messages are only sent to users who can receive private messages

Expected result AFTER applying this Pull Request

Messages can now also be send to users with an account and email address.

Documentation Changes Required

Yes, need to document the addition of the email option.

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
5.00

avatar roland-d roland-d - open - 22 May 2022
avatar roland-d roland-d - change - 22 May 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 22 May 2022
Category SQL Administration com_admin Postgresql Language & Strings Installation Front End Plugins
avatar Kostelano
Kostelano - comment - 22 May 2022

Great addition.
I tested a little, I'll leave a comment.

The "Additional Message Text" field must be removed. First, we find ourselves in a similar situation, which I described in the issue #37493. Secondly, if we add text in this field and we select both methods of receiving a notification, then we will receive this additional text in personal messages, but we will NOT receive it in email, since this additional text will not be in the letter template. Thirdly, in this field we do not see the real message (only the description), but we see it in the email template.

In short, we really need to leave 1 place for setting the "body" of the notification and this place in the email templates. We can mention this in the description for the "Notification Type" parameter.

avatar brianteeman
brianteeman - comment - 22 May 2022

Doesn't make any sense to keep the old broken behaviour as an option You're introducing a new method that fixes the bugs in the old method but still leave the old method there with its bugs

avatar brianteeman
brianteeman - comment - 22 May 2022

You cannot have a required field that is conditionaly displayed.

You have made the notification type a required field but is only shown when send email is set to yes.

avatar brianteeman
brianteeman - comment - 22 May 2022

I really dont undrstand why you have kept the broken private messages version. There is no explanation or why you would use method email or method private messags and you can still select groups and users to receive private messages which will neveer be sent.

avatar brianteeman
brianteeman - comment - 22 May 2022

It needs to be clearer that you can customise the mail further if you are using the email method by using th email templates BUT that it is a generic template for all transition notification emails and you must put anything specific to the transition in the notification config only.

avatar brianteeman
brianteeman - comment - 22 May 2022

In what scenario would anyone ever send a message by email AND private message?

avatar roland-d roland-d - change - 23 May 2022
Labels Added: Language Change ?
avatar roland-d
roland-d - comment - 23 May 2022

@Kostelano

The "Additional Message Text" field must be removed.

This cannot be removed as it will be a backwards compatible break.

Secondly, if we add text in this field and we select both methods of receiving a notification, then we will receive this additional text in personal messages, but we will NOT receive it in email, since this additional text will not be in the letter template.

You can add the additional text to the email. This is a valid point that the extra text should be in the default mail template. I have updated this PR with that change.

Thirdly, in this field we do not see the real message (only the description), but we see it in the email template.

I do not understand this. Can you explain this further?

In short, we really need to leave 1 place for setting the "body" of the notification and this place in the email templates.

That is something I was thinking about but really have no idea how we should do this. First I cannot remove this addition message text due to backwards compatibility break. Second, we could create a mail template for every transition but these cannot be in core because we have no idea which transitions there will be.

@brianteeman

Doesn't make any sense to keep the old broken behaviour as an option

I cannot remove sending messages to private messages because that would be a backwards compatible break. I would not say it is broken but rather limited as we can only send messages to users with access to private messages.

You cannot have a required field that is conditionaly displayed.

Actually you can. The required condition is only active when the field is displayed.

There is no explanation or why you would use method email or method private messags

Any suggestions for a text to add?

you can still select groups and users to receive private messages which will neveer be sent.

I saw your other issue about that but this is not in scope of this PR. I can look at that in a separate PR.

It needs to be clearer that you can customise the mail further if you are using the email method by using th email templates BUT that it is a generic template for all transition notification emails and you must put anything specific to the transition in the notification config only.

See my remark above for Kostelano, he basically says the same thing. Any suggestions on how to improve this are welcome.

In what scenario would anyone ever send a message by email AND private message?

I knew this was coming :) If I chose a single dropdown the request would come to make it multiple, make it multiple and the request will come for a single dropdown. Ghehehehe.

So my reasoning is that the transition message can be send but may not arrive in the mail but you can see it in your private message box. Why limit the options for the user? We can let the user decide what they want to use, we do not have to decide for them.

avatar brianteeman
brianteeman - comment - 23 May 2022

Doesn't make any sense to keep the old broken behaviour as an option

I cannot remove sending messages to private messages because that would be a backwards compatible break. I would not say it is broken but rather limited as we can only send messages to users with access to private messages.

No its not B/C break you are just changing from a completely broken method to a working method

You cannot have a required field that is conditionaly displayed.

Actually you can. The required condition is only active when the field is displayed.

I do not write things just for the sake of writing words. I tested the PR. (and I know that we already have the same bug elsewhere.)

image

image

you can still select groups and users to receive private messages which will neveer be sent.

I saw your other issue about that but this is not in scope of this PR. I can look at that in a separate PR.

Of course it is in the scope of this PR. Otherwise it doesn't fix the bug you state this pr fixes.

avatar brianteeman
brianteeman - comment - 23 May 2022

@Kostelano you need the "additional text" field. Without it you cannot have a unique message for each notification.

Mail templates are for general customisations of all notification emails

avatar roland-d
roland-d - comment - 23 May 2022

No its not B/C break you are just changing from a completely broken method to a working method

It works on the site where we are using workflow. Messages go to the private message box.

I do not write things just for the sake of writing words. I tested the PR. (and I know that we already have the same bug elsewhere.)

I also tested this of course but did not read between your lines that there should not be an option selected. I removed the requirement.

avatar brianteeman brianteeman - test_item - 23 May 2022 - Tested unsuccessfully
avatar brianteeman
brianteeman - comment - 23 May 2022

I have tested this item ? unsuccessfully on a94c821

Does not fix the reported problems


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37860.

avatar Kostelano
Kostelano - comment - 23 May 2022

If you add some text in the transition field, then it will be sent to the mail with a visible br tag, although I just added the text in the field. This is the EMAIL method.

Screenshot_1

There is no br tag in private messages.

avatar brianteeman
brianteeman - comment - 26 May 2022

How about updating the text below the selection of the notification type with something like

image

It wont prevent someone trying to send to users who won't receive the private messages email but at least they have been warned

avatar roland-d
roland-d - comment - 27 May 2022

There is no br tag in private messages.

Good catch. I moved it to the private message where it comes from.

So I have updated this PR to also check if there is any recipient selected when saving the transition and if not, an error message is shown and the transition is not saved. The same goes if for some reason a user manages to save with no notification types selected. That should fix the issue of enabling the notification but not selecting any recipients.

avatar richard67 richard67 - change - 6 Jun 2022
Labels Added: Conflicting Files
avatar richard67
richard67 - comment - 6 Jun 2022

@roland-d I've allowed myself to fix the conflict in the supports.sql files.

avatar joomla-bot
joomla-bot - comment - 27 Jun 2022

This pull requests has been automatically converted to the PSR-12 coding standard.

avatar HLeithner HLeithner - change - 27 Jun 2022
Labels Added: ?
Removed: Conflicting Files
avatar roland-d
roland-d - comment - 3 Jul 2022

Rebased to 4.3

avatar spamzini
spamzini - comment - 5 Sep 2022

Hi,

i'v upgraded a CLEAN installation of joomla 4.2.0 to the last one 4.2.2.
I'v upgrade by the file upload (administrator/index.php?option=com_joomlaupdate&view=upload) and not by the automatic update
The check of the database by the internal function .../administrator/index.php?option=com_installer&view=database shows two errors of database:
One of this was the XXXX_mail_templates has MEDIUMTEXT character set.
I solved with this SQL command:

ALTER TABLE XXXX_mail_templates CHANGE htmlbody htmlbody MEDIUMTEXT CHARACTER SET utf8 COLLATE utf8_unicode_ci NOT NULL;


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37860.

avatar obuisard obuisard - change - 23 Oct 2022
Labels Added: PR-4.3-dev
Removed: ? ?
avatar richard67
richard67 - comment - 23 Oct 2022

@roland-d As the PR has been rebased to 4.3-dev, the update SQL scripts "4.2.0-2022-05-22.sql" need to be renamed to something later than the up to now latest 4.3 update SQL scripts "4.3.0-2022-09-23.sql". I would suggest "4.3.0-2022-10-23.sql".

avatar richard67
richard67 - comment - 23 Oct 2022

@obuisard Please don't merge this before the update SQL scripts have been renamed, see my previous comment.

avatar obuisard
obuisard - comment - 23 Oct 2022

@obuisard Please don't merge this before the update SQL scripts have been renamed, see my previous comment.

No worries Richard, I was not going to merge it as I saw your comments :-)
Thank you for your help!

avatar Magnytu2 Magnytu2 - test_item - 12 Jan 2023 - Tested unsuccessfully
avatar Magnytu2
Magnytu2 - comment - 12 Jan 2023

I have tested this item ? unsuccessfully on cd97f4b

Still no mail received after a transition. The private message notification works, but not the email.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37860.

avatar obuisard obuisard - change - 12 Jan 2023
Title
[4.2] Workflow emails
[4.3] Workflow emails
avatar obuisard obuisard - edited - 12 Jan 2023
avatar HLeithner
HLeithner - comment - 8 May 2023

This pull request has been automatically rebased to 5.0-dev. No new features will be merged into Joomla! 4.3 series. Joomla! 4.4 series is a bridge release to make migration from Joomla! 4 to 5 as smooth as possible.

avatar micker
micker - comment - 19 Jun 2023

hello i would help to test this but after save my item i have this message "No notifications sent as there are no users to send this message to." bug or need to configure something ?
image
regards

avatar richard67
richard67 - comment - 19 Jun 2023

@micker If there is no other super user besides the one who causes the notification, then there is nobody to send email to. Create another user user and that other one should get the email if the first super user changes something and your email sending is set up right.

avatar micker micker - test_item - 19 Jun 2023 - Tested successfully
avatar micker
micker - comment - 19 Jun 2023

I have tested this item successfully on cd97f4b

thanks @richardnet its great now
@roland-d tested and validate !

maybe 2 return ?

avatar obuisard obuisard - change - 9 Sep 2023
Title
[4.3] Workflow emails
[5.0] Workflow emails
avatar obuisard obuisard - edited - 9 Sep 2023
avatar HLeithner
HLeithner - comment - 30 Sep 2023

This pull request has been automatically rebased to 5.1-dev.

avatar HLeithner
HLeithner - comment - 24 Apr 2024

This pull request has been automatically rebased to 5.2-dev.

avatar HLeithner HLeithner - change - 24 Apr 2024
Title
[5.0] Workflow emails
[5.2] Workflow emails
avatar HLeithner HLeithner - edited - 24 Apr 2024
avatar carcam
carcam - comment - 17 Jul 2024

I'm trying to apply the patch to test this but I get this error:

The file marked for modification does not exist: plugins/workflow/notification/notification.php

It seems the notification plugin was updated to the new architecture and you need to update your pr @roland-d . Can you please check?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37860.

avatar roland-d roland-d - change - 18 Jul 2024
Labels Added: Feature Conflicting Files Updates Requested PR-5.0-dev PR-5.2-dev
Removed: PR-4.3-dev
avatar roland-d
roland-d - comment - 18 Jul 2024

Just a heads up, I have merged the 5.2 dev into this branch but I have not tested anything yet. More to follow later.

avatar roland-d roland-d - change - 4 Aug 2024
Labels Removed: PR-5.0-dev
avatar cybersalt cybersalt - test_item - 21 Aug 2024 - Tested unsuccessfully
avatar cybersalt
cybersalt - comment - 21 Aug 2024

I have tested this item ? unsuccessfully on 1f172e8

I followed all of the steps - selecting the notification to go to the superuser group.

Upon saving the article this was the error message:

Menu
Frontend
An error has occurred.
0 Joomla\Plugin\Workflow\Notification\Extension\Notification::getUsersFromGroup(): Argument #1 ($data) must be of type Joomla\Plugin\Workflow\Notification\Extension\stdClass, stdClass given, called in /home/basicjoo/j5core.basicjoomla.com/stageit/plugins/workflow/notification/src/Extension/Notification.php on line 195
Call Stack

Function Location

1 () JROOT/plugins/workflow/notification/src/Extension/Notification.php:392


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37860.

avatar HLeithner
HLeithner - comment - 2 Sep 2024

This pull request has been automatically rebased to 5.3-dev.

avatar HLeithner HLeithner - change - 2 Sep 2024
Title
[5.2] Workflow emails
[5.3] Workflow emails
avatar HLeithner HLeithner - edited - 2 Sep 2024

Add a Comment

Login with GitHub to post a comment