Hotfix - Encuestas - Duplicación de preguntas mediante un flujo de trabajo

[ainaraRT]

• Closes Incidencia - Encuestas - Al duplicar una encuesta se duplican las preguntas en la encuesta duplicada #91

Description

Como se describe en el issue, al tener un FdT y duplicar una encuesta o crear se duplicaban también las preguntas, ya que el código se ejecutaba sin ninguna verificación del flujo de trabajo. Esto era debido a que:

• Cuando se duplicaba la encuesta X a Y o se creaba la encuesta X, teniendo activo el FdT, el código guardaba las preguntas normalmente.
• Pero debido a que no había control sobre el flujo de trabajo, cuando se ejecutaba el workflow:
  • El código volvía a ejecutar el bucle foreach que guarda las preguntas.
  • Como resultado, las preguntas se guardaban dos veces: una vez durante el guardado normal y otra durante la ejecución del workflow.

Por ello, al principio, con el FdT, la encuesta se guardaba con 4 preguntas en lugar de 2:

• 2 preguntas del guardado inicial
• 2 preguntas adicionales del workflow

Se ha solucionado el problema al agregar una variable de control already_saved que se usa para verificar que no se vuelva a ejecutar el método. Si already_saved está definida, significa que el método ya fue ejecutado y se evita procesar nuevamente las preguntas.

How To Test This

1. Crear una encuesta con varias preguntas con Estado->Borrador
2. Crear un Flujo de Trabajo

• Módulo base: Encuestas
• Ejecutar al guardar
• Registros nuevos
• Acción:
  • Modificar registro, por ejemplo, pasar de estado borrador a publicado.
  • Calcular campos, por ejemplo, calcular el nombre.

3. Duplicar la encuesta del paso 1, comprobando el número de preguntas que tiene.
4. Comprobar que se ha duplicado la encuesta sin duplicar las preguntas y que tiene las mismas que la encuesta del paso 1.
5. Crear una encuesta con varias preguntas y ver que no se duplican las encuestas.

• Adicional:

1. Teniendo el FdT Inactivo.
2. Crear una encuesta con varias preguntas y ver que no se duplican las encuestas.
3. Duplicar dicha encuesta y verificar que siguen estando las mismas preguntas

[github-actions]

Actions executed at: 2024-12-17 16:12:06.

[ManuSinergiaCRM]

(A)probado

[AlbertoSTIC]

Para evitar que se ejecute en el FDT, estamos mirando la variable $this->logicHookDepth["after_save"]. Pero puede que sí queramos que se ejecute en un after_save. Pienso que hay otra variable disponible que referencia explícitamente el FdT. ¿Se ha revisado en otros contextos del módulo del core de SuiteCRM para ver cómo lo hacen?

[ManuSinergiaCRM]

Buenas, la propiedad $this->in_workflow no parece que funcione como su nombre indica.

Debuggeando en la línea en cuestión he visto que en las diferentes paradas que se realizan en las pruebas, tanto en la del save() de la encuesta duplicada como en las dos acciones del FdT creado, dicha propiedad siempre está a false. Haciendo una búsqueda rápida sobre el código, dicha propiedad veo que se configura a true en la edición inline... 👎

En consecuencia creo que podemos omitir el bloque de código que realiza esa comparación:

// Avoid execution if the save method is triggered within a workflow
if (!empty($this->in_workflow)) {
    return parent::save($check_notify);
}

El resto del código propuesto sí consigue evitar que se dupliquen las preguntas, aunque dejo una propuesta que creo que es más clara para su comprensión y mantenimiento:

    /**
     * @param bool $check_notify
     * @return string
     */
    public function save($check_notify = false)
    {
        // STIC-Custom 20210729 - There is a bug when duplicating because the question ids 
        // of the first survey are not removed when creating the duplicate one. More info: 
        // STIC#367
        if (($_POST["duplicateSave"] && $_POST["duplicateSave"]="true"))
        {
            unset($_REQUEST['survey_questions_ids']);
        }
        // STIC End
    

        $res = parent::save($check_notify);
        if (empty($_REQUEST['survey_questions_supplied'])) {
            return $res;
        }

        // STIC-Custom 20241121 ART - Duplication of questions by having a workflow
        // https://github.com/SinergiaTIC/SinergiaCRM/pull/483
        // Prevent redundant execution of this method if it has already been executed
        if (!isset($this->already_saved)){
            foreach ($_REQUEST['survey_questions_names'] as $key => $val) {
                if (!empty($_REQUEST['survey_questions_ids'][$key])) {
                    $question = BeanFactory::getBean('SurveyQuestions', $_REQUEST['survey_questions_ids'][$key]);
                    // STIC-Custom 20211110 AAM - With deleted questions, call mark_deleted and skip the loop
                    // STIC#457
                    if ($_REQUEST['survey_questions_deleted'][$key]) {
                        $question->mark_deleted($question->id);
                        continue;
                    }
                    // END STIC
                } else {
                    $question = BeanFactory::newBean('SurveyQuestions');
                }
                $question->name = $val;
                $question->type = $_REQUEST['survey_questions_types'][$key];
                $question->sort_order = $_REQUEST['survey_questions_sortorder'][$key];
                $question->survey_id = $this->id;
                $question->deleted = $_REQUEST['survey_questions_deleted'][$key];
                $question->save();
                // STIC-Custom 20211110 AAM - Removing the "s" from the word question(s)
                // STIC#457
                // if (!empty($_REQUEST['survey_questions_options'][$key])) {
                //     $this->saveOptions(
                //         $_REQUEST['survey_questions_options'][$key],
                //         $_REQUEST['survey_questions_options_id'][$key],
                //         $_REQUEST['survey_questions_options_deleted'][$key],
                if (!empty($_REQUEST['survey_question_options'][$key])) {
                    $this->saveOptions(
                        $_REQUEST['survey_question_options'][$key],
                        $_REQUEST['survey_question_options_id'][$key],
                        $_REQUEST['survey_question_options_deleted'][$key],
                        $question->id
                    );
                }
            }
        } 

        // Set a control variable to indicate that the save method has already been executed
        $this->already_saved = true;
        // END STIC-Custom

        return $res;
    }

[ainaraRT]

Buenas, la propiedad $this->in_workflow no parece que funcione como su nombre indica.

Debuggeando en la línea en cuestión he visto que en las diferentes paradas que se realizan en las pruebas, tanto en la del save() de la encuesta duplicada como en las dos acciones del FdT creado, dicha propiedad siempre está a false. Haciendo una búsqueda rápida sobre el código, dicha propiedad veo que se configura a true en la edición inline... 👎

En consecuencia creo que podemos omitir el bloque de código que realiza esa comparación:

// Avoid execution if the save method is triggered within a workflow
if (!empty($this->in_workflow)) {
    return parent::save($check_notify);
}

El resto del código propuesto sí consigue evitar que se dupliquen las preguntas, aunque dejo una propuesta que creo que es más clara para su comprensión y mantenimiento:

    /**
     * @param bool $check_notify
     * @return string
     */
    public function save($check_notify = false)
    {
        // STIC-Custom 20210729 - There is a bug when duplicating because the question ids 
        // of the first survey are not removed when creating the duplicate one. More info: 
        // STIC#367
        if (($_POST["duplicateSave"] && $_POST["duplicateSave"]="true"))
        {
            unset($_REQUEST['survey_questions_ids']);
        }
        // STIC End
    

        $res = parent::save($check_notify);
        if (empty($_REQUEST['survey_questions_supplied'])) {
            return $res;
        }

        // STIC-Custom 20241121 ART - Duplication of questions by having a workflow
        // https://github.com/SinergiaTIC/SinergiaCRM/pull/483
        // Prevent redundant execution of this method if it has already been executed
        if (!isset($this->already_saved)){
            foreach ($_REQUEST['survey_questions_names'] as $key => $val) {
                if (!empty($_REQUEST['survey_questions_ids'][$key])) {
                    $question = BeanFactory::getBean('SurveyQuestions', $_REQUEST['survey_questions_ids'][$key]);
                    // STIC-Custom 20211110 AAM - With deleted questions, call mark_deleted and skip the loop
                    // STIC#457
                    if ($_REQUEST['survey_questions_deleted'][$key]) {
                        $question->mark_deleted($question->id);
                        continue;
                    }
                    // END STIC
                } else {
                    $question = BeanFactory::newBean('SurveyQuestions');
                }
                $question->name = $val;
                $question->type = $_REQUEST['survey_questions_types'][$key];
                $question->sort_order = $_REQUEST['survey_questions_sortorder'][$key];
                $question->survey_id = $this->id;
                $question->deleted = $_REQUEST['survey_questions_deleted'][$key];
                $question->save();
                // STIC-Custom 20211110 AAM - Removing the "s" from the word question(s)
                // STIC#457
                // if (!empty($_REQUEST['survey_questions_options'][$key])) {
                //     $this->saveOptions(
                //         $_REQUEST['survey_questions_options'][$key],
                //         $_REQUEST['survey_questions_options_id'][$key],
                //         $_REQUEST['survey_questions_options_deleted'][$key],
                if (!empty($_REQUEST['survey_question_options'][$key])) {
                    $this->saveOptions(
                        $_REQUEST['survey_question_options'][$key],
                        $_REQUEST['survey_question_options_id'][$key],
                        $_REQUEST['survey_question_options_deleted'][$key],
                        $question->id
                    );
                }
            }
        } 

        // Set a control variable to indicate that the save method has already been executed
        $this->already_saved = true;
        // END STIC-Custom

        return $res;
    }

Cierto, no hay hecho el debug... He aplicado tu propuesta y es correcta!

[ManuSinergiaCRM]

(A)probado