Add a solution to the WhatsApp Cloud payload character limitation #1605
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
SNCFamiraBenkaci
force-pushed
the
16604_add_solution_limitation_payload_button
branch
from
327ed0d
to
f8e6cb0
Compare
vsct-jburet
reviewed
May 14, 2024
|
|
||
| object PayloadWhatsAppCloudMongoDAO: PayloadWhatsAppCloudDAO { | ||
|
|
||
| private const val COLLECTION_NAME = "sncv_whatsapp_payload" |
There was a problem hiding this comment.
I would prefer to write
private val collectionName = property("tock_whatsapp_payload", "whatsapp_payload")
| @@ -94,7 +94,7 @@ data class WhatsAppCloudBotActionButton( | |||
|
|
|||
| data class WhatsAppCloudBotActionButtonReply( | |||
| val title: String, | |||
| val id: String, | |||
| var id: String, | |||
There was a problem hiding this comment.
use val and copy() when you need to change it
| @@ -62,7 +62,7 @@ sealed class Component { | |||
|
|
|||
|
|
|||
| data class TextParameter(val type: ParameterType, val text: String) | |||
| data class PayloadParameter(val type: ParameterType, val payload: String?, val text: String) | |||
| data class PayloadParameter(val type: ParameterType, var payload: String?, val text: String) | |||
| @@ -17,6 +17,6 @@ | |||
| package ai.tock.bot.connector.whatsapp.cloud.model.webhook.message.content | |||
|
|
|||
| data class ButtonContent( | |||
| val payload: String, | |||
| var payload: String, | |||
| @@ -27,7 +27,7 @@ data class InteractiveContent( | |||
| ) | |||
|
|
|||
| data class ButtonReply( | |||
| val id: String, | |||
| var id: String, | |||
bot/connector-whatsapp-cloud/pom.xml
Outdated
| <dependency> | ||
| <groupId>org.litote.kmongo</groupId> | ||
| <artifactId>kmongo-async</artifactId> | ||
| </dependency> |
There was a problem hiding this comment.
remove kmongo-async - it should work with kmongo only
There was a problem hiding this comment.
Just wondering, what's the reason ?
SNCFamiraBenkaci
force-pushed
the
16604_add_solution_limitation_payload_button
branch
from
f8e6cb0
to
15ea187
Compare
Fabilin
reviewed
May 15, 2024
Comment on lines
56
to
58
| .capped(true) | ||
| .sizeInBytes(100000000) | ||
| .maxDocuments(50000) |
There was a problem hiding this comment.
I believe a capped collection may not be the most appropriate here - maybe just use a regular collection with a TTL index?
|
|
||
| object PayloadWhatsAppCloudMongoDAO: PayloadWhatsAppCloudDAO { | ||
|
|
||
| private val COLLECTION_NAME = property("tock_whatsapp_payload", "whatsapp_payload") |
There was a problem hiding this comment.
Would it be possible to document this property in the module's README?
There was a problem hiding this comment.
this is not a const so private val collectionName =
| } | ||
|
|
||
| override fun getPayloadById(id: String): String? { | ||
| return collection.findOneById(id)?.payload |
There was a problem hiding this comment.
Maybe first check that the id is a valid UUID before looking up the database?
| } | ||
|
|
||
| private fun updateButton(button: WhatsAppCloudBotActionButton): WhatsAppCloudBotActionButton = | ||
| if (button.reply.id.length > 256) { |
|
|
||
| private fun updateTemplateButton(button: Component.Button): Component.Button = | ||
| button.copy(parameters = button.parameters.mapNotNull { parameters -> | ||
| parameters.payload?.takeIf { it.length > 128 }?.let { |
| } | ||
| } | ||
|
|
||
| fun sendMessageNOTRef(phoneNumberId: String, token: String, messageRequest: WhatsAppCloudSendBotMessage) { |
Comment on lines
342
to
273
| messageRequest.template.components | ||
| .filterIsInstance<Component.Carousel>() | ||
| .flatMap { it.cards } | ||
| .flatMap { it.components } | ||
| .filterIsInstance<Component.Header>() | ||
| .flatMap { it.parameters } | ||
| .filterIsInstance<HeaderParameter.Image>() | ||
| .toList() | ||
| .forEach { imageHeader -> | ||
| imageHeader.image.id?.let { imageId -> | ||
| val newImageId = sendMedia(client, phoneNumberId, token, imageId, FileType.PNG.type).id | ||
| imageHeader.image.id = newImageId | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggested change
| messageRequest.template.components | |
| .filterIsInstance<Component.Carousel>() | |
| .flatMap { it.cards } | |
| .flatMap { it.components } | |
| .filterIsInstance<Component.Header>() | |
| .flatMap { it.parameters } | |
| .filterIsInstance<HeaderParameter.Image>() | |
| .toList() | |
| .forEach { imageHeader -> | |
| imageHeader.image.id?.let { imageId -> | |
| val newImageId = sendMedia(client, phoneNumberId, token, imageId, FileType.PNG.type).id | |
| imageHeader.image.id = newImageId | |
| } | |
| } | |
| messageRequest.template.components | |
| .asSequence() | |
| .filterIsInstance<Component.Carousel>() | |
| .flatMap { it.cards } | |
| .flatMap { it.components } | |
| .filterIsInstance<Component.Header>() | |
| .flatMap { it.parameters } | |
| .filterIsInstance<HeaderParameter.Image>() | |
| .forEach { imageHeader -> | |
| imageHeader.image.id?.let { imageId -> | |
| val newImageId = sendMedia(client, phoneNumberId, token, imageId, FileType.PNG.type).id | |
| imageHeader.image.id = newImageId | |
| } | |
| } |
avoiding unnecessary list creations and iterations - although ideally it should perform an object copy instead of mutating the message
SNCFamiraBenkaci
force-pushed
the
16604_add_solution_limitation_payload_button
branch
2 times, most recently
from
f7e0f2c
to
d065487
Compare
vsct-jburet
reviewed
May 16, 2024
| import com.github.salomonbrys.kodein.bind | ||
| import com.github.salomonbrys.kodein.singleton | ||
|
|
||
| val whastappCloudConnectorModule = Kodein.Module { |
There was a problem hiding this comment.
typo whatsAppCloudConnectorModule
vsct-jburet
reviewed
May 16, 2024
| } | ||
|
|
||
| fun isUUID(uuid: String): Boolean { | ||
| val uuidRegex = Regex("[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}") |
There was a problem hiding this comment.
This should be a val at class level in order to avoir calculate the regexp each time
SNCFamiraBenkaci
force-pushed
the
16604_add_solution_limitation_payload_button
branch
from
d065487
to
d1e894e
Compare
vsct-jburet
approved these changes
May 17, 2024
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.
resolve #1604