diff --git a/commitlint.config.js b/commitlint.config.js index 55f1dff..ce91a16 100644 --- a/commitlint.config.js +++ b/commitlint.config.js @@ -1,7 +1,7 @@ module.exports = { - extends: ["@commitlint/config-conventional"], - // Disable default 100-char (or 72-char) header length limit for commit messages - rules: { - "header-max-length": [0, "always", 100], - }, + extends: ["@commitlint/config-conventional"], + // Disable default 100-char (or 72-char) header length limit for commit messages + rules: { + "header-max-length": [0, "always", 100], + }, }; diff --git a/frappe_desk_theme/frappe_desk_theme/doctype/desk_theme/test_desk_theme.py b/frappe_desk_theme/frappe_desk_theme/doctype/desk_theme/test_desk_theme.py index 4153c63..757fc23 100644 --- a/frappe_desk_theme/frappe_desk_theme/doctype/desk_theme/test_desk_theme.py +++ b/frappe_desk_theme/frappe_desk_theme/doctype/desk_theme/test_desk_theme.py @@ -1,20 +1,171 @@ # Copyright (c) 2025, Dhwani RIS and Contributors # See license.txt -# import frappe +import importlib.util +import os +import unittest + +import frappe +from frappe.auth import CookieManager, LoginManager from frappe.tests import IntegrationTestCase +from playwright.sync.api import sync_playwright -# On IntegrationTestCase, the doctype test records and all -# link-field test record dependencies are recursively loaded -# Use these module variables to add/remove to/from that list -EXTRA_TEST_RECORD_DEPENDENCIES = [] # eg. ["User"] -IGNORE_TEST_RECORD_DEPENDENCIES = [] # eg. ["User"] +def _playwright_available() -> bool: + return importlib.util.find_spec("playwright") is not None -class IntegrationTestDeskTheme(IntegrationTestCase): - """ - Integration tests for DeskTheme. - Use this class for testing interactions between multiple components. + +class TestDeskTheme(unittest.TestCase): + """Schema + data-layer tests for the Desk Theme hide_search / + hide_notification fields. + + Uses plain unittest.TestCase (not IntegrationTestCase) because these + tests don't insert any Desk Theme rows — they only read meta and + round-trip the existing singleton. Avoiding IntegrationTestCase skips + Frappe's transitive fixture loader (Email Account → ToDo → …) which + can fail on downstream sites that customise those doctypes.""" + + def test_doctype_exposes_hide_notification_and_hide_search_fields(self): + """Field schema is the contract the bundle JS reads against — the + bug history was: schema field exists, JS never consumed it. This + test fails fast if a future refactor removes either field, so the + client-side toggle code wouldn't silently regress.""" + meta = frappe.get_meta("Desk Theme") + hide_notification = meta.get_field("hide_notification") + hide_search = meta.get_field("hide_search") + + self.assertIsNotNone(hide_notification, "hide_notification field removed") + self.assertEqual(hide_notification.fieldtype, "Check") + + self.assertIsNotNone(hide_search, "hide_search field removed") + self.assertEqual(hide_search.fieldtype, "Table MultiSelect") + self.assertEqual(hide_search.options, "Has Role") + + def test_api_round_trips_both_fields(self): + """The bundle JS reads both fields from get_custom_theme via /api/method. + Round-tripping through frappe.get_doc proves the data layer carries + both values to the client; if either disappears here, the JS reads + undefined and silently no-ops.""" + theme = frappe.get_single("Desk Theme") + prev_notif = theme.hide_notification + prev_roles = [row.role for row in (theme.hide_search or [])] + + try: + theme.hide_notification = 1 + theme.set("hide_search", []) + theme.append("hide_search", {"role": "Administrator"}) + theme.save(ignore_permissions=True) + frappe.db.commit() # nosemgrep + + reloaded = frappe.get_single("Desk Theme") + self.assertEqual(reloaded.hide_notification, 1) + self.assertEqual( + [row.role for row in reloaded.hide_search], + ["Administrator"], + ) + finally: + theme = frappe.get_single("Desk Theme") + theme.hide_notification = prev_notif + theme.set("hide_search", []) + for role in prev_roles: + theme.append("hide_search", {"role": role}) + theme.save(ignore_permissions=True) + frappe.db.commit() # nosemgrep + + +@unittest.skipUnless(_playwright_available(), "playwright not installed") +@unittest.skipUnless(os.environ.get("DESK_THEME_E2E_SITE"), "DESK_THEME_E2E_SITE not set") +class E2EDeskThemeToggle(IntegrationTestCase): + """End-to-end browser test that exercises the JS bundle directly. + + Set DESK_THEME_E2E_SITE=http://bsebeamv16.localhost:8003 (a running bench + with the user `Administrator`) before invoking. Skipped otherwise so + standard CI runs aren't blocked on the bench dev server. + + Browser launch lives in setUp / tearDown (per-instance) rather than + setUpClass / tearDownClass so the test file conforms to the project's + snake_case naming policy. The extra ~2s per test is negligible given + these tests only run in opt-in E2E mode. """ - pass + def setUp(self): + super().setUp() + self._pw = sync_playwright().start() + self.browser = self._pw.chromium.launch(headless=True) + self.base = os.environ["DESK_THEME_E2E_SITE"].rstrip("/") + self._original_notif = frappe.db.get_single_value("Desk Theme", "hide_notification") + + def tearDown(self): + try: + theme = frappe.get_single("Desk Theme") + theme.hide_notification = self._original_notif + theme.set("hide_search", []) + theme.save(ignore_permissions=True) + frappe.db.commit() # nosemgrep + finally: + self.browser.close() + self._pw.stop() + super().tearDown() + + def _login_url(self) -> str: + """Mint a single-use SID for Administrator on the live site.""" + frappe.local.cookie_manager = CookieManager() + frappe.local.login_manager = LoginManager() + frappe.local.login_manager.login_as("Administrator") + sid = frappe.session.sid + return f"{self.base}/app?sid={sid}" + + def _open(self): + ctx = self.browser.new_context(viewport={"width": 1440, "height": 900}) + page = ctx.new_page() + page.goto(self._login_url(), wait_until="networkidle", timeout=60000) + page.goto(self.base + "/app/dashboard", wait_until="networkidle", timeout=60000) + # Wait for sidebar to render (polling — wait_for_selector is flaky on first paint) + for _ in range(60): + if page.evaluate("document.querySelectorAll('.body-sidebar .sidebar-item-container').length") > 5: + break + page.wait_for_timeout(250) + return ctx, page + + def _set_theme(self, hide_notification: bool, hide_for_roles: list[str]): + theme = frappe.get_single("Desk Theme") + theme.hide_notification = 1 if hide_notification else 0 + theme.set("hide_search", []) + for role in hide_for_roles: + theme.append("hide_search", {"role": role}) + theme.save(ignore_permissions=True) + frappe.db.commit() # nosemgrep + + def _is_hidden(self, page, selector: str) -> bool: + return page.evaluate(f"""() => {{ + const el = document.querySelector('{selector}'); + if (!el) return null; + const s = getComputedStyle(el); + return s.display === 'none' || el.offsetHeight === 0; + }}""") + + def test_hide_notification_flag_actually_hides_sidebar_bell(self): + # Off → bell visible + self._set_theme(hide_notification=False, hide_for_roles=[]) + ctx, page = self._open() + self.assertFalse(self._is_hidden(page, ".body-sidebar .sidebar-notification")) + ctx.close() + + # On → bell hidden + self._set_theme(hide_notification=True, hide_for_roles=[]) + ctx, page = self._open() + self.assertTrue(self._is_hidden(page, ".body-sidebar .sidebar-notification")) + ctx.close() + + def test_hide_search_for_role_actually_hides_sidebar_search(self): + # Administrator not in list → search visible + self._set_theme(hide_notification=False, hide_for_roles=[]) + ctx, page = self._open() + self.assertFalse(self._is_hidden(page, ".body-sidebar .navbar-search-bar")) + ctx.close() + + # Administrator in list → search hidden + self._set_theme(hide_notification=False, hide_for_roles=["Administrator"]) + ctx, page = self._open() + self.assertTrue(self._is_hidden(page, ".body-sidebar .navbar-search-bar")) + ctx.close() diff --git a/frappe_desk_theme/public/js/frappe_desk_theme.bundle.js b/frappe_desk_theme/public/js/frappe_desk_theme.bundle.js index 2436adb..d671798 100644 --- a/frappe_desk_theme/public/js/frappe_desk_theme.bundle.js +++ b/frappe_desk_theme/public/js/frappe_desk_theme.bundle.js @@ -677,6 +677,7 @@ class FrappeDeskTheme { this.setCSSVariables(); this.toggleSidebar(); this.toggleSearchBar(); + this.toggleNotification(); this.hideStandardMenu(); this.applyFixedSidebarBehavior(); this.performInitialSidebarLoginRedirect(); @@ -725,18 +726,53 @@ class FrappeDeskTheme { } /** - * Toggle search bar visibility based on user roles - * Hides search bar if current user's role matches hide_search configuration + * Toggle search bar visibility based on user roles. + * Hides search bar if current user's role matches hide_search configuration, + * otherwise restores its native display. + * + * Frappe v15 used `.input-group.search-bar.text-muted` in the top navbar. + * Frappe v16 moved the search affordance into the sidebar rail as + * `.body-sidebar .navbar-search-bar`. Both are queried so the setting + * works on either version. */ toggleSearchBar() { - const searchBar = document.querySelector(".input-group.search-bar.text-muted"); - if (!searchBar) { - return; - } + const selectors = [ + // v15 navbar search box + ".input-group.search-bar.text-muted", + // v16 sidebar search rail + ".body-sidebar .navbar-search-bar", + ]; + const shouldHide = this.getUserRoles(); + selectors.forEach((sel) => { + document.querySelectorAll(sel).forEach((el) => { + // Restore via empty string so the element falls back to its + // stylesheet display value when the role no longer matches. + el.style.display = shouldHide ? "none" : ""; + }); + }); + } - if (this.getUserRoles()) { - searchBar.style.display = "none"; + /** + * Toggle the desk notification bell based on the hide_notification flag. + * Acts on both the v15 navbar bell and the v16 sidebar bell so the same + * theme setting works regardless of Frappe version. + */ + toggleNotification() { + if (!this.themeData) { + return; } + const selectors = [ + // v16 sidebar notification + ".body-sidebar .sidebar-notification", + // v15 navbar notification + ".navbar-nav .dropdown-notifications", + ]; + const shouldHide = !!this.themeData.hide_notification; + selectors.forEach((sel) => { + document.querySelectorAll(sel).forEach((el) => { + el.style.display = shouldHide ? "none" : ""; + }); + }); } /** @@ -1106,6 +1142,7 @@ class FrappeDeskTheme { let footerTimeout; const observer = new MutationObserver(() => { this.toggleSearchBar(); + this.toggleNotification(); this.hideStandardMenu(); this.applyFixedSidebarBehavior(); this.performInitialSidebarLoginRedirect();