Skip to content
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

fix(dialog): find flow now case insensitive #5771

Merged
merged 3 commits into from
Jan 12, 2022
Merged

fix(dialog): find flow now case insensitive #5771

merged 3 commits into from
Jan 12, 2022

Conversation

EFF
Copy link
Member

@EFF EFF commented Jan 5, 2022

Description

Find Flow usage is inconsistent, providing a flow name was case insensitive, providing a flow file name was case sensitive providing a poor user experience. The changes provide a quality of live improvement for botpress sdk users.

Potential risks:
Currently Botpress Studio allows for creating a flow with same name with different casing, if any user uses that weird feature, this is breaking. We'll have to change flow creation logic in studio (which IMO would be the best)

Another non breaking solution would be to make this 100% case sensitive all the time, but that might be breaking for some users as well

Example

Say I have a flow named HelpDesk stored as HelpDesk.flow.json
Using the jumpTo function

Before
jumpTo(/*args*/, 'helpdesk') ==> works
jumpTo(/*args*/, 'HelpDesk') ==> works
jumpTo(/*args*/, 'HelpDesk.flow.json') ==> works
jumpTo(/*args*/, 'helpdesk.flow.json') ==> not working

After
jumpTo(/*args*/, 'helpdesk') ==> works
jumpTo(/*args*/, 'HelpDesk') ==> works
jumpTo(/*args*/, 'HelpDesk.flow.json') ==> works
jumpTo(/*args*/, 'helpdesk.flow.json') ==> works

Copy link
Member

@allardy allardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with that breaking changes, makes sense

Copy link
Contributor

@davidvitora davidvitora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@EFF EFF merged commit dae17aa into master Jan 12, 2022
@EFF EFF deleted the f_fix-find-flow branch January 12, 2022 15:23
@EFF EFF mentioned this pull request Jan 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants