Conversation
priority looks like a plain enum in a ticket's field list, but iTop overwrites it on every save from impact and urgency (ComputeValues() calls ComputePriority()). create-user-request defaults to impact 3 (one person) and urgency 4 (low), which maps to the lowest priority - so a ticket opened through the MCP without explicit values lands as "low", outage or not, with nothing saying why. DatamodelService now reads the matrix out of the ComputePriority() source embedded in the datamodel XML, so it stays right on an instance that customized it, and get-class-schema renders it alongside the fields. Parsing is best-effort against the shape iTop ships; anything else yields no matrix and the section is omitted rather than guessed at. Verified against the matrix in tests/phpunit/data/datamodel-production.xml, which matches exactly. It rides in the existing class-schema cache entry, so cache hits pay nothing. The create-user-request docblock now carries what holds everywhere - priority is derived, the defaults give the lowest one, what the impact and urgency codes mean - and points at get-class-schema only for the matrix itself, the part that actually varies per instance.
There was a problem hiding this comment.
Pull request overview
Adds instance-aware ticket priority semantics to class schemas.
Changes:
- Parses priority matrices from datamodel XML.
- Marks and renders computed priority information.
- Documents impact and urgency for user-request creation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/Service/DatamodelService.php |
Extracts and caches priority matrices. |
src/Tools/schema/iTopSchemaTools.php |
Renders computed priority details. |
src/Tools/core/create/iTopCreateTools.php |
Documents derived priority inputs. |
Suppressed comments (1)
src/Service/DatamodelService.php:150
- This pattern scans the entire method for any numeric nested array; it is not scoped to the
$aPrioritiesassignment. A customized method containing another lookup such as$other = [9 => [8 => 7]]will expose that lookup as a priority row (and can merge it with the real matrix). Extract the named assignment first and reject the result unless its complete initializer matches the supported shape.
// Each 'impact => array(urgency => priority, ...)' entry of the outer array.
if (!preg_match_all('/(\d+)\s*=>\s*(?:array\s*\(|\[)([^)\]]*)(?:\)|\])/s', $code, $matches, PREG_SET_ORDER)) {
return null;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+122
to
+123
| // A class that does not redefine ComputePriority() inherits its parent's. | ||
| $priorityMatrix = $this->getPriorityMatrixFromXml($className) ?? $inheritedPriorityMatrix; |
Comment on lines
+79
to
+81
| $computed = ($code === 'priority' && ($info['priority_matrix'] ?? null) !== null) | ||
| ? ' [COMPUTED from impact and urgency, setting it directly has no effect - see below]' | ||
| : ''; |
Member
|
Hello @agent-qv, thank you for your PR. We're not willing to have this in our repo, but we're working on extensibility so people willing to add such functionalities will be able to do so |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #9
Problem
In iTop, ticket priority is recomputed from impact and urgency on each save
(
ComputeValues()callsComputePriority()). It is displayed as a normal enum field byget-class-schema, so an MCP client can reasonably but incorrectly attempt to set itdirectly.
create-user-request's defaults (impact 3, urgency 4) silently map to thelowest priority, with nothing telling the caller why.
Solution
DatamodelServicenow reads the impact x urgency -> priority matrix out of theComputePriority()source embedded in the datamodel XML, so it stays correct on aninstance that customized it. Parsing is best-effort against the shape iTop ships
(
$aPriorities = array(impact => array(urgency => priority))); anything else yields nomatrix, and the section is simply omitted rather than guessed at. It rides in the
existing class-schema cache entry, so cache hits pay nothing extra.
get-class-schemanow flagspriorityas computed when a matrix is available, andrenders the full matrix alongside the field list.
create-user-request's docblock nowsays priority is derived, that the defaults give the lowest one, and points at
get-class-schemafor the actual matrix (the part that varies per instance).Testing
Verified the parsing against
tests/phpunit/data/datamodel-production.xml(a realproduction datamodel export) - the extracted matrix for
UserRequestmatches the rawComputePriority()source in that file exactly. Existing test suite passes/failsidentically before and after this change (the pre-existing failures are unrelated
fixture/environment issues, not introduced by this PR).
Note
This PR follows up on the design discussion in #9. Happy to adjust the source/API if you'd
prefer a different approach once the planned schema webservice is further along.