Skip to content

fix(fields): don't enforce mandatory fields on automated item creation - #1274

Merged
Rom1-B merged 3 commits into
pluginsGLPI:mainfrom
MyvTsv:ticket46291
Sep 29, 2026
Merged

Rom1-B merged 3 commits into
pluginsGLPI:mainfrom
MyvTsv:ticket46291

Conversation

@MyvTsv

@MyvTsv MyvTsv commented Sep 25, 2026

Copy link
Copy Markdown
Contributor
  • I have performed a self-review of my code.
  • I have added tests (when available) that prove my fix is effective or that my feature works.
  • I have updated the CHANGELOG with a short functional description of the fix or new feature.
  • This change requires a documentation update.

Description

  • It fixes !46291 !45954

Root cause

Mandatory custom fields were enforced unconditionally, even for items created without any human filling a form (tickets generated by the Mail Collector) Since there is no way to fill a mandatory custom field from an incoming email, item creation was silently rejected with "Some mandatory fields are empty".

Fix

Mandatory field enforcement in PluginFieldsContainer::validateValues() is now
skipped when any of the following applies:

  • the code runs from the command line (isCommandLine())
  • the code runs inside a cron task, internal or external (Session::isCron())
  • the item is being created through GLPI core's "automated import" marker
    (_auto_import), set by MailCollector::buildTicket() and by recurring
    tickets, this covers the "Collect now" button too, which is a plain web
    request (neither CLI nor cron), so it wasn't covered by the first two checks
    alone.

Steps to reproduce

  1. Create a "dom" container for Ticket with a mandatory custom field and no
    default value.
  2. Configure a mail collector.
  3. Trigger an import (cron task, or the "Collect now" button on the collector).
  4. Before the fix: the ticket is rejected ("Some mandatory fields are
    empty"), even though there is no way for the sender to fill that field.
    After the fix: the ticket is created normally.

@MyvTsv MyvTsv self-assigned this Sep 25, 2026

@Rom1-B Rom1-B left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only relaxes mandatory checks for CLI/cron/_auto_import (Ticket-only). !46291's actual scenarios (Inventory agent, REST API bulk import) go through plain HTTP requests and never set any of these. Can you also check is_dynamic (set by src/Glpi/Inventory/**) and isAPI() in inc/container.class.php:1743?

Comment thread tests/Units/ContainerItemUpdateTest.php Outdated
ERROR,
);

unset($GLOBALS['GLPI_IS_COMMAND_LINE']);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the unset into the existing tearDown() so it always runs, even if the assertion above fails.

This isn't the only line in this case; please process them all.

@MyvTsv
MyvTsv marked this pull request as ready for review September 28, 2026 09:14
@MyvTsv
MyvTsv requested a review from Rom1-B September 28, 2026 09:14

@Rom1-B Rom1-B left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new isAPI() / is_dynamic bypass paths in isMandatoryCheckBypassed() have no test coverage. Can you add a case for an inventory-created item (is_dynamic) and one for an API-created item (isAPI()) skipping mandatory validation?

@MyvTsv
MyvTsv requested a review from Rom1-B September 29, 2026 12:13
@Rom1-B
Rom1-B merged commit 12ca573 into pluginsGLPI:main Sep 29, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants