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

Zero-width unicode characters removed from SMS messages too often #7676

Closed
garethbowen opened this issue Jul 13, 2022 · 3 comments
Closed

Zero-width unicode characters removed from SMS messages too often #7676

garethbowen opened this issue Jul 13, 2022 · 3 comments
Assignees
Labels
Affects: 3.16.0 Regression Affects a feature that worked in a previous release Type: Bug Fix something that isn't working as intended
Milestone

Comments

@garethbowen
Copy link
Member

Describe the bug
The fix introduced in #7654 is too aggressive and strips unicode characters from the entire message rather than just the parts needed for matching.

From @binokaryg

One minor caveat of this change is that if some text deliberately has zero-width characters, they would also be stripped out. These characters are very rarely used in general Nepali texting and the meaning and pronunciation remain the same, with or without them. For our use case, it might only be the name field that is potentially altered.

To Reproduce

Steps to reproduce the behavior:

  1. Register a new patient via SMS with the name र्‍याले
  2. See the name is changed to र्याले in the database

Also make sure we don't regress on the fix to #7654

Expected behavior
The name is a freetext field so should remain unchanged.

Environment

  • Instance: localhost
  • Browser: any
  • Client platform: any
  • App: api
  • Version: 3.16.0

Additional context
Add any other context about the problem here. What have you tried? Is there a workaround?

@garethbowen garethbowen added Type: Bug Fix something that isn't working as intended Regression Affects a feature that worked in a previous release Affects: 3.16.0 labels Jul 13, 2022
@garethbowen garethbowen self-assigned this Aug 9, 2022
@garethbowen garethbowen added this to the 4.0.0 milestone Aug 9, 2022
@garethbowen
Copy link
Member Author

This is ready for AT on 7676-more-selective-stripping

@tatilepizs tatilepizs self-assigned this Aug 12, 2022
@tatilepizs
Copy link
Contributor

Config: Standard
Environment: Local with docker helper script
Platform: WebApp
Browser: Chrome


Reproducible on Master

After adding a new person via SMS (N form) with the name र्‍याले, the person is created successfully but its name is changed to र्याले in the database

Test images

image
image
image
image

Fixed on 7676-more-selective-stripping

After adding a new person via SMS (N form) with the name र्‍याले, the person is created successfully and its name was saved correctly.

Test images

image
image
image
image

Test #7654 Not sure about this test, I had some problems uploading the Sunsari config and creating the place. @garethbowen please confirm if the test is correct.

image
image
image
image

@garethbowen
Copy link
Member Author

Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects: 3.16.0 Regression Affects a feature that worked in a previous release Type: Bug Fix something that isn't working as intended
Projects
Status: Done
Development

No branches or pull requests

2 participants