Skip to content

Change config UI#8

Open
Lainow wants to merge 5 commits into
mainfrom
change-config-ui
Open

Change config UI#8
Lainow wants to merge 5 commits into
mainfrom
change-config-ui

Conversation

@Lainow
Copy link
Copy Markdown
Contributor

@Lainow Lainow commented May 27, 2026

Checklist before requesting a review

Please delete options that are not relevant.

  • 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 # (issue number, if applicable)
  • Here is a brief description of what this PR does

Change to the configuration interface (switch from checkboxes to “Yes”/‘No’ options with the “Inheritance” option)

Screenshots (if appropriate):

image image

@Lainow Lainow requested a review from stonebuzz May 27, 2026 14:46
@Lainow Lainow self-assigned this May 27, 2026
Copy link
Copy Markdown
Contributor

@Rom1-B Rom1-B left a comment

Choose a reason for hiding this comment

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

Please fix CI

@Lainow Lainow requested a review from Rom1-B June 4, 2026 07:51
Comment thread src/Config.php
$migration->displayMessage("Installing $table");
$query = "CREATE TABLE IF NOT EXISTS `$table` (
`id` int unsigned NOT NULL AUTO_INCREMENT,
`is_active` tinyint NOT NULL DEFAULT '1',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should drop this column if table already exist

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 plugin hasn't been released yet

Comment thread src/Config.php
`mandatory_task_group` tinyint NOT NULL DEFAULT '0',
PRIMARY KEY (`id`),
KEY `entities_id` (`entities_id`),
KEY `is_active` (`is_active`)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should drop this column if table already exist

Comment thread src/Config.php
Comment on lines +185 to +196
foreach (self::getItilConfigFields() as $field) {
$inheritance_labels[$field] = self::getInheritedValueBadge($parentConfig->fields[$field] ?? 0);
}
foreach ([
'take_requester_group_ticket',
'take_requester_group_change',
'take_requester_group_problem',
'take_technician_group_ticket',
'take_technician_group_change',
'take_technician_group_problem',
] as $field) {
$inheritance_labels[$field] = self::getInheritedValueBadgeForActorGroup($parentConfig->fields[$field] ?? 0);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

self::getItilConfigFields already contain

'take_requester_group_ticket',
'take_requester_group_change',
'take_requester_group_problem',
'take_technician_group_ticket',
'take_technician_group_change',
'take_technician_group_problem',

The first loop at line
185 therefore generates a Yes/No badge for them (wrong — they need group-name labels), and the second explicit loop at line 188 immediately
overwrites with the correct badge.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The test suite doesn't cover the grandparent → parent → child chain

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