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: #65312 using regxp from v2 for url matching in text messages #108

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

Conversation

vj-venkatesan
Copy link
Contributor

@vj-venkatesan vj-venkatesan commented Jan 22, 2025

The url matcher Regular expression was not fulfilling links with characters such as umlauts. The same behavior is handled correctly in V2.
This PR replaces the current regx with the regx used in V2.
This PR also adds a test case to cover this use case

src/utils.ts Fixed Show fixed Hide fixed
src/utils.ts Fixed Show fixed Hide fixed
src/utils.ts Fixed Show fixed Hide fixed
src/utils.ts Fixed Show fixed Hide fixed
@sushmi21
Copy link
Contributor

The new regex is not recognizing the complete URL with query params
Screenshot from 2025-01-23 14-23-11

@sushmi21
Copy link
Contributor

The new regex is not recognizing the complete URL with query params Screenshot from 2025-01-23 14-23-11

You can check the message above under 'Text messages' tab of the demo page.
Screenshot from 2025-01-23 14-25-06

@vj-venkatesan
Copy link
Contributor Author

The new regex is not recognizing the complete URL with query params Screenshot from 2025-01-23 14-23-11

Ah I see , let me check . Is there any guide on the allowed patterns on URLs in cognigy docs or are we supposed to handle all possible scenarios ?

@sushmi21
Copy link
Contributor

The new regex is not recognizing the complete URL with query params Screenshot from 2025-01-23 14-23-11

Ah I see , let me check . Is there any guide on the allowed patterns on URLs in cognigy docs or are we supposed to handle all possible scenarios ?

No, we do not have any documentation. We should handle all scenarios.

@vj-venkatesan vj-venkatesan force-pushed the fix/#65312-broken-links-with-umlauts branch from f914e77 to c52866f Compare January 23, 2025 15:03
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.

2 participants