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(api): execute fallback message when no Global Fallback #552

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

omarNaifer12
Copy link

@omarNaifer12 omarNaifer12 commented Jan 11, 2025

Motivation

I just added an extra condition to check if fallback_block is not nullable, to ensure it dont enter to the condition and trigger catch block if it's nullable.If you don't want to duplicate the logic of sendMessageToSubscriber, I can remove it from the catch block.

Fixes #115

Type of change:

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have performed a self-review of my own code

  • New and existing unit tests pass locally with my changes

@medchedli medchedli added the SWOC Social Winter of Code 2025 label Jan 13, 2025
Copy link
Collaborator

@IkbelTalebHssan IkbelTalebHssan left a comment

Choose a reason for hiding this comment

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

Hi @omarNaifer12
Thank you for contributing to Hexabot 🙏 We really appreciate the effort you’ve put into this 👏 .
As seen in the screenshots below after disabling the global fallback the block- level one is not displayed.
Screenshot from 2025-01-20 14-56-50
Screenshot from 2025-01-20 14-55-19

@abdou6666 abdou6666 self-requested a review January 20, 2025 14:10
Comment on lines +489 to +504
this.sendMessageToSubscriber(event, {
id: 'global-fallback',
name: 'Global Fallback',
message: settings.chatbot_settings.fallback_message,
options: {},
patterns: [],
assign_labels: [],
starts_conversation: false,
position: { x: 0, y: 0 },
capture_vars: [],
builtin: true,
createdAt: new Date(),
updatedAt: new Date(),
attachedBlock: null,
} as any as BlockFull);

Copy link
Member

Choose a reason for hiding this comment

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

Great job on this implementation! I think it would be even better if you could refactor the code to avoid duplication in the catch block. By using a finally statement to send the message to the subscriber instead

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! I'm not sure where to include the finally statement but I noticed that if we remove the sendMessageToSubscriber from the catch block it works because if there is an error, the catch block doesn't return anything to close the function.

@marrouchi
Copy link
Contributor

marrouchi commented Jan 20, 2025

I think there's a misunderstanding about the nature of the issue, let's pause here until we validate the issue.

@omarNaifer12
Copy link
Author

Hi @IkbelTalebHssan, thank you for the feedback! Could you please clarify the purpose of enabling the fallback option in the box and what its message should achieve?

@omarNaifer12
Copy link
Author

omarNaifer12 commented Jan 20, 2025

Here is my understanding of issue #115:
When we send a message to the chatbot and it doesn't match any box it first checks the global fallback in the settings to return the fallback block. If the global fallback is not enabled the chatbot doesn't send a fallback message.
I have resolved the issue based on this condition only. Please let me know if my understanding of the issue is correct or if there are additional problems I may have missed

@IkbelTalebHssan
Copy link
Collaborator

Hi @IkbelTalebHssan, thank you for the feedback! Could you please clarify the purpose of enabling the fallback option in the box and what its message should achieve?

Hi @omarNaifer12,
The purpose of the block-level fallback message is to provide a customized response specific to a particular block when the user's input does not meet the expected criteria. This fallback is triggered when the user sends a message that cannot be accepted as valid input for that specific block.

For example, imagine you have configured a block that requires the user to input only numbers for a specific purpose, such as entering a phone number, age, or weight. If the user attempts to trigger this block by sending a message containing letters, special characters, or any invalid input, the block-level fallback message will be activated. This ensures that the user receives a tailored response explaining the issue, rather than triggering the global fallback (if one exists) or receiving no response at all (in cases where the global fallback is disabled).

In essence, the block-level fallback message serves as a context-aware error handler, improving user experience by providing clear, relevant feedback when their input does not align with the block's requirements.

@omarNaifer12
Copy link
Author

Thank you @IkbelTalebHssan,I will handle this case as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review SWOC Social Winter of Code 2025
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🤔 [ISSUE] - Block-Level Fallbacks Are Ignored When Global Fallback is Disabled
5 participants