Skip to content
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

Fix multiple issues while submitting helpdesk forms #18600

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading