Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Entity Assistance Tab UI + some AJAX #17023

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Entity Assistance Tab UI + some AJAX #17023

wants to merge 6 commits into from

Conversation

cconard96
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
  • Migrated Assistance tab of Entity form to Twig.
  • Removed specific AJAX files for ticket and change satisfaction surveys in favor of using ajax/commonitilsatisfaction.php directly.
  • Moved HTML from ajax/commonitilsatisfaction.php to Twig.
  • Minor UI sanitization/Twig/Cleanup work in some other AJAX files.
  • Addition of concat_ws to QueryFunction class

@cconard96 cconard96 added the ui label Apr 27, 2024
@cconard96 cconard96 mentioned this pull request Apr 29, 2024
Merged
cedric-anne
cedric-anne requested changes May 16, 2024
View reviewed changes
Copy link
Member

@cedric-anne cedric-anne left a comment

Choose a reason for hiding this comment

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

Please rebase to solve conflicts.

src/Entity.php Outdated Show resolved Hide resolved
src/Entity.php Outdated Show resolved Hide resolved
src/Entity.php Outdated Show resolved Hide resolved
src/Entity.php Outdated Show resolved Hide resolved
templates/pages/admin/entity/survey_config.html.twig Outdated Show resolved Hide resolved
cedric-anne
cedric-anne requested changes May 21, 2024
View reviewed changes
src/Entity.php
@@ -2526,7 +2526,38 @@ public static function getSpecificValueToDisplay($field, $values, array $options
if ($values[$field] === self::CONFIG_PARENT) {
return __('Inheritance of the parent entity');
}
return CommonITILSatisfaction::getTypeInquestName($values[$field]);
$inherit =
Copy link
Member

Choose a reason for hiding this comment

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

Did you forgot to assign a value here, or is it intended?

templates/pages/admin/entity/assistance.html.twig
Copy link
Member

Choose a reason for hiding this comment

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

I have the following error:

glpiphplog.CRITICAL:   *** Twig Error (Twig\Error\RuntimeError): "The merge filter only works with arrays or "Traversable", got "NULL" for argument 2." in template "/var/www/glpi/templates/pages/admin/entity/assistance.html.twig" at line 100
  Backtrace :
  ...tes/e0/e026b9d5fe9203796cd1513f4ebf9d2c.php:127 Twig\Extension\CoreExtension::arrayMerge()
  vendor/twig/twig/src/Template.php:430              __TwigTemplate_b8f857557d4b7425bc0d35fae94fb920->block_form_fields()
  ...tes/49/497db28d66d4b3437cf9b8a995a3a252.php:103 Twig\Template->yieldBlock()
  vendor/twig/twig/src/Template.php:360              __TwigTemplate_e7892048913a21898706f90f34fadae5->doDisplay()
  ...ates/e0/e026b9d5fe9203796cd1513f4ebf9d2c.php:46 Twig\Template->yield()
  vendor/twig/twig/src/Template.php:360              __TwigTemplate_b8f857557d4b7425bc0d35fae94fb920->doDisplay()
  vendor/twig/twig/src/Template.php:327              Twig\Template->yield()
  vendor/twig/twig/src/TemplateWrapper.php:45        Twig\Template->display()
  src/Application/View/TemplateRenderer.php:185      Twig\TemplateWrapper->display()
  src/Entity.php:2025                                Glpi\Application\View\TemplateRenderer->display()
  src/Entity.php:544                                 Entity::showHelpdeskOptions()
  src/CommonGLPI.php:682                             Entity::displayTabContentForItem()
  ajax/common.tabs.php:114                           CommonGLPI::displayStandardTab()
  public/index.php:83                                require()

src/Entity.php
Comment on lines +1714 to +1722
'admin_email' => $entity->getInheritedValueBadge(field: 'admin_email', inherit_parent_value: null),
'admin_email_name' => $entity->getInheritedValueBadge(field: 'admin_email_name', inherit_parent_value: null),
'from_email' => $entity->getInheritedValueBadge(field: 'from_email', inherit_parent_value: null),
'from_email_name' => $entity->getInheritedValueBadge(field: 'from_email_name', inherit_parent_value: null),
'noreply_email' => $entity->getInheritedValueBadge(field: 'noreply_email', inherit_parent_value: null),
'noreply_email_name' => $entity->getInheritedValueBadge(field: 'noreply_email_name', inherit_parent_value: null),
'replyto_email' => $entity->getInheritedValueBadge(field: 'replyto_email', inherit_parent_value: null),
'replyto_email_name' => $entity->getInheritedValueBadge(field: 'replyto_email_name', inherit_parent_value: null),
'notification_subject_tag' => $entity->getInheritedValueBadge(field: 'notification_subject_tag', inherit_parent_value: null),
Copy link
Member

Choose a reason for hiding this comment

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

Inherited values should not be shown on root entity.
image

Also, inherited values are invalid on child entities when the root entity value is empty.
image

src/Entity.php
'autofill_use_date' => $entity->getInheritedValueBadge('autofill_use_date'),
'autofill_warranty_date' => $entity->getInheritedValueBadge('autofill_warranty_date'),
'autofill_decommission_date' => $entity->getInheritedValueBadge('autofill_decommission_date'),
'agent_base_url' => $entity->getInheritedValueBadge(field: 'agent_base_url', inherit_parent_value: null),
Copy link
Member

Choose a reason for hiding this comment

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

Inherited value should not be shown on root entity.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there something different in my DB? This field is just empty/null by default and AFAIK, an empty value is supposed to be treated as inherited by parent.

Copy link
Member

Choose a reason for hiding this comment

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

My screenshot was taken in the root entity context. The root entity cannot inherit anything.

src/Entity.php
Comment on lines +1664 to +1669
'autofill_buy_date' => $entity->getInheritedValueBadge('autofill_buy_date'),
'autofill_order_date' => $entity->getInheritedValueBadge('autofill_order_date'),
'autofill_delivery_date' => $entity->getInheritedValueBadge('autofill_delivery_date'),
'autofill_use_date' => $entity->getInheritedValueBadge('autofill_use_date'),
'autofill_warranty_date' => $entity->getInheritedValueBadge('autofill_warranty_date'),
'autofill_decommission_date' => $entity->getInheritedValueBadge('autofill_decommission_date'),
Copy link
Member

Choose a reason for hiding this comment

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

Inherited values are not shown.
image

src/Entity.php
Comment on lines +1744 to +1746
'use_domains_alert' => $entity->getInheritedValueBadge('use_domains_alert'),
'send_domains_alert_close_expiries_delay' => $entity->getInheritedValueBadge('send_domains_alert_close_expiries_delay'),
'send_domains_alert_expired_delay' => $entity->getInheritedValueBadge('send_domains_alert_expired_delay'),
Copy link
Member

Choose a reason for hiding this comment

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

Inherited values should not be shown on root entity.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cedric-anne cedric-anne cedric-anne requested changes

@orthagh orthagh orthagh approved these changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@cconard96 @orthagh @cedric-anne