Added send location feature #2635
Conversation
…to feature/send-location
Passing run #3781 ↗︎
Details:
Review all test suite changes for PR #2635 ↗︎ |
|||||||||||||||
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #2635 +/- ##
==========================================
- Coverage 73.32% 73.18% -0.14%
==========================================
Files 241 242 +1
Lines 8427 8459 +32
Branches 1796 1803 +7
==========================================
+ Hits 6179 6191 +12
- Misses 1644 1662 +18
- Partials 604 606 +2 ☔ View full report in Codecov by Sentry. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
.github/workflows/gpt-review.yml
Outdated
| types: | ||
| - opened | ||
| - synchronize | ||
| permissions: write-all |
There was a problem hiding this comment.
The permissions: write-all setting gives all permissions to all actions. This can be a security risk. Please consider limiting the permissions to only what is necessary for the workflow to function.
.github/workflows/gpt-review.yml
Outdated
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} | ||
| OPENAI_API_MODEL: "gpt-4" # Optional: defaults to "gpt-4" | ||
| exclude: "**/*.json, **/*.md" # Optional: exclude patterns separated by commas |
There was a problem hiding this comment.
The exclude pattern is excluding all .json and .md files. If there are any relevant files with these extensions that need to be included in the workflow, please adjust the pattern accordingly.
.github/workflows/gpt-review.yml
Outdated
| pull_request: | ||
| types: | ||
| - labeled | ||
| permissions: write-all |
There was a problem hiding this comment.
The types field should not be restricted to only the labeled event type if we want to review all pull requests. Consider adding more event types such as opened, synchronize, or reopened, unless there's a specific workflow that only requires the labeled event.
.github/workflows/gpt-review.yml
Outdated
| review: | ||
| if: ${{ github.event.label.name == 'review' }} | ||
| runs-on: ubuntu-latest | ||
| steps: |
There was a problem hiding this comment.
Using runs-on: ubuntu-latest is not always recommended for reproducible builds since 'latest' will always point to the latest stable version of Ubuntu. Specify an explicit version of the ubuntu runner like ubuntu-20.04 or ubuntu-22.04 to ensure consistent behavior as newer versions are released.
| @@ -152,6 +152,9 @@ export const GUPSHUP_QUICK_REPLY = | |||
| export const CALL_TO_ACTION = 'CALL_TO_ACTION'; | |||
| export const LIST = 'LIST'; | |||
| export const QUICK_REPLY = 'QUICK_REPLY'; | |||
| export const LOCATION_REQUEST = 'LOCATION_REQUEST_MESSAGE'; | |||
There was a problem hiding this comment.
It's usually better to prefix constants with a common namespace to avoid naming collisions especially with generic terms like LOCATION_REQUEST. Consider using a more explicit constant name like MESSAGE_TYPE_LOCATION_REQUEST.
| @@ -152,6 +152,9 @@ | |||
| export const CALL_TO_ACTION = 'CALL_TO_ACTION'; | |||
| export const LIST = 'LIST'; | |||
| export const QUICK_REPLY = 'QUICK_REPLY'; | |||
| export const LOCATION_REQUEST = 'LOCATION_REQUEST_MESSAGE'; | |||
| export const TERMS_OF_USE_LINK = 'https://tides.coloredcow.com/terms-of-use'; | |||
| export const COMPACT_MESSAGE_LENGTH = 35; | |||
There was a problem hiding this comment.
Defining the COMPACT_MESSAGE_LENGTH as a constant is good, but make sure to use it wherever appropriate throughout the application to maintain consistency.
| @@ -134,6 +134,8 @@ export const getInteractiveMessageBody = (interactiveJSON: any) => { | |||
| default: | |||
| break; | |||
| } | |||
| } else if (interactiveJSON.type === 'location_request_message') { | |||
There was a problem hiding this comment.
The usage of string literal 'location_request_message' should be replaced by the LOCATION_REQUEST constant to maintain consistency and ease of code maintenance.
| @@ -272,6 +284,7 @@ | |||
| const buttonType: any = { | |||
| QUICK_REPLY: { value: '' }, | |||
| LIST: { title: '', options: [{ title: '', description: '' }] }, | |||
| LOCATION_REQUEST_MESSAGE: {}, | |||
There was a problem hiding this comment.
The key 'LOCATION_REQUEST_MESSAGE' in the buttonType object does not match any of the defined constants. This may lead to an undefined template type. Use constants that are already defined, like LOCATION_REQUEST, for consistency.
| disabled: params?.id !== undefined, | ||
| placeholder: 'Type', | ||
| optionLabel: 'label', | ||
| }, |
There was a problem hiding this comment.
The placeholder Type is too generic; consider providing a more descriptive placeholder for the template type's autocomplete component to improve usability, such as 'Select interaction type'.
| @@ -237,7 +249,7 @@ | |||
| } | |||
| setSendWithTitle(sendInteractiveTitleValue); | |||
|
|
|||
| const getTagId = tag.tags.filter((tags: any) => tags.id === tagValue.id); | |||
| const getTagId = tag.tags.filter((tags: any) => tags.id === tagValue?.id); | |||
| if (getTagId.length > 0) { | |||
There was a problem hiding this comment.
Using an optional chaining (?.) operator without handling the scenario where tagValue is undefined can lead to runtime errors. Ensure appropriate default values or checks are in place to handle when tagValue is undefined.
| @@ -702,7 +744,8 @@ | |||
| }, | |||
| ]; | |||
|
|
|||
| let formFields: any = templateType === LIST ? [...fields] : [...fields, ...attachmentInputs]; | |||
| let formFields: any = | |||
| templateType === QUICK_REPLY ? [...fields, ...attachmentInputs] : [...fields]; | |||
There was a problem hiding this comment.
The conditional statement templateType === QUICK_REPLY ? [...fields, ...attachmentInputs] : [...fields] seems to only include attachment inputs for 'QUICK_REPLY'. Ensure that the intended behavior also accommodates the new 'LOCATION_REQUEST' type if applicable and that necessary inputs are being included.
| LOCATION_REQUEST_MESSAGE: 'Location request', | ||
| QUICK_REPLY: 'Quick Reply', | ||
| LIST: 'List', | ||
| }; |
There was a problem hiding this comment.
When updating the label to match new constant values, make sure to review the entire application for consistency. Promote the use of constants for these types to reduce duplication and ease future updates.
| version "1.19.3" | ||
| resolved "https://registry.yarnpkg.com/@glific/flow-editor/-/flow-editor-1.19.3.tgz#575f56a381b6c1bc6a381c2a69329b7b6d6989d1" | ||
| integrity sha512-gb3BfAYBnOs5Mb/C2bcSNHQuHk9gNLUHA8q1B7vYNk/ctogfCWUi4akHoGcvRNhtAp2vUpocOynk2NiVyE4H1Q== | ||
| "@glific/flow-editor@^1.19.3-1": |
There was a problem hiding this comment.
This change includes an update to a package version. Please make sure to test all areas of the application that are affected by this package after the update, to ensure that no issues have arisen from this update.
Summary
Added send location feature