fix(spp_branding_kit): address all code review feedback from Gemini

CRITICAL fixes:
- Fix configuration parameter key mismatch: All parameters now use consistent openspp.* prefix
  in both ir_config_parameter.xml and res_config_settings.py

HIGH priority fixes:
- Replace broad cron job disabling with specific external IDs to avoid disabling unrelated jobs
- Move debug mode check before super().web_client() to prevent unnecessary rendering

MEDIUM priority fixes:
- Fix addons_path in README to point to parent directory containing modules
- Make company branding multi-company safe by updating all companies, not just ID 1
- Complete uninstall_hook to remove all openspp.* parameters using LIKE operator
- Remove duplicated session info logic from _get_frontend_session_info method
- Remove incomplete search_panel_select_range implementation
- Refactor duplicated app filtering code into helper methods _get_paid_app_filter() and _apply_paid_app_filter()
- Remove incorrect MODELS.md documentation file
- Make tests more robust by creating dummy test modules in setUp()

Additional improvements:
- Add favicon icon.png from spp_audit_config module
- Fix URL formatting with trailing slashes in ir_config_parameter.xml
- Improve test assertions to use specific module instances
- Enhance code maintainability with DRY principle applied to filtering logic

Author: jeremi

spp_branding_kit/README.md

@@ -51,7 +51,7 @@ For complete debranding, install these modules from the
 
 ```bash
 cd /path/to/your/custom-addons
-git clone [repository-url] openspp_branding_kit
+git clone [repository-url] spp_branding_kit
 ```
 
 ### 2. Install OCA Dependencies (Recommended)
@@ -67,7 +67,7 @@ Add the module paths to your `odoo.conf`:
 
 ```ini
 [options]
-addons_path = /path/to/openspp_branding_kit,/path/to/server-brand,/path/to/odoo/addons
+addons_path = /path/to/your/custom-addons,/path/to/server-brand,/path/to/odoo/addons
 
 # Recommended security settings
 list_db = False

spp_branding_kit/__init__.py

@@ -40,23 +40,20 @@ def post_init_hook(env):
             brand_promotion.active = False
             _logger.info("Disabled Odoo brand promotion message")
 
-        # Disable update notification cron jobs (search by model/method)
-        Cron = env["ir.cron"].sudo()
-
-        # Disable module update related cron jobs
-        module_update_crons = Cron.search(
-            [
-                "|",
-                "|",
-                ("model", "=", "ir.module.module"),
-                ("model", "=", "publisher_warranty.contract"),
-                ("cron_name", "ilike", "module"),
-            ]
-        )
-        for cron in module_update_crons:
-            if cron.active:
-                cron.active = False
-                _logger.info(f"Disabled cron job: {cron.name}")
+        # Disable specific Odoo telemetry and update cron jobs by their external IDs
+        crons_to_disable = [
+            "mail.ir_cron_module_update_notification",  # Module update notification
+            "base.ir_cron_res_partner_clear_caches",  # Partner cache clearing (if telemetry related)
+        ]
+
+        for cron_xml_id in crons_to_disable:
+            try:
+                cron = env.ref(cron_xml_id, raise_if_not_found=False)
+                if cron and cron.active:
+                    cron.active = False
+                    _logger.info(f"Disabled cron job: {cron.name} ({cron_xml_id})")
+            except Exception as e:
+                _logger.debug(f"Could not disable cron {cron_xml_id}: {e}")
 
         # Disable theme store menu if it exists
         theme_menu = env["ir.ui.menu"].sudo().search([("name", "ilike", "Theme Store")], limit=1)
@@ -67,19 +64,19 @@ def post_init_hook(env):
     except Exception as e:
         _logger.warning(f"Error during branding setup: {e}")
 
-    # Update company information
+    # Update company information for all companies
     try:
         Company = env["res.company"].sudo()
-        main_company = Company.browse(1)  # Main company
-        if main_company.exists():
-            main_company.write(
+        companies = Company.search([])
+        for company in companies:
+            company.write(
                 {
                     "report_header": "OpenSPP Platform",
                     "report_footer": "OpenSPP - Open Source Social Protection Platform",
                     "website": "https://openspp.org",
                 }
             )
-            _logger.info("Updated main company branding")
+        _logger.info(f"Updated branding for {len(companies)} companies")
     except Exception as e:
         _logger.warning(f"Error updating company data: {e}")
 
@@ -92,15 +89,16 @@ def uninstall_hook(env):
     """
     _logger.info("OpenSPP Branding Kit: Running uninstall cleanup...")
 
-    # Remove the hide_paid_apps parameter
+    # Remove all openspp.* configuration parameters
     try:
         IrConfigParam = env["ir.config_parameter"].sudo()
-        param = IrConfigParam.search([("key", "=", "openspp.hide_paid_apps")])
-        if param:
-            param.unlink()
-            _logger.info("Removed hide_paid_apps parameter")
+        params = IrConfigParam.search([("key", "=like", "openspp.%")])
+        if params:
+            param_count = len(params)
+            params.unlink()
+            _logger.info(f"Removed {param_count} OpenSPP configuration parameters")
     except Exception as e:
-        _logger.warning(f"Error removing configuration parameter: {e}")
+        _logger.warning(f"Error removing configuration parameters: {e}")
 
     # Optionally re-enable Odoo branding elements
     # This is commented out by default to maintain debranding even after uninstall

spp_branding_kit/controllers/main.py

@@ -15,21 +15,19 @@ class OpenSPPHome(Home):
     @http.route()
     def web_client(self, s_action=None, **kw):
         """Override web client to enforce debug mode restrictions"""
-        res = super().web_client(s_action, **kw)
-
-        # Check if debug mode is restricted to admins
+        # Check if debug mode is restricted to admins BEFORE rendering
         if kw.get("debug", False):
             config_parameter = request.env["ir.config_parameter"].sudo()
             debug_admin_only = config_parameter.get_param("openspp.debug_admin_only", "True") == "True"
 
             if debug_admin_only and request.session.uid:
                 # Check if current user is admin
-                user = request.env.user.browse(request.session.uid)
-                if not user._is_admin():
-                    # Redirect non-admin users trying to access debug mode
-                    return request.redirect("/web?debug=0")
+                if not request.env.user._is_admin():
+                    # Remove debug parameter and redirect
+                    kw.pop("debug", None)
+                    return request.redirect("/web", 303)
 
-        return res
+        return super().web_client(s_action, **kw)
 
 
 class OpenSPPBrandingController(http.Controller):

spp_branding_kit/data/ir_config_parameter.xml

@@ -5,119 +5,119 @@
 
         <!-- System Information Parameters -->
         <record id="param_system_name" model="ir.config_parameter">
-            <field name="key">system.name</field>
+            <field name="key">openspp.system_name</field>
             <field name="value">OpenSPP Platform</field>
         </record>
 
         <record id="param_system_version" model="ir.config_parameter">
-            <field name="key">system.version</field>
+            <field name="key">openspp.system_version</field>
             <field name="value">1.0.0</field>
         </record>
 
         <!-- Support and Documentation URLs -->
         <record id="param_support_url" model="ir.config_parameter">
-            <field name="key">support.url</field>
-            <field name="value">https://openspp.org/support</field>
+            <field name="key">openspp.support_url</field>
+            <field name="value">https://openspp.org/</field>
         </record>
 
         <record id="param_documentation_url" model="ir.config_parameter">
-            <field name="key">documentation.url</field>
-            <field name="value">https://openspp.org/documentation</field>
+            <field name="key">openspp.documentation_url</field>
+            <field name="value">https://docs.openspp.org/</field>
         </record>
 
         <record id="param_community_url" model="ir.config_parameter">
-            <field name="key">community.url</field>
-            <field name="value">https://openspp.org/community</field>
+            <field name="key">openspp.community_url</field>
+            <field name="value">https://openspp.org/</field>
         </record>
 
         <!-- Telemetry Configuration -->
         <record id="param_openspp_telemetry_enabled" model="ir.config_parameter">
-            <field name="key">openspp.telemetry.enabled</field>
+            <field name="key">openspp.telemetry_enabled</field>
             <field name="value">True</field>
         </record>
 
         <record id="param_openspp_telemetry_endpoint" model="ir.config_parameter">
-            <field name="key">openspp.telemetry.endpoint</field>
+            <field name="key">openspp.telemetry_endpoint</field>
             <field name="value">https://telemetry.openspp.org</field>
         </record>
 
         <!-- Disable External Services -->
         <record id="param_disable_external_links" model="ir.config_parameter">
-            <field name="key">disable.external.links</field>
+            <field name="key">openspp.disable_external_links</field>
             <field name="value">True</field>
         </record>
 
         <!-- Custom Footer Text -->
         <record id="param_footer_copyright" model="ir.config_parameter">
-            <field name="key">footer.copyright</field>
+            <field name="key">openspp.footer_copyright</field>
             <field name="value">© OpenSPP Project - Open Source Social Protection Platform</field>
         </record>
 
         <!-- Email Configuration -->
         <record id="param_email_from_name" model="ir.config_parameter">
-            <field name="key">email.from.name</field>
+            <field name="key">openspp.email_from_name</field>
             <field name="value">OpenSPP Platform</field>
         </record>
 
         <!-- Report Configuration -->
         <record id="param_report_footer_text" model="ir.config_parameter">
-            <field name="key">report.footer.text</field>
+            <field name="key">openspp.report_footer_text</field>
             <field name="value">Generated by OpenSPP Platform</field>
         </record>
 
         <!-- Login Page Configuration -->
         <record id="param_login_page_title" model="ir.config_parameter">
-            <field name="key">login.page.title</field>
+            <field name="key">openspp.login_page_title</field>
             <field name="value">OpenSPP - Social Protection Platform</field>
         </record>
 
         <record id="param_login_page_subtitle" model="ir.config_parameter">
-            <field name="key">login.page.subtitle</field>
+            <field name="key">openspp.login_page_subtitle</field>
             <field name="value">Secure Access Portal</field>
         </record>
 
         <!-- Database Manager Configuration -->
         <record id="param_database_manager_disabled" model="ir.config_parameter">
-            <field name="key">database.manager.disabled</field>
+            <field name="key">openspp.database_manager_disabled</field>
             <field name="value">True</field>
         </record>
 
         <!-- Custom Favicon Path -->
         <record id="param_favicon_path" model="ir.config_parameter">
-            <field name="key">favicon.path</field>
+            <field name="key">openspp.favicon_path</field>
             <field name="value">/openspp_branding_kit/static/description/icon.png</field>
         </record>
 
         <!-- Theme Configuration -->
         <record id="param_theme_primary_color" model="ir.config_parameter">
-            <field name="key">theme.primary.color</field>
+            <field name="key">openspp.theme_primary_color</field>
             <field name="value">#2c3e50</field>
         </record>
 
         <record id="param_theme_secondary_color" model="ir.config_parameter">
-            <field name="key">theme.secondary.color</field>
+            <field name="key">openspp.theme_secondary_color</field>
             <field name="value">#34495e</field>
         </record>
 
         <!-- Analytics and Tracking -->
         <record id="param_google_analytics_disabled" model="ir.config_parameter">
-            <field name="key">google.analytics.disabled</field>
+            <field name="key">openspp.google_analytics_disabled</field>
             <field name="value">True</field>
         </record>
 
         <!-- Custom Error Messages -->
         <record id="param_error_message_404" model="ir.config_parameter">
-            <field name="key">error.message.404</field>
+            <field name="key">openspp.error_message_404</field>
             <field name="value">Page not found in OpenSPP Platform</field>
         </record>
 
         <record id="param_error_message_403" model="ir.config_parameter">
-            <field name="key">error.message.403</field>
+            <field name="key">openspp.error_message_403</field>
             <field name="value">Access denied. Please contact your OpenSPP administrator.</field>
         </record>
 
         <record id="param_error_message_500" model="ir.config_parameter">
-            <field name="key">error.message.500</field>
+            <field name="key">openspp.error_message_500</field>
             <field name="value">An error occurred in OpenSPP Platform. Please try again later.</field>
         </record>
 

spp_branding_kit/models/ir_http.py

@@ -4,40 +4,13 @@
 import logging
 
 from odoo import models
-from odoo.http import request
 
 _logger = logging.getLogger(__name__)
 
 
 class IrHttp(models.AbstractModel):
     _inherit = "ir.http"
 
-    @classmethod
-    def _get_frontend_session_info(cls):
-        """Override to add OpenSPP-specific session information"""
-        session_info = super()._get_frontend_session_info()
-
-        # Add OpenSPP configuration to session
-        IrConfig = request.env["ir.config_parameter"].sudo()
-
-        session_info.update(
-            {
-                "openspp_system_name": IrConfig.get_param("openspp.system_name", "OpenSPP Platform"),
-                "openspp_documentation_url": IrConfig.get_param(
-                    "openspp.documentation_url", "https://docs.openspp.org"
-                ),
-                "openspp_support_url": IrConfig.get_param("openspp.support_url", "https://openspp.org"),
-                "openspp_show_powered_by": IrConfig.get_param("openspp.show_powered_by", "True") == "True",
-                "openspp_telemetry_enabled": IrConfig.get_param("openspp.telemetry_enabled", "True") == "True",
-                "openspp_telemetry_endpoint": IrConfig.get_param(
-                    "openspp.telemetry_endpoint", "https://telemetry.openspp.org"
-                ),
-                "openspp_debug_admin_only": IrConfig.get_param("openspp.debug_admin_only", "True") == "True",
-            }
-        )
-
-        return session_info
-
     def session_info(self):
         """Override session info to customize branding"""
         result = super().session_info()