Skip to content

N°8886 - Deprecation warning in DashetGroupBy in 3.3-dev#972

Open
bdalsass wants to merge 1 commit into
developfrom
feature/8886-dasboard-attribute-deprecated
Open

N°8886 - Deprecation warning in DashetGroupBy in 3.3-dev#972
bdalsass wants to merge 1 commit into
developfrom
feature/8886-dasboard-attribute-deprecated

Conversation

@bdalsass

Copy link
Copy Markdown
Contributor

Base information

Question Answer
Related to a SourceForge thread / Another PR / A GitHub Issue / Combodo ticket? https://support.combodo.com/pages/UI.php?operation=details&class=Bug&id=8886
Type of change? Bug fix

Symptom (bug) / Objective (enhancement)

PHP deprecated message on attribute dashboard edition

Reproduction procedure (bug)

(with PHP 8.3)
Edit the dashboard of an organization objet, add a GroupByTable dashlet, you will get a deprecated message.

Cause (bug)

'preg_replace(): Passing null to parameter #3 ($subject)

Checklist before requesting a review

  • [X ] I have performed a self-review of my code
  • [X ] I have tested all changes I made on an iTop instance
  • I have added a unit test, otherwise I have explained why I couldn't
  • [X ] Is the PR clear and detailed enough so anyone can understand without digging in the code?

Copilot AI review requested due to automatic review settings July 15, 2026 08:36
@CombodoApplicationsAccount CombodoApplicationsAccount added the internal Work made by Combodo label Jul 15, 2026
@bdalsass bdalsass added this to the 3.3.0 milestone Jul 15, 2026
@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a PHP 8.3 deprecation warning in Dashboard::Render() where null was passed as the $subject to preg_replace() inside utils::Sanitize() when $aExtraParams['dashboard_div_id'] was absent.

  • The null-coalescing fallback ?? null is replaced with ?? $this->GetId(), so when the key is missing, Sanitize receives the same value for both $value and $defaultValue and short-circuits immediately, returning the ID without hitting preg_replace() at all. The end-to-end result is identical to the pre-fix behavior.
  • The change is minimal and contained to a single line with no side-effects on callers.

Confidence Score: 5/5

Safe to merge — the single-line change corrects the null fallback to a valid string, eliminating the deprecation warning with no behavioral change in any code path.

The fix substitutes $this->GetId() for null as the null-coalescing default, which causes utils::Sanitize() to take its identity short-circuit path rather than calling Sanitize_Internal(null, ...). The end result returned to the caller is the same GetId() value as before; only the internal path changes to avoid passing null to preg_replace().

No files require special attention.

Important Files Changed

Filename Overview
application/dashboard.class.inc.php One-line fix replacing null with $this->GetId() as the null-coalescing fallback in the utils::Sanitize() call, eliminating the PHP 8.3 deprecation warning without changing the effective runtime behavior.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant Dashboard as Dashboard::Render()
    participant Sanitize as utils::Sanitize()
    participant SanitizeInt as utils::Sanitize_Internal()
    participant preg as preg_replace()

    Note over Caller,preg: Before fix (dashboard_div_id absent)
    Caller->>Dashboard: Render($oPage, ...)
    Dashboard->>Sanitize: Sanitize(null, GetId(), ELEMENT_IDENTIFIER)
    Sanitize->>SanitizeInt: Sanitize_Internal(null, ...)
    SanitizeInt->>preg: "preg_replace(..., null) <- PHP 8.3 deprecation"
    preg-->>SanitizeInt: false/null
    SanitizeInt-->>Sanitize: false
    Sanitize-->>Dashboard: GetId() (fallback)

    Note over Caller,preg: After fix (dashboard_div_id absent)
    Caller->>Dashboard: Render($oPage, ...)
    Dashboard->>Sanitize: Sanitize(GetId(), GetId(), ELEMENT_IDENTIFIER)
    Note right of Sanitize: value === defaultValue, short-circuit
    Sanitize-->>Dashboard: GetId() (no preg_replace called)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant Dashboard as Dashboard::Render()
    participant Sanitize as utils::Sanitize()
    participant SanitizeInt as utils::Sanitize_Internal()
    participant preg as preg_replace()

    Note over Caller,preg: Before fix (dashboard_div_id absent)
    Caller->>Dashboard: Render($oPage, ...)
    Dashboard->>Sanitize: Sanitize(null, GetId(), ELEMENT_IDENTIFIER)
    Sanitize->>SanitizeInt: Sanitize_Internal(null, ...)
    SanitizeInt->>preg: "preg_replace(..., null) <- PHP 8.3 deprecation"
    preg-->>SanitizeInt: false/null
    SanitizeInt-->>Sanitize: false
    Sanitize-->>Dashboard: GetId() (fallback)

    Note over Caller,preg: After fix (dashboard_div_id absent)
    Caller->>Dashboard: Render($oPage, ...)
    Dashboard->>Sanitize: Sanitize(GetId(), GetId(), ELEMENT_IDENTIFIER)
    Note right of Sanitize: value === defaultValue, short-circuit
    Sanitize-->>Dashboard: GetId() (no preg_replace called)
Loading

Reviews (1): Last reviewed commit: "N°8886 - Deprecation warning in DashetGr..." | Re-trigger Greptile

Copilot AI 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.

Pull request overview

Fixes a PHP 8.3 deprecation when rendering dashboards without a supplied division ID.

Changes:

  • Uses the dashboard ID as the sanitizer input fallback instead of null.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread application/dashboard.class.inc.php
@bdalsass
bdalsass requested review from Lenaick and steffunky July 21, 2026 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Work made by Combodo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants