-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feature - SMS - Integración inicial del plugin de Seven para el envío de SMS #182
base: develop
Are you sure you want to change the base?
Conversation
Actions executed at: 2024-11-11 10:28:24. |
…rgiaCRM into feature/sevenSmsModules
…or from contact detail
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.
Ver comentarios.
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.
¿CreateMessage o SendMessage?
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.
Puedes crear un mensaje en draft, de ahí que no le llamase Send.
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.
Diría de seguir el mismo patrón que la acción de Emails, que se llama "sendEmail".
Si se quiere crear un mensaje en draft, se puede hacer desde la acción de crear registro.
custom/Extension/modules/AOW_Actions/Ext/Language/ca_ES.sticlang.php
Outdated
Show resolved
Hide resolved
|
||
class CustomAccountsViewDetail extends AccountsViewDetail | ||
{ | ||
use Checkstic_Messages; |
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.
Entiendo que las "trait" se utilizan para darle más características a la clase que los usa. En este sentido, yo creo que la función "echoIsMessagesModuleActive" no es una caracteristica del objeto "ViewDetail".
Por otro lado, creo que el nombre de la función tampoco encaja con lo que hace. Ya que no solo comprueba si el módulo está activo o no, sino otros temas aún más importantes.
Yo le daría una vuelta a este tema. Tenemos otros módulos que también muestran botones en Accounts y en otros (como por ejemplo los botones PDF, Incorpora) y estaría bien establecer un estandar para poder aplicarlo en todos los casos donde toque.
Si nos parece que con lo que hacemos con Incorpora no nos convence, revisemos lo que ya hace SuiteCRM o hagamos una propuesta consistente.
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.
Cuando creé el trait, estaba usando alguna variable de la clase para poder hacer lo que hace, y por eso necesitaba que fuera un trait y no podía ser una función de Utils. Tal como está ahora, puede incorporarse a Utils y denominarla algo como "includeMessagesJS".
Respecto a lo que hace Incorpora y los botones que se muestran en otros módulos, no he visto que pueda ocultarse la opción de sincronización con Incorpora, cómo sí queremos que suceda con el caso de los mensajes: si el módulo de mensajes no está activo no queremos que la acción aparezca. Por eso no podemos incluirlo directamente en el js de los módulos como se hace con Incorpora u otros módulos.
Otra diferencia clave con Incorpora es que añadimos el icono al lado de cada teléfono, lo cual obliga a insertar un código JS en esos casos....que también sólo queremos que aparezca el icono si hay módulo de mensajes activo.
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.
Lo añadiría al Utils.php entonces.
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.
Hecho
require_once 'SticInclude/Views.php'; | ||
|
||
class CustomAccountsViewList extends AccountsViewList | ||
{ | ||
use Checkstic_Messages; |
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.
Ídem.
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.
Ver comentarios.
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.
¿CreateMessage o SendMessage?
Puedes crear un mensaje y dejarlo como Draft
@@ -460,7 +460,7 @@ function viewType() { | |||
return "list"; | |||
} else if ($(".sub-panel .quickcreate form").length == 1) { | |||
return "quickcreate"; | |||
} else if ($(".detail-view").length == 1) { | |||
} else if ($(".detail-view").length == 1 || $("form[name=DetailView]").length == 1) { |
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.
¿En que casos se ha detectado que se debería ampliar la condición para la vista de detalle? Estas condiciones se llevan utilizando así desde hace tiempo en diferentes procesos y no hasta ahora no ha sido necesario.
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.
Si no me equivoco es en EmailTemplates. La vista detail se construye sin tpl y no sigue el estándar habitual.
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.
Diría de seguir el mismo patrón que la acción de Emails, que se llama "sendEmail".
Si se quiere crear un mensaje en draft, se puede hacer desde la acción de crear registro.
@@ -23,6 +23,7 @@ | |||
|
|||
require_once 'modules/Accounts/views/view.detail.php'; | |||
require_once 'SticInclude/Views.php'; | |||
require_once 'modules/stic_Messages/Checkstic_Messages.php'; |
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.
¿Ya no es necesario?
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.
La acción ya ha sido renombrada. Igualmente mantengo la opción de cambiar el estado y que no sea Sent, ya que creando el registro con la acción de "Crear registro" no tienes el mismo juego de destinatarios que te permite la acción específica.
En cuanto al Checstic_Messages ya está eliminada toda traza. Me quedaron colgados estos require y ya se corrigió hace unos días.
@@ -22,6 +22,7 @@ | |||
*/ | |||
|
|||
require_once 'modules/Accounts/views/view.list.php'; | |||
require_once 'modules/stic_Messages/Checkstic_Messages.php'; |
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.
Ídem
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.
Ídem
@@ -23,6 +23,7 @@ | |||
|
|||
require_once 'modules/Contacts/views/view.detail.php'; | |||
require_once 'SticInclude/Views.php'; | |||
require_once 'modules/stic_Messages/Checkstic_Messages.php'; |
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.
Ídem
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.
Ídem
@@ -23,6 +23,7 @@ | |||
|
|||
require_once 'modules/Employees/views/view.detail.php'; | |||
require_once 'SticInclude/Views.php'; | |||
require_once 'modules/stic_Messages/Checkstic_Messages.php'; |
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.
Ídem
@@ -22,6 +22,7 @@ | |||
*/ | |||
|
|||
require_once 'modules/Employees/views/view.list.php'; | |||
require_once 'modules/stic_Messages/Checkstic_Messages.php'; |
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.
Ídem
@@ -23,6 +23,7 @@ | |||
|
|||
require_once 'modules/Leads/views/view.detail.php'; | |||
require_once 'SticInclude/Views.php'; | |||
require_once 'modules/stic_Messages/Checkstic_Messages.php'; |
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.
Ídem
@@ -22,6 +22,7 @@ | |||
*/ | |||
|
|||
require_once 'modules/Leads/views/view.list.php'; | |||
require_once 'modules/stic_Messages/Checkstic_Messages.php'; |
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.
Ídem
modules/stic_Messages/Utils.php
Outdated
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.
Faltarían comentarios en las funciones que son susceptibles de ser usadas en procesos externos.
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.
Hecho
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.
Revisado el código
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.
Validando, he visto algunos comentarios sobre las distintas pruebas a realizar:
- Aunque el módulo esté inhabilitado, aparece el icono, pero no la opción en la vista de detalle ni en la lista; solo se muestra el icono.
7.Dentro de la ventana de mensaje, al seleccionar una plantilla de email, pueden aparecer etiquetas sin convertir. Quizá se podrían filtrar las plantillas para mostrar solo las de SMS.
-
Con un FdT, se puede cambiar el estado de un SMS. Además, me sucede lo siguiente: si cambio un SMS de "Enviado" a "Error" por medio de un FdT, aunque luego intente enviarlo en la vista de lista de Mensajes, el estado no se actualiza y se queda en "Error".
-
En los FdT, la opción de elegir "para quién" en el campo de teléfono no funciona. Tampoco se puede seleccionar una plantilla de SMS (aparece como plantilla de email, pero sale en blanco). Los emails sí funcionan.
|
Documento con algunas explicaciones mientras no se completa el PR: https://docs.google.com/document/d/1Kat53nJ9CJT1Qc97Lx6IbXTQ9-eUs-AxTUBnVqqkqjk/edit?usp=sharing
Descripción
Este PR añade a SinergiaCRM la funcionalidad para poder enviar mensajes SMS a teléfonos de Personas, Interesados, Empleados y Organizaciones.
Esta funcionalidad se ha implementado teniendo presente la posibilidad de tener nuevos tipos de mensajes (whatsapp) en el futuro así como la posibilidad que alguna entidad no quiera utilizar el proveedor de SMS que se ha seleccionado inicialmente (y para el que se ha desarrollado las llamadas API pertinentes).
Para ello se desarrollan los siguientes puntos:
Configuración
Uso de la funcionalidad
Una vez habilitado el módulo, existen 5 formas de generar mensajes:
Aspectos a tener presentes
Puntos pendientes
<?php require_once('modules/stic_Messages/Utils.php'); stic_MessagesUtils::$messageableModules['stic_Assessments'] = array('phoneField' => 'phone_office', 'name' => 'name');
Pruebas