stages/prompt: Add initial_data prompt field and ability to select a default choice for choice fields #5095

sdimovv · 2023-03-27T20:30:50Z

Details

Does this resolve an issue?
Resolves Choice fields: Ability to select arbitrary default value(s) #5063

Changes

New Features

Adds ability to specify an initial value for a prompt field (separate from a placeholder). Works on all flows not only on user-settings.
Adds ability to select a default choice when creating a fixed choice field (radio-button-group, dropdown)

Breaking Changes

Strips away some of the responsibilities of the placeholder field. Namely - the responsibility of giving initial value to prompts in the user settings flow.

Additional

The breaking change should not affect the default authentik config as that will be updated from the updated default blueprints automatically. However, any custom prompt fields or blueprints will need to be manually updated so that their initial value is specified in the "initial_value" field instead of in the "placeholder" field.

netlify · 2023-03-27T20:32:48Z

✅ Deploy Preview for authentik ready!

Name	Link
🔨 Latest commit	`0c09f22`
🔍 Latest deploy log	https://app.netlify.com/sites/authentik/deploys/642201bc4aa18c0008f06aeb
😎 Deploy Preview	https://deploy-preview-5095--authentik.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

codecov · 2023-03-27T20:59:33Z

Codecov Report

Patch coverage: 90.48% and project coverage change: -0.01 ⚠️

Comparison is base (a7fc579) 92.77% compared to head (facdc71) 92.76%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5095      +/-   ##
==========================================
- Coverage   92.77%   92.76%   -0.01%     
==========================================
  Files         506      506              
  Lines       25779    25837      +58     
==========================================
+ Hits        23913    23964      +51     
- Misses       1866     1873       +7

Flag	Coverage Δ
e2e	`52.61% <19.05%> (-0.08%)`	⬇️
integration	`26.29% <7.94%> (-0.09%)`	⬇️
unit	`89.58% <90.48%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
authentik/policies/password/tests/test_flows.py	`100.00% <ø> (ø)`
authentik/stages/prompt/api.py	`100.00% <ø> (ø)`
authentik/stages/prompt/models.py	`94.18% <77.78%> (-2.87%)`	⬇️
authentik/stages/prompt/stage.py	`99.21% <100.00%> (+0.02%)`	⬆️
authentik/stages/prompt/tests.py	`100.00% <100.00%> (ø)`

... and 8 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

sdimovv · 2023-03-29T16:15:48Z

@BeryJu, would it be possible to merge this before 2023.4 is out?

BeryJu

LGTM

looking back at this retroactively I think it would've made more sense to have an option to select some sort of "Prompt mapping" in prompts instead of specifying expressions directly, but I'm also not all too sure if that would actually be better.

Also one other thing that might be worth adding is a bit better migration, basically changing the fields in the same way as the blueprints were

authentik/stages/prompt/tests.py

sdimovv · 2023-03-30T16:36:38Z

Hi, thanks for taking a look at this!

looking back at this retroactively I think it would've made more sense to have an option to select some sort of "Prompt mapping" in prompts instead of specifying expressions directly, but I'm also not all too sure if that would actually be better.

Not sure I understand this correctly. Do you mean to add some extra fields in the model which do not apply to all prompt types? Until now I have been extremely careful not to add "prompt type specific" fields to the model, which is why this weird mapping to the placeholder was necessary for fixed choice prompts.

Otherwise the way I imagine this to work is for every prompt type to be a different model (if needed) in the DB with different properties. For example, fixed choice prompts will not have an a placeholder field but will instead have all_choices or something similar. Then the prompt creation endpoint will have a oneOf type API where you will be able to submit a request with the data needed by the specific prompt type you are creating.

However, I am not sure of the extent of (potentially breaking) changes this will introduce and did not want to submit such a PR without your approval and go-ahead first.

Also one other thing that might be worth adding is a bit better migration, basically changing the fields in the same way as the blueprints were

Do you mean to create a new migration that migrates placeholder fields to an initial value fields or? I haven't created a manual migration before so if this is required I might need to look up the Django docs on how to do that.

BeryJu · 2023-03-30T16:39:42Z

I meant instead of having the expression specified directly in the prompt, adding a ForeginKey to the mapping object, to make expressions re-usable (but again that's just random thoughts so don't worry about that too much)

For the migration I meant something like

authentik/authentik/stages/prompt/migrations/0009_prompt_name.py

Lines 9 to 21 in 9666d40

    
           def set_generated_name(apps: Apps, schema_editor: BaseDatabaseSchemaEditor): 
        
               db_alias = schema_editor.connection.alias 
        
               Prompt = apps.get_model("authentik_stages_prompt", "prompt") 
        
               for prompt in Prompt.objects.using(db_alias).all(): 
        
                   name = prompt.field_key 
        
                   stage = prompt.promptstage_set.order_by("name").first() 
        
                   if stage: 
        
                       name += "_" + stage.name 
        
                   else: 
        
                       name += "_" + str(uuid4()) 
        
                   prompt.name = name 
        
                   prompt.save()

(and

authentik/authentik/stages/prompt/migrations/0009_prompt_name.py

Line 36 in 9666d40

migrations.RunPython(code=set_generated_name),

) and add it to the migration in the PR to migrate data to the new fields

sdimovv · 2023-03-30T16:49:43Z

For the migration I meant something like

Ah, so basically for every prompt in the DB that has "evaluate as expression" set (except fixed choice fields):

prompt.initial_value = prompt.placeholder
prompt.initial_value_expression = True
promp.placeholder = ""
prompt.placeholder_expression = False

Is that correct?

sdimovv · 2023-03-30T17:35:07Z

I think that should do it?

sdimovv · 2023-04-07T20:29:31Z

Hi @BeryJu ,

does this need anything else?

tanberry · 2023-04-12T22:38:31Z

Hi @sdimovv quick question... you mention the "breaking change" of needing to manually update the any custom prompt fields or blueprints. Do you mention this in your docs in index.md? I didn't see it, but might have missed it. If it is not there, can you add a Note or some such to call it out? Thanks!

sdimovv · 2023-04-17T20:40:59Z

Hi @sdimovv quick question... you mention the "breaking change" of needing to manually update the any custom prompt fields or blueprints. Do you mention this in your docs in index.md? I didn't see it, but might have missed it. If it is not there, can you add a Note or some such to call it out? Thanks!

Hi @tanberry ,

Yes I don't think it is in the docs. However, since I wrote this, @BeryJu requested to add a manual migration step. You can see this here:

authentik/authentik/stages/prompt/migrations/0011_prompt_initial_value_prompt_initial_value_expression_and_more.py

Lines 8 to 22 in facdc71

    
           def migrate_placeholder_expressions(apps: Apps, schema_editor: BaseDatabaseSchemaEditor): 
        
               from authentik.stages.prompt.models import CHOICE_FIELDS 
        
               db_alias = schema_editor.connection.alias 
        
               Prompt = apps.get_model("authentik_stages_prompt", "prompt") 
        
               for prompt in Prompt.objects.using(db_alias).all(): 
        
                   if not prompt.placeholder_expression or prompt.type in CHOICE_FIELDS: 
        
                       continue 
        
                   prompt.initial_value = prompt.placeholder 
        
                   prompt.initial_value_expression = True 
        
                   prompt.placeholder = "" 
        
                   prompt.placeholder_expression = False 
        
                   prompt.save()

This should automatically migrate all prompts (except the new choice prompts) that interpret the placeholder as expression to the new initial_value field. So the breaking change should be handled automatically. I tested this to be the case on a test instance but I will be happy for you to try it out as well.

sdimovv added 5 commits March 25, 2023 01:28

Added initial_value to model

f455f12

Added initial_value to admin panel

3f36450

Added initial_value support to flows; updated tests

0cb39bd

Updated default blueprints

c2a750d

update docs

b44e0c2

Merge branch 'main' into feat-initial-data-prompt-field

0c09f22

sdimovv changed the title ~~stages/prompt: Feat initial_data prompt field and ability to select default choice value~~ stages/prompt: Add initial_data prompt field and ability to select a default choice for choice fields Mar 27, 2023

sdimovv added 3 commits March 28, 2023 00:21

Fix test

5cbd127

Fix another test

83a486e

Fix yet another test

5477197

BeryJu approved these changes Mar 30, 2023

View reviewed changes

authentik/stages/prompt/tests.py Show resolved Hide resolved

Add placeholder migration

b5ab2fe

Remove unused import

facdc71

BeryJu approved these changes Apr 19, 2023

View reviewed changes

BeryJu merged commit ee6edec into goauthentik:main Apr 19, 2023
51 of 53 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stages/prompt: Add initial_data prompt field and ability to select a default choice for choice fields #5095

stages/prompt: Add initial_data prompt field and ability to select a default choice for choice fields #5095

sdimovv commented Mar 27, 2023

netlify bot commented Mar 27, 2023 •

edited

codecov bot commented Mar 27, 2023 •

edited

sdimovv commented Mar 29, 2023 •

edited

BeryJu left a comment

sdimovv commented Mar 30, 2023 •

edited

BeryJu commented Mar 30, 2023

sdimovv commented Mar 30, 2023 •

edited

sdimovv commented Mar 30, 2023

sdimovv commented Apr 7, 2023

tanberry commented Apr 12, 2023

sdimovv commented Apr 17, 2023

stages/prompt: Add initial_data prompt field and ability to select a default choice for choice fields #5095

stages/prompt: Add initial_data prompt field and ability to select a default choice for choice fields #5095

Conversation

sdimovv commented Mar 27, 2023

Details

Changes

New Features

Breaking Changes

Additional

netlify bot commented Mar 27, 2023 • edited

✅ Deploy Preview for authentik ready!

codecov bot commented Mar 27, 2023 • edited

Codecov Report

sdimovv commented Mar 29, 2023 • edited

BeryJu left a comment

Choose a reason for hiding this comment

sdimovv commented Mar 30, 2023 • edited

BeryJu commented Mar 30, 2023

sdimovv commented Mar 30, 2023 • edited

sdimovv commented Mar 30, 2023

sdimovv commented Apr 7, 2023

tanberry commented Apr 12, 2023

sdimovv commented Apr 17, 2023

netlify bot commented Mar 27, 2023 •

edited

codecov bot commented Mar 27, 2023 •

edited

sdimovv commented Mar 29, 2023 •

edited

sdimovv commented Mar 30, 2023 •

edited

sdimovv commented Mar 30, 2023 •

edited