From 3fa29340b2f0a6a1f577edde1f32bf2f8fba61ce Mon Sep 17 00:00:00 2001 From: Jocelyn Fiat Date: Tue, 12 May 2015 20:01:14 +0200 Subject: [PATCH] Updated code to follow review comments. --- .../sqlite/src/cms_storage_sqlite_builder.e | 67 ++++++++++--------- modules/node/cms_node_api.e | 51 ++++---------- modules/node/content/cms_node.e | 16 ++--- modules/node/handler/node_form_response.e | 2 +- modules/node/handler/node_handler.e | 2 +- .../node/persistence/cms_node_storage_sql.e | 10 ++- 6 files changed, 58 insertions(+), 90 deletions(-) diff --git a/library/persistence/sqlite/src/cms_storage_sqlite_builder.e b/library/persistence/sqlite/src/cms_storage_sqlite_builder.e index c5ead60..1d7ae35 100644 --- a/library/persistence/sqlite/src/cms_storage_sqlite_builder.e +++ b/library/persistence/sqlite/src/cms_storage_sqlite_builder.e @@ -47,61 +47,62 @@ feature -- Factory initialize (a_setup: CMS_SETUP; a_storage: CMS_STORAGE_STORE_SQL) local u: CMS_USER - r: CMS_USER_ROLE - l: LIST[CMS_USER_ROLE] + l_anonymous_role, l_authenticated_role, r: CMS_USER_ROLE + l_roles: LIST [CMS_USER_ROLE] do - -- Schema + --| Schema a_storage.sql_execute_file_script (a_setup.environment.path.extended ("scripts").extended ("core.sql")) + --| Roles + create l_anonymous_role.make ("anonymous") + a_storage.save_user_role (l_anonymous_role) - -- Roles - create r.make ("anonymous") - a_storage.save_user_role (r) - create r.make ("authenticated") - r.add_permission ("create page") - r.add_permission ("edit own page") - r.add_permission ("delete own page") - a_storage.save_user_role (r) + create l_authenticated_role.make ("authenticated") + a_storage.save_user_role (l_authenticated_role) - - create {ARRAYED_LIST[CMS_USER_ROLE]} l.make (1) - l.force (r) - - -- Users + --| Users create u.make ("admin") u.set_password ("istrator#") u.set_email (a_setup.site_email) a_storage.new_user (u) - create u.make ("auth") - u.set_password ("enticated#") - u.set_email (a_setup.site_email) - u.set_roles (l) - a_storage.new_user (u) + --| Node + -- FIXME: move that initialization to node module + l_anonymous_role.add_permission ("view any page") + a_storage.save_user_role (l_anonymous_role) - create u.make ("test") - u.set_password ("test#") - u.set_email (a_setup.site_email) - u.set_roles (l) - a_storage.new_user (u) + l_authenticated_role.add_permission ("create page") + l_authenticated_role.add_permission ("view any page") + l_authenticated_role.add_permission ("edit own page") + l_authenticated_role.add_permission ("delete own page") + a_storage.save_user_role (l_authenticated_role) + + + --| For testing purpose, to be removed later. -- Roles, view role for testing. create r.make ("view") r.add_permission ("view page") a_storage.save_user_role (r) - create {ARRAYED_LIST[CMS_USER_ROLE]} l.make (1) - l.force (r) + create {ARRAYED_LIST [CMS_USER_ROLE]} l_roles.make (1) + l_roles.force (r) + + create u.make ("auth") + u.set_password ("enticated#") + u.set_email (a_setup.site_email) + a_storage.new_user (u) + + create u.make ("test") + u.set_password ("test#") + u.set_email (a_setup.site_email) + a_storage.new_user (u) create u.make ("view") u.set_password ("only#") u.set_email (a_setup.site_email) - u.set_roles (l) + u.set_roles (l_roles) a_storage.new_user (u) - - -- Test custom value - a_storage.set_custom_value ("abc", "123", "test") - a_storage.set_custom_value ("abc", "OK", "test") end end diff --git a/modules/node/cms_node_api.e b/modules/node/cms_node_api.e index af5946c..9922722 100644 --- a/modules/node/cms_node_api.e +++ b/modules/node/cms_node_api.e @@ -249,22 +249,25 @@ feature -- Access: Node end end - user_is_node_owner (u: READABLE_STRING_32; nid: INTEGER_64): BOOLEAN + is_author_of_node (u: CMS_USER; a_node: CMS_NODE): BOOLEAN -- Is the user `u' owner of the node `n'. do - if attached {CMS_USER} node_storage.node_author (nid) as l_user then - Result := l_user.name.is_case_insensitive_equal (u) + if attached node_storage.node_author (a_node.id) as l_author then + Result := u.same_as (l_author) end end feature -- Permission Scope: Node - permission_scope (u: detachable READABLE_STRING_32; nid: INTEGER_64): STRING - -- Result 'own' if the user `u' is the owner of the node `nid', in other case + permission_scope (u: detachable CMS_USER; a_node: CMS_NODE): STRING + -- Result 'own' if the user `u' is the owner of the node `a_node', in other case -- `any'. do + -- FIXME: check if this is ok, since a role may have "any" permission enabled, and "own" disabled, + -- in this case, we should check both permissions + -- obviously such case should be rare, and look like bad configured permissions, but this may occurs. Result := "any" - if attached u as l_u and then user_is_node_owner (l_u, nid) then + if u /= Void and then is_author_of_node (u, a_node) then Result := "own" end end @@ -302,41 +305,13 @@ feature -- Change: Node feature -- Node status - Not_published: INTEGER = 1 + Not_published: INTEGER = 0 -- The node is not published. - Published: INTEGER = 2 + Published: INTEGER = 1 -- The node is published. - Trashed: INTEGER = 3 - -- The node is trashed (soft delete), ready to be deleted (physical). - --- update_node_title (a_user_id: like {CMS_USER}.id; a_node_id: like {CMS_NODE}.id; a_title: READABLE_STRING_32) --- -- Update node title, with user identified by `a_id', with node id `a_node_id' and a new title `a_title'. --- do --- debug ("refactor_fixme") --- fixme ("Check preconditions") --- end --- node_storage.update_node_title (a_user_id, a_node_id, a_title) --- end - --- update_node_summary (a_user_id: like {CMS_USER}.id; a_node_id: like {CMS_NODE}.id; a_summary: READABLE_STRING_32) --- -- Update node summary, with user identified by `a_user_id', with node id `a_node_id' and a new summary `a_summary'. --- do --- debug ("refactor_fixme") --- fixme ("Check preconditions") --- end --- node_storage.update_node_summary (a_user_id, a_node_id, a_summary) --- end - --- update_node_content (a_user_id: like {CMS_USER}.id; a_node_id: like {CMS_NODE}.id; a_content: READABLE_STRING_32) --- -- Update node content, with user identified by `a_user_id', with node id `a_node_id' and a new content `a_content'. --- do --- debug ("refactor_fixme") --- fixme ("Check preconditions") --- end --- node_storage.update_node_content (a_user_id, a_node_id, a_content) --- end - + Trashed: INTEGER = -1 + -- The node is trashed (soft delete), ready to be deleted/destroyed from storage. end diff --git a/modules/node/content/cms_node.e b/modules/node/content/cms_node.e index 559c32f..2bd437d 100644 --- a/modules/node/content/cms_node.e +++ b/modules/node/content/cms_node.e @@ -13,10 +13,6 @@ inherit REFACTORING_HELPER ---create --- make, --- make_empty - feature{NONE} -- Initialization make_empty @@ -36,10 +32,6 @@ feature{NONE} -- Initialization set_modification_date (l_time) set_publication_date (l_time) mark_not_published - - debug ("refactor_fixme") - fixme ("Remove default harcoded format") - end ensure title_set: title = a_title end @@ -82,7 +74,9 @@ feature -- Access status: INTEGER -- Associated status for the current node. - -- [{0,Not_Published}, {1, Published}, {2, Trash}] + -- default: {CMS_NODE_API}.Not_Published} + -- {CMS_NODE_API}.Published + -- {CMS_NODE_API}.Trashed feature -- Access @@ -233,7 +227,7 @@ feature -- Element change status_published: status = {CMS_NODE_API}.published end - mark_trash + mark_trashed -- Set status to published do set_status ({CMS_NODE_API}.trashed) @@ -242,7 +236,7 @@ feature -- Element change end -feature {CMS_NODE_STORAGE_I} -- Selective Export +feature {CMS_NODE_STORAGE_I} -- Access: status change. set_status (a_status: like status) -- Assign `status' with `a_status'. diff --git a/modules/node/handler/node_form_response.e b/modules/node/handler/node_form_response.e index 7ce134b..8ade0b6 100644 --- a/modules/node/handler/node_form_response.e +++ b/modules/node/handler/node_form_response.e @@ -48,7 +48,7 @@ feature -- Execution attached node_api.node (nid) as l_node then if attached node_api.node_type_for (l_node) as l_type then - if has_permission ("edit " + node_api.permission_scope (current_user_name (request), nid) + " " + l_type.name) then + if has_permission ("edit " + node_api.permission_scope (current_user (request), l_node) + " " + l_type.name) then f := edit_form (l_node, url (request.path_info, Void), "edit-" + l_type.name, l_type) if request.is_post_request_method then f.validation_actions.extend (agent edit_form_validate (?, b)) diff --git a/modules/node/handler/node_handler.e b/modules/node/handler/node_handler.e index 2cb013a..0e6d8b9 100644 --- a/modules/node/handler/node_handler.e +++ b/modules/node/handler/node_handler.e @@ -150,7 +150,7 @@ feature -- HTTP Methods l_id.is_integer and then attached node_api.node (l_id.integer_value) as l_node then - if api.user_has_permission (l_user, "delete " + node_api.permission_scope (current_user_name (req), l_id.integer_value) + " " + l_node.content_type) then + if api.user_has_permission (l_user, "delete " + node_api.permission_scope (l_user, l_node) + " " + l_node.content_type) then node_api.delete_node (l_node) res.send (create {CMS_REDIRECTION_RESPONSE_MESSAGE}.make (req.absolute_script_url (""))) else diff --git a/modules/node/persistence/cms_node_storage_sql.e b/modules/node/persistence/cms_node_storage_sql.e index 1d30306..ebbf4a8 100644 --- a/modules/node/persistence/cms_node_storage_sql.e +++ b/modules/node/persistence/cms_node_storage_sql.e @@ -265,15 +265,13 @@ feature -- Helpers feature {NONE} -- Queries - sql_select_nodes_count: STRING = "SELECT count(*) FROM Nodes WHERE status != 3;" + sql_select_nodes_count: STRING = "SELECT count(*) FROM Nodes WHERE status != -1 ;" -- Nodes count (Published and not Published) - -- {CMS_NODE_API}.not_published - -- TODO: add queries to retrieve published_nodes_count, no_published_nodes_count. etc + --| note: {CMS_NODE_API}.trashed = -1 - - sql_select_nodes: STRING = "SELECT * FROM Nodes WHERE status != 3;" + sql_select_nodes: STRING = "SELECT * FROM Nodes WHERE status != -1 ;" -- SQL Query to retrieve all nodes. - -- {CMS_NODE_API}.not_published + --| note: {CMS_NODE_API}.trashed = -1 sql_select_node_by_id: STRING = "SELECT nid, revision, type, title, summary, content, format, author, publish, created, changed, status FROM Nodes WHERE nid =:nid ORDER BY revision desc, publish desc LIMIT 1;"