-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Save user input during the chat for further registration (KIDS-877) #718
Conversation
@@ -11,7 +11,7 @@ | |||
"next": { | |||
"type": "textMessage", | |||
"side": "interlocutor", | |||
"text": "Great! Hi Peterson Family! I believe you're exactly what Tulsa needs to turn things around", | |||
"text": "Great! Hi lastName Family! I believe you're exactly what Tulsa needs to turn things around", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
There's currently a risk that one of the keys that you are using will now unintenionally replace text that does not need/want replacement. For example one of your keys is "email". Therefore a text like "We will send you an email at..." or "You might receive some emails..." will actually get replaced.
-
Having the enum ChatScriptSaveKey actually makes the use of the json a bit inflexible. It means that for every key we need to save we would have to implement it on the front-end. Ideally we could just change the hosted json files and not need to care about any app changes anymore.
Suggestion: Since you store and retrieve your user data as a Map<String, dynamic> I think both issues could be fixed by making use of a "Generic Template String".
https://stackoverflow.com/questions/57230201/generic-template-string-like-in-python-in-dart/57231174#57231174
https://gist.github.com/creativecreatorormaybenot/7c57666a53cbf21f2adab08b4ad84ef5
In the json, the part you want to replace, for example "lastName" would become "{lastName}". You can just delete the ChatScriptSaveKey class. In _formatChatTextWithUserData you would then do:
return TemplateString(source).format(userData) instead of the for loop you have now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TammiLion , thank you for the review!
Good point about the key words in texts, thanks. They are with brackets {} now.
Re TemplateString - I’ve found a similar https://pub.dev/packages/format package and the formatting part definitely looks more compact now.
I’m not sure about getting rid of ChatScriptSaveKey enum, since it was the point to have specific list of values we gonna use for both user registration and family profiles creation.
Amplitude tracking for the chat is also fixed (kids-962)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood!
…ault for user (saved) input; utm events removed from the scripts (kids-962 is fixed)
lib/features/children/generosity_challenge/repositories/generosity_challenge_repository.dart
Show resolved
Hide resolved
lib/features/children/generosity_challenge/cubit/generosity_challenge_cubit.dart
Show resolved
Hide resolved
lib/features/children/generosity_challenge_chat/chat_scripts/cubit/chat_scripts_cubit.dart
Show resolved
Hide resolved
@@ -38,6 +38,7 @@ dependencies: | |||
flutter_svg: ^2.0.9 | |||
flutter_timezone: ^1.0.8 | |||
font_awesome_flutter: ^10.6.0 | |||
format: ^1.5.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice find! 💪
No description provided.