-
Notifications
You must be signed in to change notification settings - Fork 44
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
Simulator functionality #558
Conversation
Codecov Report
@@ Coverage Diff @@
## master #558 +/- ##
==========================================
- Coverage 75.88% 75.82% -0.06%
==========================================
Files 132 133 +1
Lines 2641 2697 +56
Branches 495 508 +13
==========================================
+ Hits 2004 2045 +41
- Misses 455 464 +9
- Partials 182 188 +6
Continue to review full report at Codecov.
|
.env.example
Outdated
@@ -1,3 +1,4 @@ | |||
REACT_APP_API_PREFIX="api" | |||
SENTRY_DSN="" | |||
REACT_APP_GLIFIC_API_PORT=4000 | |||
REACT_APP_SIMULATOR_CONTACT_NUMBER="9876543210" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to define the phone number here? Since it will be used internally should we define it in constant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is something we're unsure of what exactly would be the number. So added this flexibility that it can easily changed in Glific installations by others
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not sending actual messages, so in a way, the number does not matter right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the number needs to be tracked by the backend as it is treated as a contact and the messages are stored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove "REACT_APP_SIMULATOR_CONTACT_NUMBER" from .env.example as it is defined in constant
@rathorevaibhav Also I am getting some weird errors in the console when I send a message in the simulator. Sorry, forgot to upload it earlier. |
src/config/index.ts
Outdated
@@ -25,3 +25,5 @@ export const USER_SESSION = GLIFIC_API_URL + '/v1/session'; | |||
export const RESET_PASSWORD = GLIFIC_API_URL + '/v1/registration/reset-password'; | |||
export const RENEW_TOKEN = USER_SESSION + '/renew'; | |||
export const FLOW_EDITOR_CONFIGURE_LINK = `${PROTOCOL}//${window.location.host}/automation/configure`; | |||
export const GUPSHUP_CALLBACK_URL = GLIFIC_BACKEND_URL + '/gupshup'; | |||
export const SIMULATOR_CONTACT = envVariables.REACT_APP_SIMULATOR_CONTACT_NUMBER; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove this as it is defined in constant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good now 👍
Summary
Simulator functionality
Test Plan
Added