feat: refactor msg template components; add telegram character limit #1148
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
23 tasks
zwliew
changed the title
zwliew
commented
Apr 22, 2021
lamkeewei
reviewed
Apr 26, 2021
| @@ -11,40 +12,51 @@ import { | |||
| StepSection, | |||
| } from 'components/common' | |||
| import SaveDraftModal from 'components/dashboard/create/save-draft-modal' | |||
| import { exceedsCharacterThreshold, saveTemplate } from 'services/sms.service' | |||
| import { saveTemplate } from 'services/sms.service' | |||
There was a problem hiding this comment.
This component does not work with Telegram.
There was a problem hiding this comment.
Ah my bad. I guess this is a case where frontend tests would've caught the error haha
lamkeewei
requested changes
Apr 26, 2021
|
I think I'll move the unit tests for this component to this PR |
Features: * Disable the next button when the body lengh is greater than the error limit * Define a warning threshold of 3000 chars and an error threshold of 4096 chars for Telegram (4096 chars is the char limit for Telegram)
zwliew
force-pushed
the
refactor-frontend-templates
branch
from
7be2fa8
to
9cc7c23
Compare
lamkeewei
approved these changes
May 5, 2021
lamkeewei
added a commit
that referenced
this pull request
May 5, 2021
* develop: feat: refactor msg template components; add telegram character limit (#1148) refactor: use shared function to initialize models (#1172) chore: setup scaffolding for backend tests (#940) 1.23.0 fix(frontend): fix frontend test flakiness (#1162) fix: update successful delivery status only if error does not exist (#1150) chore: upgrade dependencies (#1153) feat: add unit tests for error states in critical workflows (#1118) feat: support whitelisting domains through `agencies` table (#1141) feat: add tests for happy paths in critical workflows (#1110) fix: prevent campaign names from causing dashboard rows to overflow (#1147)
lamkeewei
added a commit
that referenced
this pull request
May 10, 2021
* develop: fix: fix error when updating Telegram ID for an existing phone number (#1178) chore: upgrade React; use new JSX transform; sort imports (#1129) fix(backend): docker build and tsc output directory structure (#1177) feat: refactor msg template components; add telegram character limit (#1148) refactor: use shared function to initialize models (#1172) chore: setup scaffolding for backend tests (#940) 1.23.0 fix(frontend): fix frontend test flakiness (#1162) fix: update successful delivery status only if error does not exist (#1150) chore: upgrade dependencies (#1153)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Currently, the
SMSTemplateandTelegramTemplatecomponents are nearly identical. The only difference is thatSMSTemplatehas an error message (which I'll also add to the Telegram template in this PR)Since they're nearly identical, some unit tests written for both components will be duplicated for both of them as well (ref: #1118 (comment)).
To reduce code and test duplication, refactor both components to a common component.
Solution
Features:
exceedsCharacterThresholdwithwarningCharacterCountanderrorCharacterCountthresholdswarningCharacterCountmerely displays an error messageerrorCharacterCountdisplays an error message and disables the Next buttonImprovements:
SMSTemplateandTelegramTemplateto a common componentBefore & After Screenshots
To be added
The only visual difference is the Telegram template should look exactly the same as the SMS template now.
Tests
SMS and Telegram templates should function just as before
Exceeding error threshold should display error msg and disable the next button
Deploy Notes
No deploy notes.
Todo