Problem/Motivation
In #3301370: Model core's standard install profile as recipes @nedjo wrote
It also does something that could potentially be modelled as a config action - sets the site email address as a recipient of the feedback contact form. To get a sense of how recipes might work, a useful exercise would be to look at different models of how the standard install profile might be divided up into recipes.
This brings up an interesting thought - how can we ask the user for information that the recipe can use while be applied. Things like site email address are one possible example but also say you are configuring a payment gateway - there are API keys and the like that need to be provided (thanks @mdencev).
Proposed resolution
Allow recipes to define inputs, and then use those inputs as poor-man's-token replacements in the config actions.
Recipes should be able to define inputs like this, providing different UIs with all the information they need to properly collect a value:
# Each item in `input` is an "input definition". This must be an associative array of arrays.
input:
# The name of the input. In config actions, the token `${recipient}` will be replaced by the inputted value.
recipient:
# Required: a primitive data type, known to Typed Data.
data_type: email
# Required: a human-readable description, which can be used in different ways by various UIs.
description: 'The email address of the person who will receive submissions from the contact form.'
# Optional validation constraints, in exactly the same way as they're done in config schema.
constraints:
Email: []
# `prompt` is optional, and things in it are only used when collecting input at the
prompt:
# The name of a method of Symfony Console's StyleInterface which can return a value.
method: ask
arguments:
# Arguments to pass to the method, keyed by parameter name.
question: 'Tell me who should get website feedback.'
# Required: a default value, if the input cannot be collected from the user.
default:
# Required: can be `config` or `value`.
source: config
# The `config` member is a two-element indexed array, with a config name and a property path, used when `source` is `config`.
config: ['system.site', 'mail']
# Used if `source` is `value`, can be anything.
value: Anything
The recipe can then use the input values in config actions, like so:
config:
actions:
contact.form.feedback:
setRecipients:
- ${recipient}
I propose one hard limitation up front: inputs cannot be used for dynamic IDs (like "enter the name of your content editor role"). Why? Because that makes recipes unpredictable, which would in turn make them less composable, which would violate a foundational design goal. Recipes are automatons, not functions. Inputs can only be used in the parameters passed to config actions.
(Having just written all that, I could see a way to maybe pull off dynamic IDs without breaking composability, but it would need further thought and hashing out, and we could always add it later. It's definitely out of this issue's scope.)
To help recipe consumers, we'll also add a new recipe:info console command, which prints the recipe's description, and lists every input defined by the recipe and its dependencies, along with their descriptions, in a nice table.
Remaining tasks
Implement it with test coverage.
API changes
Recipes will have some new syntax, but this does not introduce any changes to our data model or PHP API because the recipe system is experimental.
| Comment | File | Size | Author |
|---|---|---|---|
| #47 | 3303126-47-manual-testing.png | 242.55 KB | phenaproxima |
Issue fork drupal-3303126
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Issue fork distributions_recipes-3303126
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3303126-getting-user-input
changes, plain diff MR !150
Comments
Comment #2
aangel commentedI would find this useful for my recipe. I'm setting up a demo module and it needs a unique email address and password that the user must provide or the demo won't be completely set up (some tests will work but those involving emails won't).
https://github.com/Performant-Labs/atk_demo/blob/main/recipe.yml
Comment #3
aangel commentedHow about this?
1. Ask for input before work begins so the recipe can be cancelled with no side effects.
2. Running recipe with -y skips asking questions.
3. Control-C cancels.
4. Define questions with:
Which provides:
Enter email address ([email@null.com]):
The colon after the input text is provided by Recipes.
5. Input is stored as a variable in the form ${{ input.variable_name }}
6. Blank is allowed in which case the default is used.
7. Run variable_name through checks unless sanitize:no is specified.
8. Straight substitution occurs in actions with ${{ input.variable_name }}.
9. Provide input values on the command line with --input variable_name:.
10. Also provide input values via a file with --input-file .
Plus an additional section that provides tokens that the input above would override:
Maybe they can be merged.
Comment #4
aangel commentedWe could eliminate:
- variable: variable_name
And make the variable name the name of the section after
input:.Is there any reason to have the extra flexibility?
Comment #5
wim leersThe examples here so far are about unique identifiers: e-mail addresses, API keys, URLs, etc.
But there's a different pattern that also requires input: what if there's a recipe that requires some media type/node type/text editor/…, and you may want to apply it to 1 or more such config entity types? For example: adding a field, adding a view mode, adding a filter that references a field, adding a filter that references a view mode, et cetera.
Comment #6
phenaproximaWe ran into this limitation in #3417835-13: Convert the Standard install profile into a set of recipes.
In that case, we needed a recipe that creates a contact form to do one of two things:
system.site:mailconfig value into another piece of configBoth of these approaches are ways of collecting input, so I think they fall under the scope of this issue. I'm updating the issue summary to also mention this particular use case.
Comment #7
phenaproximaI was thinking about this and I had this idea for how we could define this stuff in recipes.
I propose we add a new section,
input, torecipe.yml. In this example, the recipe is creating a contact form. It asks the user for a recipient email address, with a default value coming from config:(Imagine for a sec that contact forms have a
addRecipientaction.)Some of my thoughts here:
inputdefines a named variable, which can then be reused in theconfig:actionssection.--input recipient=sisko@ds9.spaceThoughts on this idea?
Comment #8
wim leersPer #6, this blocks #3417835: Convert the Standard install profile into a set of recipes.
#7: I like your proposal! Thoughts:
--input-recipientis guaranteed to run into collisions. I think--input--website-feedback-contact-form--recipient=sisko@ds9.spaceis more verbose but also more reliable. Verbosity is less important in this context, predictability matters more.Comment #9
phenaproximaThis doesn't block #3417835: Convert the Standard install profile into a set of recipes; untagging. Sorry if my comment wasn't clear. We ran into the problem, but can work around it as described in #3417835-14: Convert the Standard install profile into a set of recipes.
That said, I'd guess this does block the recipe system being "finished" and fully usable by authors. So, escalating it.
Comment #10
phenaproximaResponding to the points in #8:
Agreed.
I think it might make sense for input definitions to have a
weightfield so that ordering can be tweaked if needed. Otherwise they should probably just be presented in the order that they are defined (inputs from dependencies come first, in their defined order; followed by the inputs from the recipe the user has asked to apply, in their defined order). All inputs in the "stack" would ordered by weight.That's a good point.
Yeah, I concur. Collisions are inevitable. Maybe the syntax could be something like
--input RECIPE_NAME.VARIABLE=foo.Comment #11
ltrainI notice that Drush GenerateCommands uses the Symfony Console component to get input from the user for use in the commands. Maybe it could be used here?
Comment #12
njim commentedI foresee some challenges with the current state of recipes that this sort of feature may address. In particular, there may be a need to map site-specific values to the configuration in the recipe. A few examples:
1. Avoid bloat when installing multiple recipes: For example, imagine a small business that is creating a site and wants to include a blog recipe, an event calendar recipe, an image gallery recipe, and a survey recipe. Individually, each one of these recipes may install functionally redundant configuration. A 'blog manager' role, an 'events calendar admin' role, an 'image gallery creator' role, a 'survey creator' role, etc. While it is valid for a user to have multiple roles, this can grow unnecessarily large. I can imagine a command line (or mapping file) that would prompt for things like the name of your content creator role.
2. Overcoming barriers with installing recipes on established sites: For this example, I'm a large, enterprise website that's been using Drupal for many years. I want to use a recipe that turns event content into different calendar feeds (rss, ical, and maybe some JSON standard.) I already have an event content type with named fields, so I don't want to create anything new in my data architecture. I just want to tell the recipe how to map the module's configuration to my content type. Typically we may create a layer of abstraction such as a module settings form or custom plugin. But in this case, I'm just interested in passing a few values such as the name of my content type to the script that is installing the recipe.
Just documenting as food for thought. There are multiple approaches to these issues.
Comment #13
wim leers#12
In the case of user roles, for example, it's probably necessary to allow the site builder using a recipe to choose to EITHER use an existing role OR create a new one.
Interesting … can you elaborate? Because AFAICT we need to support all of these use cases to make Recipes practically usable? 😄
I think that we need a way to allow a user to make a choice once, and to have multiple recipes follow that instruction. Lots of recipes would need to know the "content creator role", the "content moderator role", the "administrator role", et cetera. Hence to prevent spamming the user, we need a reliable mechanism to ask this question once, and for dependent recipes to respect that.
I think the clearest path forward (and I suspect the most reliable) is to use the one piece of information we already have and that is strongly related: recipe dependencies.
Example
Say a recipe BAR depends on a recipe FOO. Recipe FOO has an input for "content creator role". Because BAR depends on FOO, it knows that FOO already needs to know the ID of the content creator role. So FOO should still specify
but then BAR should NOT specify the same; instead it should specify:
Comment #14
phenaproxima#13: I like the idea of being able to explicitly use input values from a recipe you depend on.
Comment #15
thejimbirch commentedComment #16
thejimbirch commentedComment #17
phenaproximaSelf-assigning to start implementing this.
Comment #18
phenaproximaComment #20
phenaproximaGot an initial implementation up with some basic test coverage.
This only allows getting input from an existing config value (we can add prompting and such in another issue). It supports reusing inputs from recipes you depend on.
Comment #22
thejimbirch commentedI tested this successfully.
I had previously applied the standard recipe which included the feedback_contact_form recipe.
The Contact form needs to be deleted to re-apply.
I then changed the site email as this is the value that is being used in the recipe to set the contact form.
I then re-apply the feedback_contact_form and see that the changes email address is not the recipient.
Comment #23
alexpottI added some review comments to the MR. I agree with doing this on fork because I think until we have the ability to actually get user input we should not solidify the API by adding to core.
The issue title here is wrong because we're not getting user input here at all.
Comment #24
phenaproximaComment #25
phenaproximaComment #26
phenaproximaComment #27
phenaproximaComment #28
phenaproximaComment #29
phenaproximaComment #30
phenaproximaComment #31
phenaproximaComment #32
akhil babuI tested this in my local and it works perfectly. However no errors or warnigs were shown for invalid configurations like
Should we add a check to verify if the config exists or not?
Comment #33
phenaproximaComment #34
phenaproximaComment #35
b_sharpe commentedEverything looks great and resolved from previous review. Added one comment above about invalid sources. Leaving as NR to let others comment, but I feel this could be an easy fix/win. Current fatal is as such for reference:
Comment #36
phenaproximaI like that idea, @b_sharpe. Fixed! Also made some changes to accommodate what's coming in #3466374: Getting user input before applying a recipe.
Comment #37
b_sharpe commentedAll looks great, tests pass and expected results tested locally, RTBC
Comment #38
longwaveAdded some review comments/feedback, happy for all of it to be pushed back on but just some thoughts that came to mind while reviewing.
Comment #39
phenaproximaComment #40
phenaproximaComment #41
phenaproximaComment #42
phenaproxima@alexpott and I agreed that this can probably be committed directly to core, since its scope has been condensed to this one issue. Moving the issue, and will open a new MR.
Comment #45
phenaproximaComment #46
phenaproximaComment #47
phenaproximaGave this a quick manual test and it worked exactly the way you'd expect it to in interactive mode. Screenshot:
Comment #48
thejimbirch commentedComment #49
alexpottI've reviewed this work quite a few times now and I think it is in a great place and it ready. It puts us in a great position to support getting input via forms for recipe install via the UI and we've got tests for using the CLI. Great work @phenaproxima.
I think we need to document this capability in the recipe docs however we've still not landed the docs we wrote for recipes for the original core patch so it is hard to know what to do. I think the minimum we should be doing here is adding a CR based on the issue summary (while making sure that it is up-to-date). Once the CR has been created I think this is ready for RTBC.
Comment #50
phenaproximaCR written: https://www.drupal.org/node/3470507. Tagging for a final issue summary update.
Comment #51
phenaproximaThe issue summary is fully up to date!
Comment #52
thejimbirch commentedMarking as rtbc
Comment #53
moshe weitzman commentedWould be helpful if someone tried this MR with Drush recipe command - https://www.drush.org/13.x/commands/recipe/. Drush just includes the symfony command from Drupal as is so hopefully it works fine. Testing would be to make sure recipe options appear in command help and can be used successfully.
Comment #54
alexpottUpdating contribution credit. Crediting all those who have had input on the issue.
Comment #55
alexpottCommitted 1384a2e and pushed to 11.x. Thanks!
Comment #58
gábor hojtsyAdding to highlights
Comment #59
zaporylieRe:
Is there an issue where dynamic IDs as being actively discussed? For all Commerce projects, we'd need to require the currency input from users applying the recipe. Currencies are config entities and the currency code is featured in the config ID. My understanding is that creating a Currency entity programmatically based on the user input would not be possible with the current state of things until support for dynamic IDs is added which is effectively blocking Commerce-based recipes.
Comment #60
zaporylieIt seems like my understanding was wrong :) We are still able to define a custom config action plugin (
currencyImporter) that utilizes a currency importer service for dynamically defining currency config entity. The only non-clean part is the signature of theConfigActionPluginInterface::apply(string $configName, mixed $value): void;that requires$configNamewhich in my case would be something likecommerce_price.commerce_currency.XYZso it doesn't do much and feels hacky. Otherwise, it seems like I can achieve what I am aiming for.