Change message field type to textarea in the System send email action.

The solution will rely on #2471481: Integrate Typed Data Widgets and #3091923: Add a new "text" datatype for long text

To cut through the long history (the issue was started in May 2016) read Comment #33 to #37, then skip to Comment #55

CommentFileSizeAuthor
#74 Change_message_field_to_textarea_in_Send_email_action-2724129-74.patch2.54 KBsl27257
#73 interdiff_70-73.txt709 bytesalison
#73 email_text_area-2724129-73.patch2.76 KBalison
#70 email_text_area-2724129-70.patch1.45 KBjoelrotelli
#69 email_text_area-2724129-69.patch1.45 KBrrotari
#68 email_textarea_via_typed_data.patch684 bytesNicasso
#64 2724129-64.email_textarea_via_typed_data.patch2.07 KBpaul_serval
#63 2724129-63.email_textarea_via_typed_data.patch1.39 KBpaul_serval
#62 2724129-62.email_textarea_via_typed_data.patch1.31 KBMartijn Houtman
#61 2724129-61.email_textarea_via_typed_data.patch696 bytesMartijn Houtman
#58 2724129-58.email_textarea_via_typed_data.patch4 KBjonathan1055
#48 rules-textarea-default-2724129-48.patch1.54 KBchrisolof
#46 rules_sendmail_textarea-2724129-46.patch4.18 KBKevin W
#40 Edit_Condition.png127.64 KBalex.skrypnyk
#40 Edit_reaction_rule.png80.38 KBalex.skrypnyk
#40 Edit_Action.png121.19 KBalex.skrypnyk
#35 rules_sendmail_textarea-2724129-34.patch4.1 KBAnas_maw
#29 rules_sendmail-2724129-28-8.0.patch1.64 KBsaranya ashokkumar
#20 message_after.png9.97 KByanniboi
#20 message_before.png9.86 KByanniboi
#18 453.patch3.01 KByanniboi
#17 453.patch.txt3.01 KByanniboi
#8 rules-2724129-8.patch1.78 KBToxaViking
#6 rules-2724129-6.patch1.79 KBToxaViking
#5 rules-2724129-1-8.x-3.x-dev.patch1.79 KBToxaViking
#2 rules-2724129-1-8.x-3.x-dev.patch1.88 KBToxaViking
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ToxaViking created an issue. See original summary.

ToxaViking’s picture

Status: Needs review » Needs work

The last submitted patch, 2: rules-2724129-1-8.x-3.x-dev.patch, failed testing.

The last submitted patch, 2: rules-2724129-1-8.x-3.x-dev.patch, failed testing.

ToxaViking’s picture

ToxaViking’s picture

FileSize
1.79 KB
yanniboi’s picture

+++ b/modules/rules/src/Form/Expression/ContextFormTrait.php
@@ -50,6 +50,7 @@ public function buildContextForm(array $form, FormStateInterface $form_state, $c
       '#type' => 'textfield',
+      '#type' => isset($context_definition->formElement) ? $context_definition->formElement : 'textfield',

You didn't remove the '#type' => 'textfield' line...

Other than that this looks like a good idea :)

ToxaViking’s picture

FileSize
1.78 KB

#7 deleted line

ToxaViking’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 8: rules-2724129-8.patch, failed testing.

yanniboi’s picture

Looks like tests failed:

  • ConfigEntityTest
  • RulesUiEmbedTest

But I cant tell much more from Drupals test logs. Maybe try creating a pull request on github? The test results there are often quite good.

ToxaViking’s picture

ToxaViking’s picture

Status: Needs work » Needs review
yanniboi’s picture

Status: Needs review » Needs work

Tests are failing: https://travis-ci.org/fago/rules/builds/131121886

I've commented on you pull request.

ToxaViking’s picture

ToxaViking’s picture

Status: Needs work » Needs review
yanniboi’s picture

FileSize
3.01 KB

Im uploading @ToxaViking's patch here so I can test it on simpletest.me.

yanniboi’s picture

FileSize
3.01 KB

Ahh wrong file extension :( (stupid windows...)

yanniboi’s picture

Status: Needs review » Needs work
yanniboi’s picture

Issue summary: View changes
Status: Needs work » Reviewed & tested by the community
FileSize
9.86 KB
9.97 KB

Great, @ToxaViking updated https://github.com/fago/rules/pull/453.

Tests pass, code reviewed and visual test passes too.

bisonbleu’s picture

I've applied patch in #18 but I still see just a text field - no textarea ?

Update: I ended up manually updating 8.x-3.x-dev based on GitHub pull request email body as text area #453 and I can now see the textarea.

ToxaViking’s picture

Assigned: ToxaViking » Unassigned

Please merge

klausi’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Looks good, left some small comments in the PR.

A test case is missing that asserts that the email action form now contains a text area for the message context. I would suggest a BrowserTest for that.

ToxaViking’s picture

changed

ToxaViking’s picture

Status: Needs work » Needs review
fago’s picture

Status: Needs review » Postponed

Thanks, this is a good start. However, instead of form elements we plan to have some light-weight widget API, see #2471481: Integrate Typed Data Widgets. Given that, I don't think this should be committed as it would be an interim API that just gets changed again anyway.
Howsoever, this can be a good base for the widget work. I'm setting this to postponed for now as we just need to take care of defining the typed data widgets api.

Jaypan’s picture

I noticed Typed Data has been implemented, but the mail body is still a textfield and not a textarea.

saranya ashokkumar’s picture

Status: Postponed » Needs review

Please use the patch core-new_textarea_typedata-2409059-37-8.0.patch(comment-#37) from https://www.drupal.org/node/2409059 after that apply the below patch "rules_sendmail-2724129-28-8.0.patch" to resolve this.

saranya ashokkumar’s picture

ckaotik’s picture

Just a stray thought of mine: We could get form elements for free if we went with a sensible config schema, wouldn't we? Config translation already uses those and form elements get generated depending on the type that is assigned in the schema (e.g. string for textfield and text for textarea).
Would be two birds with one stone, providing the ability for translation along with deduced form elements.

akcakaya’s picture

hi, I applied almost all patches fresh installed Drupal 8.x but no result. Is there any exact plan to add this patch to the Rules code? We would like to use Drupal 8 with Rules, how long we have to wait for stable release?

saranya ashokkumar’s picture

Hi akcakaya,

I tried the patch in new instance of drupal-8.2.7 to replicate your problem. But I didn't get any issue the patch is working fine for me. Can You please explain how did you apply the patch?

Steps To Apply this patch:
1. I have applied the patch core-new_textarea_typedata-2409059-37-8.0.patch from https://www.drupal.org/node/2409059#comment-11979816 .
2. Then apply patch from comment #29 .

Please try by this steps. If still you facing the same issue, tell me the steps to replicate.

fago’s picture

Status: Needs review » Postponed

Just a stray thought of mine: We could get form elements for free if we went with a sensible config schema, wouldn't we?

That's what we are doing, but based upon context definitions as they provide us with more, better metadata, such as knowledge about dates etc.

So as a next step, we need to get #2471481: Integrate Typed Data Widgets done to solve this. Also we need to implement https://www.drupal.org/node/2871403#comment-12047613 - which is a more approachable contributor-task in case someone is up for helping!

Anas_maw’s picture

@fago how we should make this work after committing https://www.drupal.org/node/2871403#comment-12047613 ?

Anas_maw’s picture

Reroll @ToxaViking patch from https://github.com/fago/rules/pull/453 to the latest dev version as temp fix until we have this done throw typed data.

max-kuzomko’s picture

@fago on the latest call we discussed that we need to create a typed data widget to cover the ItemList data type.

In this way we can provide the default widget id for the following data types, if a developer hasn't specified a widget id in the annotation:

text_input can cover: datetime_iso8601, duration_iso8601, email, float, integer, string, timespan, timestamp, uri
Select typed data widget can cover: filter_format
Item list typed data widget can cover item list data type

1. Do we need to create widgets for the following data types: any, binary, boolean, map, language_reference, language?
If no - what we need to return in ContextDefinition::getWidgetId() for the types above?
2. Which form element should represent the ItemList data type?

fago’s picture

1. Do we need to create widgets for the following data types: any, binary, boolean, map, language_reference, language?

I'd suggest we add a "BrokenWidget", id "broken" which is default for everything that cannot be input, or is not implemented. Then in the UI we just show an error message and avoid the form to be saved by providing a validation error.

For boolean, we can go with a textfield and entering 1 or 0 for now + creating a follow-up for a proper widget.

2. Which form element should represent the ItemList data type?

Let's use the usual simple textarea with one line being used per item + split it by line on saving. E.g. see the configuration form for available options of the ListItemInteger field type.

fago’s picture

Issue summary: View changes
fago’s picture

Issue summary: View changes

Let's move the discussion over to #2471481: Integrate Typed Data Widgets

alex.skrypnyk’s picture

FileSize
121.19 KB
80.38 KB
127.64 KB

#35 works with 8.3-alpha4

For poor souls like myself, who spent 2 hours trying to fins how the data selection works in D8, here are the screenshots of simple action: send email when a node of specific content type is created + use the value from the submitted field on content type.

https://www.drupal.org/files/issues/2018-07-14/Edit_reaction_rule.png
https://www.drupal.org/files/issues/2018-07-14/Edit_Condition.png
https://www.drupal.org/files/issues/2018-07-14/Edit_Action.png

alex.skrypnyk’s picture

pyxio’s picture

thanks a million. #35 works. is it possible to add tokens or any dynamic values in the text area? thanks for the great module.

rodmarasi’s picture

Status: Postponed » Needs review
TR’s picture

Status: Needs review » Postponed

This was postponed by @fago in #26:

Thanks, this is a good start. However, instead of form elements we plan to have some light-weight widget API, see #2471481: Integrate Typed Data Widgets. Given that, I don't think this should be committed as it would be an interim API that just gets changed again anyway.

Howsoever, this can be a good base for the widget work. I'm setting this to postponed for now as we just need to take care of defining the typed data widgets api.

(also see #33 and #37.)

The patch in #35 is just a re-roll of the pull request from back in #20 - there is nothing new, and the status should remain postponed.

Since @fago's comments, there HAS been movement in those other issue, so we are closer to a solution. Specifically, Typed Data Widgets, including a textarea widget, have been added to the Typed Data module (although only in the -dev). Getting these integrated into Rules has stalled a bit, but a couple of weeks ago I re-started work on that - see #2471481: Integrate Typed Data Widgets.

If you want to see this fixed in Rules, the best thing you can do to help are:
1) Participate in the Typed Data issue queue - https://www.drupal.org/project/issues/typed_data - by reviewing the pending patches and helping to get a new release published so that the Typed Data Widgets and other new features and bug fixes are available for most Rules users.
2) Participate in #2471481: Integrate Typed Data Widgets by helping to get a patch that passes the tests and fixes this issue. Plus we need tests written for that!

The development of this feature in Rules has been complicated by the fact that some of the work (like the above pull request) is only on GitHub, which requires @fago's active participation. He's not currently actively participating, so this is a bottleneck. By keeping the development HERE on drupal.org in the Rules issue queue, anyone/everyone can help out with the patch and the drupal.org testbot can review things.

TR’s picture

@drupalstrap:

#35 works. is it possible to add tokens or any dynamic values in the text area?

Well, yeah, of course - it may not be well documented, but you can use Twig-style placeholders, like {{ node.title.value }}, which work like tokens. Any piece of data that shows up in the data selector can be used as a token, by enclosing it in {{ }}. And if you use the -dev version of Typed Data, you can even use (and define your own) Twig-style filters, like {{ node.title.value|lower }} to lower-case the value. There are lots of examples in this issue queue. See #2991249: Send email with node URL on comment save for example.

Again, if you help out in https://www.drupal.org/project/issues/typed_data and help get a new release published, Rules will be able to automatically use all the new features developed over there.

Kevin W’s picture

One line update to match the latest DEV.

TR’s picture

Again, that patch is NOT the way we need to fix this. If you want to help, please contribute to #2471481: Integrate Typed Data Widgets as that's what needs to be done.

chrisolof’s picture

Here's a real simple fix that simply swaps the default input element from textfield to textarea. I believe this is the way D7 rules was set up and it always seemed to work pretty well (and was much less restrictive).

aiphes’s picture

I'm interested too. Which is the best way to have a texte area instead of a text field for email message ? Which patch to apply to or dev version is enought ?

TR’s picture

Best way is to contribute to #2471481: Integrate Typed Data Widgets because that will fix this and many related issues.

webdevfreak’s picture

I have tried all patches one by one i.e 48, 46, 35, 29 but none of them work for me.

Only thing I want to achieve is to replace MESSAGE textfield with textarea.

Can anyone advise a solution?

wombatbuddy’s picture

@webdevfreak,
I have created 'Rules Send Big Email' module as the temporary solution.

Sandoran’s picture

#52 Works for me! Thank you!
But now, when I switch to data selection, I do not see data available anymore. (Drupal -v 8.7.8)

wombatbuddy’s picture

@Sandoran, you can open a new issue here: https://www.drupal.org/project/issues/rules_send_big_email

TR’s picture

Category: Bug report » Task

I am leaving this open because it is easier to find this issue, but the solution has to come in #2471481: Integrate Typed Data Widgets so if this is important to you please contribute to that issue, not here.

rivimey’s picture

Please folk, can we decide on an actual fix? I tried the (current) state of typed data widgets without any apparent change in behaviour. If it is a case of RTM then please provide a pointer to the FM.

rivimey’s picture

TR, re #55, Jonathan1055 notes in #2471481:

As I understand it, this issue will not in itself fix the e-mail textarea problem directly, but it will allow #2724129: Make message field as textarea in Send email action to be sorted out properly, see comment 2471481#31 and 2471481#32

jonathan1055’s picture

Title: Make message field as textarea in Send email action » Change message field to textarea in Send email action
Issue summary: View changes
Related issues: +#2871403: Add a textarea input widget
FileSize
4 KB

I have done some investigation of what is required here. None of the patches and solutions already posted to this issue are actually what the maintainers want, as we need to use the widgets from Typed Data (and their pre-built form components) and not re-invent form elements here in Rules. I am tackling this with no prior understanding of the requirements other than what I have read in these two issues.

In #2471481-54: Integrate Typed Data Widgets patch #54 makes a working attempt at integrating the widgets (based entirely on previous peoples work). With that patch applied, here is my first attempt at using the typed data widget for the e-mail textarea. To try this patch, you will need to apply patch #54 from the other issue first.

I do not know if I am on the right track as per what @fago and @TR are wanting, so apologies if this is totally wrong. There is one big hack where the valid values of plugin ids for typed data does not include anyting like textarea. The valid plugins ids are any, binary, boolean, datetime_iso8601, duration_iso8601, email, float, integer, list, language, language_reference, map, string, timespan, timestamp, uri So I have hardcoded a switch from 'string' to 'bigtext' just to see how it works. This is not the proper way to do it, but I have not figured out any other way round (yet).

[If Rules had a drupalci.yml file then I could use that to apply the other patch first, then test this new one. The tests all pass when run locally]

If this is the correct approach then this work could be added in to the patch on #2471481: Integrate Typed Data Widgets and work could continue on that issue solely.

TR’s picture

Status: Postponed » Active

(Sigh, I had a nice long reply written when my browser crashed. I've tried to reproduce that reply but it's not as good as what I had before ... )

Yes, I wish that @fago would weigh in on some of these questions, because I don't know what he had in mind years ago.

None of the patches and solutions already posted to this issue are actually what the maintainers want

Correct. The whole idea behind Rules is that we should be able to use any datatype - not only core datatypes, but any and all core AND user-defined datatypes. That means we shouldn't have to 'know' about all datatypes, we should just be able to use them. That also means not having to make special cases and exceptions for (potentially) each and every condition and action. Rules shouldn't have to specify the form element to use in the condition or action context definitions, but should be able to determine which form widgets to use based solely on the datatype of the context variable. This means the datatype itself needs to contain the type/widget association.

There is one big hack where the valid values of plugin ids for typed data does not include anything like textarea

If I may rephrase ... In Rules, all context variables that require text input use the "string" datatype, as that is the only thing provided by core Drupal for text input. Thus, there is no way to differentiate between a context variable that accepts only a short string and needs a textfield from a context variable that accepts an arbitrarily long string and thus needs a textarea for input. That is true, and it's something I didn't fully appreciate until now.

In D7, all context variables that needed string input used a textarea for this input. The textarea was only a few rows, so it didn't overwhelm the UI with large textareas, but it was not ideal because when the condition/action only expected one short string you could still enter multiple lines, AND because when the condition/action expected long text (like with an email body) Rules only showed a few rows, making it impossible to see the whole thing at once. In D8, we instead currently use a textfield for all string input, which is also not ideal and in fact is less useful than in D7. But the intention all along was to make different inputs for 'short' and 'long' text in D8, as an improvement over D7.

In D8, core Drupal does NOT have a datatype for long text, but the Configuration API and the Field API both use the datatype "text" to refer to this concept. So that's the name I propose to use here as well. And the way to use it is to define a new datatype called "text" which is the same as "string" but intended for 'long' text. Defining a new datatype means that the widget to use is determined by the type, not by the context in which that type is used. And the place to put this new datatype is in the typed_data module.

The whole purpose of the Typed Data API Enhancements module (typed_data) is to add extensions, enhancements, and missing features to the core Typed Data API. The typed_data module already provides a textarea form widget, and I propose that's also where we should define a new "text" datatype - this is essentially a missing core feature. Note that typed_data is already a Rules dependency, and in fact all the code in that module used to be part of Rules ...

I think the fix to this current issue has to be done in three steps:

  1. Define a new "text" datatype in the typed_data module. See #3091923: Add a new "text" datatype for long text
  2. Modify all Rules actions and conditions to use "text" instead of "string" where appropriate. Notably in the email body of the "Send email" action, but probably in other places as well. The patch for this should be HERE in this issue.
  3. Fix #2471481: Integrate Typed Data Widgets, the purpose of which is to actually USE the form widgets provided by the Typed Data API module as input form elements when configuring conditions and actions. Because of the new datatype, that patch should actually be able to use different elements for "string" datatypes than for "text" datatypes.

Actually, step 3) should probably be (mostly) moved into the typed_data module - all Rules should have to do is getWidget() (or whatever the function is called) in order to get the form element for a datatype. Rules should not have to maintain a switch/case which maps datatypes to elements, because that's not extensible. What happens if someone defines their own datatype? Then what form element should be used? That information should be part of the datatype, and should be something that the typed_data module manages. FOR NOW, however, I am content to get this working, then it can be moved to typed_data in the future.

jonathan1055’s picture

Assigned: Unassigned » jonathan1055
Issue summary: View changes
Related issues: +#3091923: Add a new "text" datatype for long text

... there is no way to differentiate between a context variable that accepts only a short string and needs a textfield from a context variable that accepts an arbitrarily long string and thus needs a textarea for input.

That's great, I am pleased you said that, and that I was not on the wrong track. #3091923: Add a new "text" datatype for long text is the perfect way to solve it.

step 3) should probably be (mostly) moved into the typed_data module ... Rules should not have to maintain a switch/case which maps datatypes to elements

Yes that makes sense. As you say, for now we will keep it where it is, and I will work on the solution. Then when it is stable and working we can move some of the code.

Martijn Houtman’s picture

I know @TR said in #55 that this patch should not be further picked up in favour of #2471481: Integrate Typed Data Widgets, but that patch does not properly apply to -beta5, which I am using.

However, both modules came a long way since the above work, and with the latest TypedData support for type 'text', this module needs only a slight adjustment to make the textarea work. Please see a patch against -beta5.

Martijn Houtman’s picture

Forgot to change the type of the message field in SystemSendEmail from 'string' to 'text'. Updated patch attached.

paul_serval’s picture

paul_serval’s picture

Same patch as 63 but with a textarea in MailToUsersOfRole System in addition

ronaldmulero’s picture

#64 Works for me with message textarea.

Drupal 8.9.0
rules 8.x-3.x-dev

TR’s picture

Status: Active » Postponed

Please READ the issue. Patch #64 will not be committed. NONE of the patches in this issue will be committed. They are at best temporary hacks to give you partial functionality. A better use of your time and everyone else's time would be to test and otherwise help out with the real issue #2471481: Integrate Typed Data Widgets. Please try THAT patch and comment on THAT issue - that's where the input from the community is needed.

ronaldmulero’s picture

Somebody NEEDS a hug.

Nicasso’s picture

For those wanting a quick fix for 8.x-3.0-alpha6.

rrotari’s picture

This is the fix for 8.x-3.0-alpha6. Nicasso's patch is missing rules action.

joelrotelli’s picture

Same pach as #64 with a textarea in EmailToUsersOfRole System instead of MailToUsersOfRole

TTNT’s picture

Now that the depending issues are resolved, is there any chance a patch could be committed for this issue?

jonathan1055’s picture

Now that the depending issues are resolved,

You are mistaken, the issue #2471481: Integrate Typed Data Widgets has not been completed yet. See TR's comment in #66 above, there is no commit that will be done on this issue. The patches here are temporary.

alison’s picture

New version of the "temporary"/"band-aid" patch, to apply the change to the "rules_email_to_users_of_role" action, too (in addition to the "rules_send_email" action).

sl27257’s picture

I didn't find this one when I first did a quick search so I did an own version of it. But I see that this one is quite close to one above but with some added extra features. Anyhow here is my patch.

stevenx’s picture

#73 works for me. thanks

thomas.wardin’s picture

#73 and #74 only work for rules created afterwards. None of them works for a rule imported from an already patched config.

DBlog shows warning and error:

Warning: array_flip(): Can only flip STRING and INTEGER values! in Drupal\Core\Entity\EntityStorageBase->loadMultiple() (Zeile 312 in (site)/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php)

Error: Call to a member function get() on null in rules_webform_form_rules_reaction_rule_edit_form_alter() (Zeile 189 in (site)/web/modules/contrib/rules_webform/rules_webform.module)

sl27257’s picture

#76: I do fully agree with you! :) #74 is by no means a complete solution.

There are at least three cases that need to be handled, given the solutions in #74 (or #73):

  1. Upgrading from D7 to D9: This case have not been necessary for me, but there are methods for handling it using the upgrade route that exists in Drupal 8+.
  2. Upgrading an existing installation: Here a MODULE.install function must be written to do the casting.
  3. Importing a previous exported configuration. This one I have not seen any example of being done in Drupal. I am not sure that there is support for it. Please advice me if I am wrong!

Anyhow, given that this issue has been open for 7 (!) years and postponed many time (see the issue queue above) due to that a different solution approach is planned, my conclusion was that any other temporary solution will require some or even a lot of special handling, unfortunately.