Skip to content

Default new graph assignments to Time Graph View - #293

Merged
TheWitness merged 8 commits into
Cacti:developfrom
alcatron:feat/detailed-graph-default
Oct 11, 2026
Merged

TheWitness merged 8 commits into
Cacti:developfrom
alcatron:feat/detailed-graph-default

Conversation

@alcatron

@alcatron alcatron commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

New graph assignments default to Time Graph View so clicking a traffic link opens its detailed graph. Explicit stored Standard/Preview choices remain supported, and existing map links are unchanged.

Validation: 226 PHP 8.3 tests passed in an isolated fixture layout, including default and stored-preference regression checks. Syntax, whitespace and manifest checks passed; measured changed-line coverage is 100%. Full running-installation integration remains unverified for this revision.

@TheWitness
TheWitness requested review from TheWitness and a balanced review from Copilot October 9, 2026 15:12

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.

🟢 Approval recommended

The focused behavior change is correct and covered, with only a non-blocking formatting issue noted.

1 open finding
What changed in this PR

Updates new graph assignments to default to Time Graph View without altering stored preferences or existing map links.

Changes:

  • Changes the Info URL default to Time Graph View.
  • Adds regression coverage for default and stored preference behavior.
  • Documents the change.
File Description
setup.php Updates the Info URL style default.
tests/​Unit/​GraphUrlDefaultTest.php Tests the new default and preference preservation.
CHANGELOG.md Records feature #293.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/Unit/GraphUrlDefaultTest.php Outdated
alcatron pushed a commit to alcatron/plugin_weathermap that referenced this pull request Oct 9, 2026
@TheWitness
TheWitness requested a balanced review from Copilot October 9, 2026 18:08

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.

🟢 Approval recommended

The focused default change is correct, backward-compatible, and covered by regression testing.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

@alcatron
alcatron force-pushed the feat/detailed-graph-default branch from b7808dd to b320198 Compare October 9, 2026 23:41
@TheWitness
TheWitness requested a balanced review from Copilot October 10, 2026 09:50

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.

🟢 Approval recommended

The focused default change preserves explicit settings and includes appropriate regression coverage.

0 open findings

🧠 Review effort: Balanced

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.

🟢 Approval recommended

The focused default change is consistent with Cacti’s settings fallback behavior and has appropriate regression coverage.

0 open findings

🧠 Review effort: Balanced

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.

🟢 Approval recommended

The focused change is backward-compatible and adequately tested.

0 open findings

🧠 Review effort: Balanced

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.

🟢 Approval recommended

The focused default change is consistent with existing editor behavior and has appropriate regression coverage.

0 open findings

🧠 Review effort: Balanced

@alcatron

alcatron commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor Author

Merged latest develop and the existing graph-default regression formatting update, resolving the changelog conflict while preserving the feature. All 231 local PHP tests pass (5464 assertions); syntax, manifest, whitespace and translation-template checks pass. These remain isolated fixture checks, not a fresh full-installation browser audit.

@somethingwithproof somethingwithproof added the enhancement New feature or request label Oct 10, 2026
@alcatron

Copy link
Copy Markdown
Contributor Author

Resolved the new changelog conflict after #292 merged. All 234 local tests pass (5489 assertions), with syntax, manifest and whitespace checks passing. The feature changes remain intact; validation uses isolated fixtures.

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.

🟢 Approval recommended

The focused configuration change is correct, backward-compatible, and covered by an appropriate regression test.

0 open findings

🧠 Review effort: Balanced

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants