Skip to content

Issue#250991 Feat: Upgrade to Joomla6#180

Merged
tekvishal merged 1 commit intotechjoomla:j6x-masterfrom
tekvishal:j6x-local
Dec 11, 2025
Merged

Issue#250991 Feat: Upgrade to Joomla6#180
tekvishal merged 1 commit intotechjoomla:j6x-masterfrom
tekvishal:j6x-local

Conversation

@tekvishal
Copy link

@tekvishal tekvishal commented Dec 11, 2025

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced Joomla 6 compatibility with updated sidebar and toolbar rendering
  • Improvements

    • Updated form field behaviors from Chosen to multiselect for improved list table interactions
    • Modernized toolbar helper implementations for consistent interface handling
    • Improved application termination handling for better stability
  • Refactor

    • Modernized internal dependency injection and database access patterns
    • Updated form field class structure and inheritance for Joomla compatibility

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 11, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This pull request updates the TJNotifications Joomla component to align with modern Joomla APIs and patterns. Changes include replacing deprecated toolbar and form helpers, updating token validation to use application closure instead of jexit, refactoring form field inheritance, and migrating error handling to exceptions. Additionally, the installer script is refactored to use dependency injection, and model save logic is significantly reworked.

Changes

Cohort / File(s) Summary
Token Validation & Request Termination
src/com_tjnotifications/admin/controllers/notification.php, src/com_tjnotifications/admin/controllers/notifications.php, src/com_tjnotifications/admin/controllers/subscription.php, src/com_tjnotifications/site/controllers/preferences.php
Replaced jexit(Text::_('JINVALID_TOKEN')) with Factory::getApplication()->close() for invalid CSRF token handling across multiple controller methods (save, add, delete). Changes the termination mechanism while preserving functional behavior of halting processing on validation failure.
Form Field Class Inheritance & Imports
src/com_tjnotifications/admin/models/fields/backends.php, src/com_tjnotifications/admin/models/fields/clients.php, src/com_tjnotifications/admin/models/fields/platforms.php, src/com_tjnotifications/admin/models/fields/tjnotificationsbackends.php
Updated form field classes to extend modern ListField instead of legacy JFormFieldList; replaced FormHelper::loadFieldClass with direct use statements. Updated docblock references from "JHtml options" to "HTMLHelper options".
Grouped List Field Migration
src/com_tjnotifications/admin/models/fields/mobilenumberfields.php
Changed inheritance from JFormFieldGroupedList to GroupedlistField; replaced static JLoader::register with conditional require_once check for helper loading.
View Toolbar Refactoring
src/com_tjnotifications/admin/views/notification/view.html.php, src/com_tjnotifications/admin/views/notifications/view.html.php, src/com_tjnotifications/admin/views/subscription/view.html.php, src/com_tjnotifications/admin/views/subscriptions/view.html.php
Replaced JToolBarHelper with ToolbarHelper; renamed addToolBar() to addToolbar() and _setToolBar() to _setToolbar(); added renderSidebar() method for Joomla 6 compatibility; converted error handling to throw exceptions instead of JError::raiseError; updated sidebar rendering.
Template Form Behavior Updates
src/com_tjnotifications/admin/views/logs/tmpl/default.php, src/com_tjnotifications/admin/views/notifications/tmpl/default_bs2.php, src/com_tjnotifications/admin/views/subscription/tmpl/edit.php, src/com_tjnotifications/admin/views/subscriptions/tmpl/default.php
Replaced deprecated formbehavior.chosen with behavior.multiselect for form element initialization.
View Helper & CSV Handling
src/com_tjnotifications/admin/views/logs/view.csv.php, src/com_tjnotifications/admin/views/logs/view.html.php, src/com_tjnotifications/admin/views/notification/view.html.php
Replaced jimport-based imports with conditional require_once includes; converted JError::raiseError to throwing Exception; replaced JHtmlSidebar::render() with empty string placeholder.
Filesystem Import Update
src/com_tjnotifications/admin/helpers/tjnotifications.php
Updated Path class import from Joomla\CMS\Filesystem\Path to Joomla\Filesystem\Path.
Model Changes
src/com_tjnotifications/admin/models/notification.php
Refactored save() method with hardcoded $isNew = true logic, deterministic templateId derivation from data or insertid, explicit backend config save with error handling, simplified webhook validations, and try/catch wrapper around database operations.
Subscription Model Query Update
src/com_tjnotifications/admin/models/subscription.php
Replaced loadResult() with loadColumn()[0] ?? null for retrieving maximum ordering value.
Installer Script Refactoring
src/com_tjnotifications/install.tjnotifications.php
Added constructor with dependency injection for DatabaseInterface; updated method signatures with typed parameters (InstallerAdapter, return types); replaced Factory::getDbo() with injected $this->db; updated error handling with try/catch and Log usage; replaced Installer instantiations with parentheses.
JavaScript Localization Update
src/com_tjnotifications/media/js/tjnotifications.js
Updated Joomla localization lookup from Joomla.JText to Joomla.Text in SMS character limit calculation.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas requiring extra attention:

  • src/com_tjnotifications/admin/models/notification.php — Significant refactoring of save() logic with new error handling patterns; verify templateId derivation logic, backend config save flow, and webhook validation consolidation align with intended behavior.
  • src/com_tjnotifications/install.tjnotifications.php — Constructor dependency injection and typed parameters; review parameter usage consistency, error handling with Log, and InstallSQLFiles return type compliance.
  • Multiple view files (views/*/view.html.php) — Toolbar method renames, exception handling, and Joomla 6 compatibility changes; ensure consistency across all views and that sidebar handling works correctly.
  • Form field inheritance changes — Verify that all field classes properly extend new base classes and that form field functionality is preserved across the migration from legacy to modern Joomla APIs.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 5f7190c and 2f1efbc.

⛔ Files ignored due to path filters (8)
  • src/com_tjnotifications/admin/language/en-GB.com_tjnotifications.ini is excluded by !**/language/**/*.ini
  • src/com_tjnotifications/admin/models/forms/emailfields.xml is excluded by !**/*.xml
  • src/com_tjnotifications/admin/models/forms/pushfields.xml is excluded by !**/*.xml
  • src/com_tjnotifications/admin/models/forms/smsfields.xml is excluded by !**/*.xml
  • src/com_tjnotifications/admin/models/forms/webhookfields.xml is excluded by !**/*.xml
  • src/com_tjnotifications/admin/models/forms/whatsappfields.xml is excluded by !**/*.xml
  • src/com_tjnotifications/media/js/tjnotifications.min.js is excluded by !**/*.min.js, !**/*.min.js
  • src/com_tjnotifications/tjnotifications.xml is excluded by !**/*.xml
📒 Files selected for processing (24)
  • src/com_tjnotifications/admin/controllers/notification.php (2 hunks)
  • src/com_tjnotifications/admin/controllers/notifications.php (1 hunks)
  • src/com_tjnotifications/admin/controllers/subscription.php (1 hunks)
  • src/com_tjnotifications/admin/helpers/tjnotifications.php (1 hunks)
  • src/com_tjnotifications/admin/models/fields/backends.php (3 hunks)
  • src/com_tjnotifications/admin/models/fields/clients.php (1 hunks)
  • src/com_tjnotifications/admin/models/fields/mobilenumberfields.php (2 hunks)
  • src/com_tjnotifications/admin/models/fields/platforms.php (3 hunks)
  • src/com_tjnotifications/admin/models/fields/tjnotificationsbackends.php (2 hunks)
  • src/com_tjnotifications/admin/models/notification.php (7 hunks)
  • src/com_tjnotifications/admin/models/subscription.php (1 hunks)
  • src/com_tjnotifications/admin/views/logs/tmpl/default.php (1 hunks)
  • src/com_tjnotifications/admin/views/logs/view.csv.php (1 hunks)
  • src/com_tjnotifications/admin/views/logs/view.html.php (4 hunks)
  • src/com_tjnotifications/admin/views/notification/view.html.php (6 hunks)
  • src/com_tjnotifications/admin/views/notifications/tmpl/default_bs2.php (1 hunks)
  • src/com_tjnotifications/admin/views/notifications/view.html.php (9 hunks)
  • src/com_tjnotifications/admin/views/subscription/tmpl/edit.php (1 hunks)
  • src/com_tjnotifications/admin/views/subscription/view.html.php (1 hunks)
  • src/com_tjnotifications/admin/views/subscriptions/tmpl/default.php (1 hunks)
  • src/com_tjnotifications/admin/views/subscriptions/view.html.php (6 hunks)
  • src/com_tjnotifications/install.tjnotifications.php (10 hunks)
  • src/com_tjnotifications/media/js/tjnotifications.js (1 hunks)
  • src/com_tjnotifications/site/controllers/preferences.php (2 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tekvishal tekvishal merged commit 32da5f5 into techjoomla:j6x-master Dec 11, 2025
2 of 4 checks passed
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
3.9% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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.

1 participant