Skip to content

Conversation

SebSept
Copy link
Contributor

@SebSept SebSept commented Sep 24, 2025

Checklist before requesting a review

Please delete options that are not relevant.

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
Capture d’écran du 2025-09-24 14-55-12

I'm not sure of the array value to use as key value, I used schema_name, maybe it's itemtype

@cconard96
Copy link
Contributor

This fix doesn't seem complete yet as there is at least one other itemtype that triggers an error in getMonacoSuggestions.

You may add the following test to at least ensure all itemtypes in the dropown get at least some suggestions without error:

    public function testGetMonacoSuggestions()
    {
        $itemtypes = Webhook::getItemtypesDropdownValues();
        $failed_itemtypes = [];
        foreach ($itemtypes as $types) {
            $this->assertIsArray($types);
            foreach ($types as $itemtype => $label) {
                try {
                    $suggestions = Webhook::getMonacoSuggestions($itemtype);
                    $this->assertNotEmpty($suggestions);
                } catch (\Throwable $e) {
                    $failed_itemtypes[$itemtype] = [
                        'error' => $e->getMessage(),
                        'trace' => $e->getTraceAsString()
                    ];
                }
            }
        }
        $this->assertEmpty($failed_itemtypes, print_r($failed_itemtypes, true));
    }

@SebSept
Copy link
Contributor Author

SebSept commented Sep 25, 2025

I fixed other undefined index errors & added a test.
It reveals that a Missing suggestions for ITILFollowup that probably needs to be fixed (or ignored in the test). (so ci is red)

@cconard96
Copy link
Contributor

I think I may take this over and rewrite the integration with the HLAPI to reduce the complexity and rely more on static data instead of guesses. The identification of parent itemtypes does not seem reliable as-is.

@SebSept
Copy link
Contributor Author

SebSept commented Sep 29, 2025

Relying on object not arrays is a good option.
Will you fix the failing test ? @cconard96 (it will take a long for me, because of the time to find why there is a missing suggestion for an ITILFollowUp.)

@cedric-anne
Copy link
Member

replaced by #21181

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.

3 participants