Skip to content

Commit

Permalink
Fix multiple issues while submitting helpdesk forms (#18600)
Browse files Browse the repository at this point in the history
* Fix fatal error preventing defaults forms to be submitted
* Fix missing description
* Prevent fatal error when pasted image is submitted
* Make sure richtext images are saved properly (no base64)
  • Loading branch information
AdrienClairembault authored Jan 6, 2025
1 parent 20cc359 commit 2c5976a
Show file tree
Hide file tree
Showing 11 changed files with 220 additions and 39 deletions.
6 changes: 0 additions & 6 deletions .phpstan-baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -2719,12 +2719,6 @@
'count' => 1,
'path' => __DIR__ . '/src/Glpi/Form/Question.php',
];
$ignoreErrors[] = [
'message' => '#^Call to function is_array\\(\\) with array\\<mixed\\> will always evaluate to true\\.$#',
'identifier' => 'function.alreadyNarrowedType',
'count' => 1,
'path' => __DIR__ . '/src/Glpi/Form/QuestionType/AbstractQuestionTypeActors.php',
];
$ignoreErrors[] = [
'message' => '#^PHPDoc tag @return with type int is incompatible with native type array\\.$#',
'identifier' => 'return.phpDocType',
Expand Down
7 changes: 6 additions & 1 deletion js/modules/Forms/RendererController.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
* ---------------------------------------------------------------------
*/

/* global glpi_toast_info */
/* global glpi_toast_info, tinymce */

/**
* Client code to handle users actions on the form_renderer template
Expand Down Expand Up @@ -113,6 +113,11 @@ export class GlpiFormRendererController
async #submitForm() {
// Form will be sumitted using an AJAX request instead
try {
// Update tinymce values
tinymce.get().forEach(editor => {
editor.save();
});

// Submit form using AJAX
const response = await $.post({
url: $(this.#target).prop("action"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@

namespace test\units\Glpi\Helpdesk;

use Auth;
use CommonITILActor;
use Computer;
use DbTestCase;
Expand Down
12 changes: 9 additions & 3 deletions src/Glpi/Controller/Form/SubmitAnswerController.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,16 +114,22 @@ private function saveSubmittedAnswers(
Request $request
): AnswersSet {
$post = $request->request->all();
$answers = (new EndUserInputNameProvider())->getAnswers($post);
$provider = new EndUserInputNameProvider();

$answers = $provider->getAnswers($post);
$files = $provider->getFiles($post, $answers);
if (empty($answers)) {
throw new BadRequestHttpException();
}

$handler = AnswersHandler::getInstance();
return $handler->saveAnswers(
$answers = $handler->saveAnswers(
$form,
$answers,
Session::getLoginUserID()
Session::getLoginUserID(),
$files,
);

return $answers;
}
}
9 changes: 6 additions & 3 deletions src/Glpi/Form/AnswersHandler/AnswersHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ public static function getInstance(): AnswersHandler
public function saveAnswers(
Form $form,
array $answers,
int $users_id
int $users_id,
array $files = []
): AnswersSet {
/** @var \DBmysql $DB */
global $DB;
Expand All @@ -103,7 +104,7 @@ public function saveAnswers(
$DB->beginTransaction();

try {
$answers_set = $this->doSaveAnswers($form, $answers, $users_id);
$answers_set = $this->doSaveAnswers($form, $answers, $users_id, $files);
$DB->commit();
return $answers_set;
} catch (\Throwable $e) {
Expand Down Expand Up @@ -137,14 +138,16 @@ public function saveAnswers(
protected function doSaveAnswers(
Form $form,
array $answers,
int $users_id
int $users_id,
array $files = []
): AnswersSet {
// Save answers
$answers_set = $this->createAnswserSet(
$form,
$answers,
$users_id
);
$answers_set->setSubmittedFiles($files);

// Create destinations objects
$this->createDestinations(
Expand Down
13 changes: 12 additions & 1 deletion src/Glpi/Form/AnswersSet.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
use CommonDBTM;
use CommonGLPI;
use Glpi\Application\View\TemplateRenderer;
use Glpi\Form\AnswersHandler\AnswersHandler;
use Glpi\Form\Destination\AnswersSet_FormDestinationItem;
use Glpi\Form\Destination\FormDestinationTypeManager;
use Log;
Expand All @@ -56,6 +55,8 @@ final class AnswersSet extends CommonDBChild
public static $itemtype = Form::class;
public static $items_id = 'forms_forms_id';

public array $files = [];

#[Override]
public static function getTypeName($nb = 0)
{
Expand Down Expand Up @@ -317,6 +318,16 @@ public function getLinksToCreatedItems(): array
return $links;
}

public function getSubmittedFiles(): array
{
return $this->files;
}

public function setSubmittedFiles(array $files): void
{
$this->files = $files;
}

/**
* Count answers for a given form
*
Expand Down
17 changes: 17 additions & 0 deletions src/Glpi/Form/Destination/AbstractCommonITILFormDestination.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ final public function createDestinationItems(
);
}

// Add linked items
$input = $this->setFilesInput($input, $answers_set);

// Create commonitil object
$itil_object = new $itemtype();
if (!($itil_object instanceof CommonITILObject)) {
Expand Down Expand Up @@ -257,4 +260,18 @@ private function applyPredefinedTemplateFields(array $input): array

return $input;
}

private function setFilesInput(array $input, AnswersSet $answers_set): array
{
$files = $answers_set->getSubmittedFiles();
if (empty($files) || empty($files['filename'])) {
return $input;
}

$input['_filename'] = $files['filename'];
$input['_prefix_filename'] = $files['prefix'];
$input['_tag_filename'] = $files['tag'];

return $input;
}
}
35 changes: 35 additions & 0 deletions src/Glpi/Form/EndUserInputNameProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,32 @@ public function getEndUserInputName(Question $question): string
return sprintf(self::END_USER_INPUT_NAME, $question->getID());
}


public function getFiles(array $inputs, array $answers): array
{
$files = [
'filename' => [],
'prefix' => [],
'tag' => []
];

foreach (array_keys($answers) as $answer_id) {
if (
isset($inputs["_answers_$answer_id"])
&& isset($inputs["_prefix_answers_$answer_id"])
&& isset($inputs["_tag_answers_$answer_id"])
) {
foreach (array_keys($inputs["_answers_$answer_id"]) as $i) {
$files['filename'][] = $inputs["_answers_$answer_id"][$i];
$files['prefix'][] = $inputs["_prefix_answers_$answer_id"][$i];
$files['tag'][] = $inputs["_tag_answers_$answer_id"][$i];
}
}
}

return $files;
}

/**
* Get the answers submitted by the end user
* The answers are indexed by question ID
Expand All @@ -78,6 +104,15 @@ public function getAnswers(array $inputs): array
*/
private function filterAnswers(array $answers): array
{
// Remove files
foreach (array_keys($answers) as $key) {
foreach (["_$key", "_prefix_$key", "_tag_$key"] as $extra_file_info) {
if (isset($answers[$extra_file_info])) {
unset($answers["_$key"]);
}
}
}

return array_filter(
$answers,
function ($key) {
Expand Down
46 changes: 24 additions & 22 deletions src/Glpi/Form/QuestionType/AbstractQuestionTypeActors.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,37 +114,39 @@ public function validateExtraDataInput(array $input): bool
#[Override]
public function prepareEndUserAnswer(Question $question, mixed $answer): mixed
{
if (empty($answer)) {
return [];
}

if (!is_array($answer)) {
$answer = [$answer];
}

$actors = [];
if (is_array($answer)) {
foreach ($answer as $actor) {
// The "0" value can occur when the empty label is selected.
if (empty($actor)) {
continue;
}

$actor_parts = explode('-', $actor);
$itemtype = getItemtypeForForeignKeyField($actor_parts[0]);
$item_id = $actor_parts[1];
foreach ($answer as $actor) {
// The "0" value can occur when the empty label is selected.
if (empty($actor)) {
continue;
}

// Check if the itemtype is allowed
if (!in_array($itemtype, $this->getAllowedActorTypes())) {
throw new Exception("Invalid actor type: $itemtype");
}
$actor_parts = explode('-', $actor);
$itemtype = getItemtypeForForeignKeyField($actor_parts[0]);
$item_id = $actor_parts[1];

// Check if the item exists
if ($itemtype::getById($item_id) === false) {
throw new Exception("Invalid actor ID: $item_id");
}
// Check if the itemtype is allowed
if (!in_array($itemtype, $this->getAllowedActorTypes())) {
throw new Exception("Invalid actor type: $itemtype");
}

$actors[] = [
'itemtype' => $itemtype,
'items_id' => $item_id
];
// Check if the item exists
if ($itemtype::getById($item_id) === false) {
throw new Exception("Invalid actor ID: $item_id");
}

$actors[] = [
'itemtype' => $itemtype,
'items_id' => $item_id
];
}

if (!$this->isMultipleActors($question) && count($actors) > 1) {
Expand Down
95 changes: 95 additions & 0 deletions tests/cypress/e2e/form/service_catalog/default_forms.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/**
* ---------------------------------------------------------------------
*
* GLPI - Gestionnaire Libre de Parc Informatique
*
* http://glpi-project.org
*
* @copyright 2015-2025 Teclib' and contributors.
* @licence https://www.gnu.org/licenses/gpl-3.0.html
*
* ---------------------------------------------------------------------
*
* LICENSE
*
* This file is part of GLPI.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
* ---------------------------------------------------------------------
*/

describe('Default forms', () => {
it('can fill and submit the incident form', () => {
// Go to form
cy.login();
cy.visit("/Form/Render/1");

// Fill form
cy.getDropdownByLabelText('Urgency').selectDropdownValue('High');
cy.findByRole('textbox', {'name': "Title"}).type("My title");
cy.findByLabelText("Description").awaitTinyMCE().type("My description");

// Submit form
cy.findByRole('button', {'name': "Send form"}).click();
cy.findByRole('alert')
.should('contain.text', 'Item successfully created')
;

// Validate ticket values using API
cy.findByRole('alert')
.findByRole('link')
.invoke("attr", "href")
.then((href) => {
const id = /\?id=(.*)/.exec(href)[1];
cy.getWithAPI('Ticket', id).then((fields) => {
expect(fields.urgency).to.equal(4);
expect(fields.name).to.equal('My title');
expect(fields.content).to.equal('<p>My description</p>');
});
})
;
});

it('can fill and submit the service form', () => {
// Go to form
cy.login();
cy.visit("/Form/Render/2");

// Fill form
cy.getDropdownByLabelText('Urgency').selectDropdownValue('High');
cy.findByRole('textbox', {'name': "Title"}).type("My title");
cy.findByLabelText("Description").awaitTinyMCE().type("My description");

// Submit form
cy.findByRole('button', {'name': "Send form"}).click();
cy.findByRole('alert')
.should('contain.text', 'Item successfully created')
;

// Validate ticket values using API
cy.findByRole('alert')
.findByRole('link')
.invoke("attr", "href")
.then((href) => {
const id = /\?id=(.*)/.exec(href)[1];
cy.getWithAPI('Ticket', id).then((fields) => {
expect(fields.urgency).to.equal(4);
expect(fields.name).to.equal('My title');
expect(fields.content).to.equal('<p>My description</p>');
});
})
;
});
});
Loading

0 comments on commit 2c5976a

Please sign in to comment.