test: assert the shipped rows, not what the resolver made of them

Greptile P2, and right: the fixture checks read `resolve_navigation`, which has
already dropped a row naming a missing doctype, a heading over nothing and a
linked item whose sidebar emptied. Against that output they were asserting that
the resolver works, which is frappe's test.

They now read the standard `Rail` and `Sidebar` rows as authored. Checked that
each can fail: an orphan `parent_key` and an empty section both insert happily.
A missing doctype does not -- `link_to` is a Dynamic Link and `_validate_links`
refuses the row at import -- so that test now says what it is actually for, a
doctype removed after the rows were imported.

Adds one: every `parent_key` names a section beside it. An orphan is promoted to
the top level rather than dropped, so it loses its nesting in silence.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

erpnext/tests/test_navigation.py

@@ -108,17 +108,18 @@ class TestERPNextNavigation(IntegrationTestCase):
 	def test_a_linked_item_names_a_sidebar_this_app_ships(self):
 		"""A typo in `link_to` costs the item its panel and nothing else, so nothing reports it.
 
-		`Sidebar` declares the `Derived From Children` permission rule, so an item naming an
-		address that resolves to no rows is dropped by the server's own cascade -- silently, and
-		correctly, because that is also what an emptied sidebar has to do.
+		Read off the **shipped rows**, not the resolved payload. `Sidebar` declares the
+		`Derived From Children` permission rule, so an item naming an address that resolves to no
+		rows is dropped by the server's own cascade -- silently, and correctly, because that is
+		also what an emptied sidebar has to do. Asserting against the resolved list would
+		therefore be asserting that the cascade works, which is frappe's test and not this one.
 		"""
-		frappe.set_user("Administrator")
-		navigation = resolve_navigation(APP)
-		linked = [item for item in navigation["rail"] if item["item_type"] == "Sidebar"]
+		linked = [row for row in shipped("Rail", "erpnext") if row.item_type == "Sidebar"]
+		addresses = set(frappe.get_all("Sidebar", filters={"app": APP, "standard": 1}, pluck="name"))
 
-		self.assertEqual(tuple(item["key"] for item in linked), LINKED)
-		for item in linked:
-			self.assertIn(item["link_to"], navigation["sidebars"])
+		self.assertEqual(tuple(row.key for row in linked), LINKED)
+		for row in linked:
+			self.assertIn(row.link_to, addresses, row.key)
 
 	def test_an_independent_item_is_a_module_and_opens_nothing(self):
 		"""Charter point 1 makes independent a first-class state, and needs no field to say so:
@@ -142,35 +143,45 @@ class TestERPNextNavigation(IntegrationTestCase):
 			self.assertEqual(name, frappe.scrub(f"Module Def {module}"))
 
 	def test_every_destination_is_a_doctype_this_site_has(self):
-		"""A row naming a doctype that is gone is dropped by the filter, not refused at import."""
-		frappe.set_user("Administrator")
-		navigation = resolve_navigation(APP)
-		rows = [*navigation["rail"], *(row for rows in navigation["sidebars"].values() for row in rows)]
+		"""Read off the shipped rows rather than the resolved payload, which has already dropped
+		any row it could not place.
 
-		for row in rows:
-			if row["item_type"] == "DocType":
-				self.assertTrue(frappe.db.exists("DocType", row["link_to"]), row["key"])
+		A typo cannot get this far: `link_to` is a Dynamic Link, so `_validate_links` refuses the
+		row at import with `Could not find Row #N: Link To` -- checked, not assumed. What is left
+		for this to catch is a doctype removed *after* the rows were imported, which arrives as a
+		quietly shorter sidebar and nothing else.
+		"""
+		for container, address in every_container():
+			for row in shipped(container, address):
+				if row.item_type == "DocType":
+					self.assertTrue(frappe.db.exists("DocType", row.link_to), f"{address}/{row.key}")
 
 	def test_keys_are_unique_within_each_container(self):
 		"""Every site and user edit is filed against a key, so two rows sharing one collide."""
-		frappe.set_user("Administrator")
-		navigation = resolve_navigation(APP)
-
-		for rows in (navigation["rail"], *navigation["sidebars"].values()):
-			keys = [row["key"] for row in rows]
-			self.assertEqual(len(keys), len(set(keys)))
+		for container, address in every_container():
+			keys = [row.key for row in shipped(container, address)]
+			self.assertEqual(len(keys), len(set(keys)), address)
 
 	def test_no_section_is_shipped_over_nothing(self):
-		"""A heading with no rows under it is dropped by the cascade at read time, so shipping
-		one would mean shipping a row that can never render. They are left out at authoring."""
-		frappe.set_user("Administrator")
-		navigation = resolve_navigation(APP)
-
-		for rows in navigation["sidebars"].values():
-			parents = {row.get("parent_key") for row in rows}
+		"""A heading with no rows under it is dropped by the cascade at read time, so shipping one
+		would mean shipping a row that can never render. Left out at authoring instead -- and read
+		here off the shipped rows, since the cascade would have hidden it either way."""
+		for container, address in every_container():
+			rows = shipped(container, address)
+			parents = {row.parent_key for row in rows}
 			for row in rows:
-				if row["item_type"] == "Section":
-					self.assertIn(row["key"], parents, row["key"])
+				if row.item_type == "Section":
+					self.assertIn(row.key, parents, f"{address}/{row.key}")
+
+	def test_every_parent_key_names_a_section_beside_it(self):
+		"""A row filed under a heading that is not there loses its nesting silently: the resolver
+		promotes an orphan to the top level rather than dropping it."""
+		for container, address in every_container():
+			rows = shipped(container, address)
+			sections = {row.key for row in rows if row.item_type == "Section"}
+			for row in rows:
+				if row.parent_key:
+					self.assertIn(row.parent_key, sections, f"{address}/{row.key}")
 
 	def test_a_real_user_sees_fewer_modules_than_administrator(self):
 		"""The filter is doing something, which is the one thing an Administrator suite cannot
@@ -220,6 +231,33 @@ class TestERPNextNavigation(IntegrationTestCase):
 		)
 
 
+def every_container() -> list[tuple[str, str]]:
+	"""Every standard record ERPNext ships, as `(doctype, name)`."""
+	return [("Rail", "erpnext")] + [
+		("Sidebar", name)
+		for name in frappe.get_all("Sidebar", filters={"app": APP, "standard": 1}, pluck="name")
+	]
+
+
+def shipped(container: str, address: str) -> list:
+	"""The rows as authored, before the resolver has filtered or cascaded anything away.
+
+	The distinction matters for every fixture assertion below: `resolve_navigation` drops a row
+	naming a doctype that is gone, a heading over nothing and a linked item whose sidebar
+	emptied. Read against its output, a test for those is asserting that the resolver works.
+	"""
+	return frappe.get_all(
+		"Navigation Item",
+		filters={
+			"parenttype": container,
+			"parent": address,
+			"parentfield": "items" if container == "Rail" else "navigation_items",
+		},
+		fields=["key", "parent_key", "item_type", "link_doctype", "link_to"],
+		order_by="idx asc",
+	)
+
+
 def block(module: str, user: str):
 	"""Withdraw a module from one person, the way an administrator does on the User form.