Simplified the add child mechanism, by using a query parameter ?parent=nid

(instead of specific node/{nid}/add_child/page url)
Fixed implementation of `CMS_NODE_API.is_node_a_parent_of (..)',
  and improved parent validity checking against cycle.

modules/node/cms_node_api.e

@@ -354,8 +354,6 @@ feature -- Access: page/book outline
 			no_cycle: across Result as c all not is_node_a_parent_of (a_node, c.item) end
 		end
 
-feature {NONE} -- Implementation
-
 	is_node_a_parent_of (a_node: CMS_NODE; a_child: CMS_NODE): BOOLEAN
 			-- Is `a_node' a direct or indirect parent of node `a_child'?
 		require
@@ -365,7 +363,7 @@ feature {NONE} -- Implementation
 				attached {CMS_PAGE} full_node (a_child) as l_child_page and then
 				attached l_child_page.parent as l_parent
 			then
-				if l_parent.same_node (l_child_page) then
+				if l_parent.same_node (a_node) then
 					Result := True
 				else
 					Result := is_node_a_parent_of (a_node, l_parent)

modules/node/cms_node_module.e

@@ -207,10 +207,6 @@ feature -- Access: router
 			a_router.handle ("/node/{id}/delete", l_node_handler, a_router.methods_get_post)
 			a_router.handle ("/node/{id}/trash", l_node_handler, a_router.methods_get_post)
 
-				-- Add child
-			a_router.handle ("/node/{id}/add_child/{type}", l_node_handler, a_router.methods_get_post)
-
-
 			a_router.handle ("/node/{id}", l_node_handler, a_router.methods_get)
 				-- For now: no REST API handling... a_router.methods_get_put_delete + a_router.methods_get_post)
 

modules/node/handler/cms_page_node_type_webform_manager.e

@@ -29,30 +29,35 @@ feature -- Forms ...
 	populate_form (response: NODE_RESPONSE; f: CMS_FORM; a_node: detachable CMS_NODE)
 		local
 			ti: WSF_FORM_NUMBER_INPUT
+			fs: WSF_FORM_FIELD_SET
 			l_parent_id, nid: INTEGER_64
 		do
 			Precursor (response, f, a_node)
 
 			if attached {CMS_PAGE} a_node as l_page then
+				create fs.make
+				fs.set_legend ("Pages structure")
+				fs.set_collapsible (True)
+				f.extend (fs)
 				create ti.make ("select_parent_node")
-
+				ti.set_label ("Parent page")
+				ti.set_description ("The parent page is the book structure.")
 				if attached l_page.parent as l_parent_node then
 					l_parent_id := l_parent_node.id
-					f.extend_html_text ("<div><strong>Currently, the parent page is </strong> ")
-					f.extend_html_text (response.node_html_link (l_parent_node,  l_parent_node.title))
-					f.extend_html_text ("</div>")
-					ti.set_label ("Change parent")
-					ti.set_description ("Select a new parent ...")
-				else
-					ti.set_label ("Select parent")
-					ti.set_description ("Select a parent ...")
+					fs.extend_html_text ("<div><strong>Currently, the parent page is </strong> ")
+					fs.extend_html_text (response.node_html_link (l_parent_node,  l_parent_node.title))
+					fs.extend_html_text ("</div>")
 				end
 				ti.set_validation_action (agent parent_validation (response, ?))
-				f.extend (ti)
+				fs.extend (ti)
 
-				if response.location.ends_with_general ("/add_child/page") then
-					nid := response.node_id_path_parameter (response.request)
-					l_parent_id := nid
+				-- FIXME: add notion of "weight"
+
+				if
+					attached {WSF_STRING} response.request.query_parameter ("parent") as p_parent and then
+					p_parent.is_integer
+				then
+					l_parent_id := p_parent.integer_value.to_integer_64
 				end
 				if l_parent_id > 0 then
 					ti.set_default_value (l_parent_id.out)
@@ -100,6 +105,8 @@ feature -- Forms ...
 			l_selected: BOOLEAN
 			node_api: CMS_NODE_API
 			l_parent_id: INTEGER_64
+			nid: INTEGER_64
+			l_parent_node: detachable CMS_NODE
 		do
 			node_api := a_response.node_api
 			if attached fd.integer_item ("select_parent_node") as s_parent_node then
@@ -107,22 +114,18 @@ feature -- Forms ...
 			else
 				l_parent_id := 0
 			end
-			if
-				l_parent_id > 0 and then
-				attached node_api.node (a_response.node_id_path_parameter (a_response.request)) as l_node
-			then
-				if not a_response.location.ends_with_general ("/add_child/page") then
-					across
-						node_api.available_parents_for_node (l_node) as ic
-					until
-						l_selected
-					loop
-						if ic.item.id = l_parent_id then
-							l_selected := True
-						end
-					end
-					if not l_selected then
-						fd.report_invalid_field ("select_parent_node", "Invalid node id " +  l_parent_id.out)
+			if l_parent_id > 0 then
+				l_parent_node := node_api.node (l_parent_id)
+				if l_parent_node = Void then
+					fd.report_invalid_field ("select_parent_node", "Invalid parent, not found id #" + l_parent_id.out)
+				else
+					nid := a_response.node_id_path_parameter
+					if
+						nid > 0 and then
+						attached node_api.node (nid) as l_node and then
+						node_api.is_node_a_parent_of (l_node, l_parent_node)
+					then
+						fd.report_invalid_field ("select_parent_node", "Invalid parent due to cycle (node #" + nid.out + " is already a parent of node #" + l_parent_id.out)
 					end
 				end
 			elseif l_parent_id = -1 or else l_parent_id = 0 then
@@ -147,7 +150,7 @@ feature -- Output
 
 			if a_node.has_id and then not a_node.is_trashed then
 				if node_api.has_permission_for_action_on_node ("create", a_node, a_response.user) then
-					create lnk.make ("Add Child", node_api.node_path (a_node) + "/add_child/page")
+					create lnk.make ("Add Child", "node/add/page?parent=" + a_node.id.out)
 					lnk.set_weight (3)
 					a_response.add_to_primary_tabs (lnk)
 				end

modules/node/handler/node_form_response.e

@@ -40,7 +40,7 @@ feature -- Execution
 			nid: INTEGER_64
 		do
 			create b.make_empty
-			nid := node_id_path_parameter (request)
+			nid := node_id_path_parameter
 			if
 				nid > 0 and then
 				attached node_api.node (nid) as l_node
@@ -61,12 +61,6 @@ feature -- Execution
 						node_api.has_permission_for_action_on_node ("trash", l_node, user)
 					then
 						trash_node (l_node, l_type, b)
-					elseif
-						location.ends_with_general ("/add_child/page") and then
-						has_permissions (<<"create any", "create " + l_type.name>>)
-					then
-							-- FIXME: remove page dep from node module.
-						create_new_node (l_type, b)
 					else
 						b.append ("<h1>")
 						b.append (translation ("Access denied", Void))
@@ -264,7 +258,6 @@ feature -- Form
 			l_node: detachable CMS_NODE
 			s: STRING
 			l_path_alias: detachable READABLE_STRING_8
-			nid: INTEGER_64
 		do
 			fixme ("Refactor code per operacion: Preview, Save")
 			l_preview := attached {WSF_STRING} fd.item ("op") as l_op and then l_op.same_string ("Preview")

modules/node/handler/node_handler.e

@@ -99,15 +99,15 @@ feature -- HTTP Methods
 				edit_response.execute
 			elseif req.percent_encoded_path_info.ends_with ("/revision") then
 				do_revisions (req, res)
-			elseif req.percent_encoded_path_info.ends_with ("/add_child/page") then
-					-- Add child node
-				l_nid := node_id_path_parameter (req)
-				if l_nid > 0 then
-						-- create a new child node with node id `l_id' as parent.
-					create_new_node (req, res)
-				else
-					send_not_found (req, res)
-				end
+--			elseif req.percent_encoded_path_info.ends_with ("/add_child/page") then
+--					-- Add child node
+--				l_nid := node_id_path_parameter (req)
+--				if l_nid > 0 then
+--						-- create a new child node with node id `l_id' as parent.
+--					create_new_node (req, res)
+--				else
+--					send_not_found (req, res)
+--				end
 			else
 					-- Display existing node
 				l_nid := node_id_path_parameter (req)

modules/node/handler/node_response.e

@@ -27,12 +27,12 @@ feature -- Access
 
 feature -- Helpers
 
-	node_id_path_parameter (req: WSF_REQUEST): INTEGER_64
-			-- Node id passed as path parameter for request `req'.
+	node_id_path_parameter: INTEGER_64
+			-- Node id passed as path parameter for request `request'.
 		local
 			s: STRING
 		do
-			if attached {WSF_STRING} req.path_parameter ("id") as p_nid then
+			if attached {WSF_STRING} request.path_parameter ("id") as p_nid then
 				s := p_nid.value
 				if s.is_integer_64 then
 					Result := s.to_integer_64

modules/node/handler/node_view_response.e

@@ -61,7 +61,7 @@ feature -- Execution
 		do
 			l_node := node
 			if l_node = Void then
-				nid := node_id_path_parameter (request)
+				nid := node_id_path_parameter
 				if nid > 0 then
 					l_node := node_api.node (nid)
 				end