From 18e159ad3c81f2beeadd881e6af9ae4b4ecca46e Mon Sep 17 00:00:00 2001 From: Jocelyn Fiat Date: Wed, 9 Sep 2015 23:04:08 +0200 Subject: [PATCH] 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 | 4 +- modules/node/cms_node_module.e | 4 -- .../cms_page_node_type_webform_manager.e | 63 ++++++++++--------- modules/node/handler/node_form_response.e | 9 +-- modules/node/handler/node_handler.e | 18 +++--- modules/node/handler/node_response.e | 6 +- modules/node/handler/node_view_response.e | 2 +- 7 files changed, 48 insertions(+), 58 deletions(-) diff --git a/modules/node/cms_node_api.e b/modules/node/cms_node_api.e index 5942337..9aff292 100644 --- a/modules/node/cms_node_api.e +++ b/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) diff --git a/modules/node/cms_node_module.e b/modules/node/cms_node_module.e index 9636d47..f4f6e79 100644 --- a/modules/node/cms_node_module.e +++ b/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) diff --git a/modules/node/handler/cms_page_node_type_webform_manager.e b/modules/node/handler/cms_page_node_type_webform_manager.e index 808d76a..78a5686 100644 --- a/modules/node/handler/cms_page_node_type_webform_manager.e +++ b/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 ("
Currently, the parent page is ") - f.extend_html_text (response.node_html_link (l_parent_node, l_parent_node.title)) - f.extend_html_text ("
") - 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 ("
Currently, the parent page is ") + fs.extend_html_text (response.node_html_link (l_parent_node, l_parent_node.title)) + fs.extend_html_text ("
") 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 diff --git a/modules/node/handler/node_form_response.e b/modules/node/handler/node_form_response.e index 80b039a..4b68eca 100644 --- a/modules/node/handler/node_form_response.e +++ b/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 ("

") 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") diff --git a/modules/node/handler/node_handler.e b/modules/node/handler/node_handler.e index ac3762a..734cda8 100644 --- a/modules/node/handler/node_handler.e +++ b/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) diff --git a/modules/node/handler/node_response.e b/modules/node/handler/node_response.e index 8bc0e28..0a8115d 100644 --- a/modules/node/handler/node_response.e +++ b/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 diff --git a/modules/node/handler/node_view_response.e b/modules/node/handler/node_view_response.e index 1faf51e..70e0f34 100644 --- a/modules/node/handler/node_view_response.e +++ b/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