fix(forms): Ensure consistent dropdown labels for Item questions in f…#23507
fix(forms): Ensure consistent dropdown labels for Item questions in f…#23507ccailly wants to merge 2 commits intoglpi-project:11.0/bugfixesfrom
Conversation
…test cleanup process
AdrienClairembault
left a comment
AdrienClairembault
left a comment
There was a problem hiding this comment.
The AJAX endpoint dropdownAllItems.php:60 (used when dynamically switching itemtype for unsaved questions) hardcodes:
- ITIL types → ['id']
- Everything else → ['otherserial', 'serial']
The new getDisplayWith() method computes a different set:
- ITIL types → ['id']
- Asset types → ['otherserial', 'serial', 'users_id']
- Non-asset, non-ITIL → []
This means saved questions with Computer type will show users_id in the dropdown while unsaved questions (loaded via AJAX) will not. The consistency goal isn't fully achieved for asset types.
Centralizing the logic so both paths call the same computation would be great.
| }); | ||
|
|
||
| // Switch to the new entity and refresh session to ensure the new entity is taken into account in the session | ||
| entity.switchToWithoutRecursion(entity_id); |
There was a problem hiding this comment.
| entity.switchToWithoutRecursion(entity_id); | |
| await entity.switchToWithoutRecursion(entity_id); |
|
|
||
| test.afterEach(async ({ entity, api }) => { | ||
| // Reset entity to default one to avoid issues with other tests in the same worker | ||
| entity.resetToDefaultWorkerEntity(); |
There was a problem hiding this comment.
| entity.resetToDefaultWorkerEntity(); | |
| await entity.resetToDefaultWorkerEntity(); |
| const persisted_question = form.getLastQuestion(); | ||
| await persisted_question.click({ position: { x: 0, y: 0 } }); | ||
|
|
||
| form.getDropdownByLabel('Select an item', persisted_question).click(); |
There was a problem hiding this comment.
| form.getDropdownByLabel('Select an item', persisted_question).click(); | |
| await form.getDropdownByLabel('Select an item', persisted_question).click(); |
| await form.setSubQuestionType(new_question, 'GLPI Objects'); | ||
| await form.setItemTypeForItemQuestion(new_question, 'Computers'); | ||
|
|
||
| form.getDropdownByLabel('Select an item', new_question).click(); |
There was a problem hiding this comment.
| form.getDropdownByLabel('Select an item', new_question).click(); | |
| await form.getDropdownByLabel('Select an item', new_question).click(); |
| * @param string|null $itemtype The itemtype to compute displaywith for. | ||
| * @return array<string> | ||
| */ | ||
| public function getDisplayWith(?string $itemtype): array |
There was a problem hiding this comment.
Method is public but is only used internally, is it normal?
| if ($item->isField('serial')) { | ||
| $displaywith[] = 'serial'; | ||
| } |
There was a problem hiding this comment.
I see you changed this for consistency with the ajax dropdown endpoint but maybe this order should be the one being kept? I think the main serial number is more important and should be displayed first.
Or we keep it like that to prevent changes on a bugfix and we use serial -> serialnumber everywhere on main.
3 participants
Checklist before requesting a review
Please delete options that are not relevant.
Description
Ensure that the labels in the dropdown menus for “Item” questions remain the same regardless of whether they have been saved or not.
Currently, we have more information available when selecting an item before saving and reloading the page.
The display is now consistent between the form editor and the end-user interface.
Screenshots (if appropriate):
Before (for saved question)
After (for saved question)