From 1ef4025caa44b0510e001665450a6fe4f071f50e Mon Sep 17 00:00:00 2001 From: jvelilla Date: Thu, 30 Jul 2015 11:06:03 -0300 Subject: [PATCH] Update user storage, Clean code --- .../handler/role/cms_role_form_response.e | 18 +- .../{user => role}/cms_role_view_response.e | 0 .../handler/user/cms_user_form_response.e | 157 +----------------- src/persistence/user/cms_user_storage_i.e | 42 +++++ src/persistence/user/cms_user_storage_null.e | 30 ++++ src/persistence/user/cms_user_storage_sql_i.e | 96 +++++++++-- 6 files changed, 161 insertions(+), 182 deletions(-) rename modules/admin/handler/{user => role}/cms_role_view_response.e (100%) diff --git a/modules/admin/handler/role/cms_role_form_response.e b/modules/admin/handler/role/cms_role_form_response.e index 18ad15b..36c296a 100644 --- a/modules/admin/handler/role/cms_role_form_response.e +++ b/modules/admin/handler/role/cms_role_form_response.e @@ -320,6 +320,7 @@ feature -- Form fs: WSF_FORM_FIELD_SET cb: WSF_FORM_CHECKBOX_INPUT ts: WSF_FORM_SUBMIT_INPUT + tb: WSF_FORM_BUTTON_INPUT do if attached a_role as l_role then create fs.make @@ -344,9 +345,11 @@ feature -- Form fs.extend (cb) end end - fs.extend_html_text ("
") create ti.make ("cms_perm[]") fs.extend (ti) + fs.extend_html_text ("
") + fs.extend_html_text ("") + a_form.extend (fs) add_javascript_content (script_add_remove_items) @@ -464,26 +467,19 @@ feature -- Generation end end - script_add_remove_items: STRING = "[ $(document).ready(function() { - var max_fields = 10; //maximum input boxes allowed var wrapper = $(".input_fields_wrap"); //Fields wrapper var add_button = $(".add_field_button"); //Add button ID - - var x = 1; //initlal text box count + $(add_button).click(function(e){ //on add input button click e.preventDefault(); - if(x < max_fields){ //max input box allowed - x++; //text box increment - $(wrapper).append('
Remove
'); //add input box - } + $(wrapper).append('
Remove
'); //add input box }); - + $(wrapper).on("click",".remove_field", function(e){ //user click on remove text e.preventDefault(); $(this).parent('div').remove(); x--; }) }); ]" - end diff --git a/modules/admin/handler/user/cms_role_view_response.e b/modules/admin/handler/role/cms_role_view_response.e similarity index 100% rename from modules/admin/handler/user/cms_role_view_response.e rename to modules/admin/handler/role/cms_role_view_response.e diff --git a/modules/admin/handler/user/cms_user_form_response.e b/modules/admin/handler/user/cms_user_form_response.e index 8ad6dfd..2c79de6 100644 --- a/modules/admin/handler/user/cms_user_form_response.e +++ b/modules/admin/handler/user/cms_user_form_response.e @@ -65,8 +65,7 @@ feature -- Process uid > 0 and then attached user_api.user_by_id (uid) as l_user then - fixme ("refactor: process_edit, process_create process edit") - if + if request.path_info.ends_with_general ("/edit") then edit_form (l_user) @@ -520,160 +519,6 @@ feature -- Generation end end - handle_admin_hack (a_api: CMS_API; req: WSF_REQUEST; res: WSF_RESPONSE) - local - s: STRING - r: CMS_RESPONSE - f: CMS_FORM - t: WSF_FORM_TEXT_INPUT - fe: WSF_FORM_EMAIL_INPUT - fs: WSF_FORM_FIELD_SET - f_submit: WSF_FORM_SUBMIT_INPUT - do - create {GENERIC_VIEW_CMS_RESPONSE} r.make (req, res, a_api) - - create f.make (req.percent_encoded_path_info, {CMS_ADMIN_MODULE}.name + "_hack_form") - create fs.make - fs.set_legend ("Create new user without password:") - create t.make_with_text ("username", "") - t.set_label ("User name") - t.enable_required - t.set_placeholder ("username") - fs.extend (t) - - create fe.make_with_text ("email", "") - fe.set_label ("Email") - fe.set_placeholder ("valid email") - fs.extend (fe) - create f_submit.make_with_text ("op", "Create user") - fs.extend (f_submit) - create f_submit.make_with_text ("op", "Update user") - fs.extend (f_submit) - f.extend (fs) - - if req.is_post_request_method then - create s.make_empty - f.validation_actions.extend (agent (fd: WSF_FORM_DATA; ia_api: CMS_API) - do - if attached fd.string_item ("op") as f_op then - if f_op.is_case_insensitive_equal_general ("Create user") then - if attached fd.string_item ("username") as l_username then - if attached ia_api.user_api.user_by_name (l_username) then - fd.report_invalid_field ("username", "Username already taken!") - end - else - fd.report_invalid_field ("username", "missing username") - end - if attached fd.string_item ("email") as l_email then - if attached ia_api.user_api.user_by_email (l_email) then - fd.report_invalid_field ("email", "Email address already associated with an existing account!") - end - else - fd.report_invalid_field ("email", "missing email address") - end - elseif f_op.is_case_insensitive_equal_general ("Update user") then - if attached fd.string_item ("username") as l_username then - if ia_api.user_api.user_by_name (l_username) = Void then - fd.report_invalid_field ("username", "Username does not exist!") - end - else - fd.report_invalid_field ("username", "missing username") - end - end - end - end(?, a_api) - ) - f.submit_actions.extend (agent (fd: WSF_FORM_DATA; ia_api: CMS_API; a_output: STRING) - local - u: CMS_USER - l_roles: detachable LIST [CMS_USER_ROLE] - l_trusted_user_role: detachable CMS_USER_ROLE - do - if attached fd.string_item ("op") as f_op then - if f_op.is_case_insensitive_equal_general ("Create user") then - if - attached fd.string_item ("username") as l_username and then - attached fd.string_item ("email") as l_email and then - l_email.is_valid_as_string_8 - then - create u.make (l_username) - u.set_email (l_email.as_string_8) - u.set_password (new_random_password (u)) - ia_api.user_api.new_user (u) - if ia_api.user_api.has_error then - - end - a_output.append ("
  • New user ["+ html_encoded (l_username) +"] created.
    • ") - else - fd.report_invalid_field ("username", "Missing username!") - fd.report_invalid_field ("email", "Missing email address!") - end - elseif f_op.is_case_insensitive_equal_general ("Update user") then - if - attached fd.string_item ("username") as l_username and then - attached ia_api.user_api.user_by_name (l_username) as l_user - then - l_trusted_user_role := ia_api.user_api.user_role_by_name ("trusted") - if l_trusted_user_role = Void then - create l_trusted_user_role.make ("trusted") - ia_api.user_api.save_user_role (l_trusted_user_role) - end - - l_trusted_user_role.add_permission ("admin wdocs") - l_trusted_user_role.add_permission ("edit wdocs page") - l_trusted_user_role.add_permission ("create wdocs page") - l_trusted_user_role.add_permission ("delete wdocs page") - l_trusted_user_role.add_permission ("edit any wdocs page") - l_trusted_user_role.add_permission ("delete any wdocs page") - l_trusted_user_role.add_permission ("clear wdocs cache") - - l_trusted_user_role.add_permission ("create page") - l_trusted_user_role.add_permission ("edit any page") - l_trusted_user_role.add_permission ("delete any page") - l_trusted_user_role.add_permission ("create blog") - l_trusted_user_role.add_permission ("edit any blog") - l_trusted_user_role.add_permission ("delete any blog") - - l_trusted_user_role.add_permission ("edit any node") - l_trusted_user_role.add_permission ("delete any node") - - - ia_api.user_api.save_user_role (l_trusted_user_role) - l_trusted_user_role := ia_api.user_api.user_role_by_name ("trusted") - if l_trusted_user_role /= Void then - u := l_user - ia_api.user_api.update_user (u) - l_roles := u.roles - if l_roles = Void then - create {ARRAYED_LIST [CMS_USER_ROLE]} l_roles.make (1) - end - l_roles.force (l_trusted_user_role) - u.set_roles (l_roles) - - ia_api.user_api.update_user (u) - a_output.append ("
  • User ["+ html_encoded (l_username) +"] updated.
    • ") - else - a_output.append ("
  • User ["+ html_encoded (l_username) +"] NOT updated! [ERROR].
    • ") - end - else - fd.report_invalid_field ("username", "User does not exist!") - end - end - end - end(?, a_api, s) - ) - - f.process (r) - f.append_to_html (create {CMS_TO_WSF_THEME}.make (r, r.theme), s) - r.set_main_content (s) - elseif req.is_get_head_request_method then - create s.make_empty - f.append_to_html (create {CMS_TO_WSF_THEME}.make (r, r.theme), s) - r.set_main_content (s) - end - r.execute - end - new_random_password (u: CMS_USER): STRING -- Generate a new token activation token local diff --git a/src/persistence/user/cms_user_storage_i.e b/src/persistence/user/cms_user_storage_i.e index 44ab40b..6c77756 100644 --- a/src/persistence/user/cms_user_storage_i.e +++ b/src/persistence/user/cms_user_storage_i.e @@ -75,6 +75,16 @@ feature -- Access deferred end + users_count: INTEGER + -- Number of users + deferred + end + + recent_users (a_lower: INTEGER; a_count: INTEGER): LIST [CMS_USER] + -- List of recent `a_count' users with an offset of `lower'. + deferred + end + feature -- Change: user save_user (a_user: CMS_USER) @@ -101,6 +111,14 @@ feature -- Change: user deferred end + delete_user (a_user: CMS_USER) + -- Delete user `a_user'. + require + has_id: a_user.has_id + deferred + end + + feature -- Access: roles and permissions -- user_has_permission (u: detachable CMS_USER; s: detachable READABLE_STRING_8): BOOLEAN @@ -160,6 +178,30 @@ feature -- Change: roles and permissions deferred end + unassign_role_from_user (a_role: CMS_USER_ROLE; a_user: CMS_USER) + -- Unassign user_role to user + require + a_user.has_id + a_role.has_id + deferred + end + + assign_role_to_user (a_role: CMS_USER_ROLE; a_user: CMS_USER) + -- Assign user_role to user + require + a_user.has_id + a_role.has_id + deferred + end + + + delete_role (a_role: CMS_USER_ROLE) + -- Remove role `a_role'. + require + a_role.has_id + deferred + end + feature -- Change: User activation save_activation (a_token: READABLE_STRING_32; a_id: INTEGER_64) diff --git a/src/persistence/user/cms_user_storage_null.e b/src/persistence/user/cms_user_storage_null.e index cc122ad..f078f45 100644 --- a/src/persistence/user/cms_user_storage_null.e +++ b/src/persistence/user/cms_user_storage_null.e @@ -46,6 +46,17 @@ feature -- Access: user do end + users_count: INTEGER + -- + do + end + + recent_users (a_lower: INTEGER; a_count: INTEGER): LIST [CMS_USER] + -- + do + create {ARRAYED_LIST[CMS_USER]} Result.make (0) + end + feature -- Change: user new_user (a_user: CMS_USER) @@ -60,6 +71,12 @@ feature -- Change: user end + delete_user (a_user: CMS_USER) + -- Delete user `a_user'. + do + end + + feature -- Access: roles and permissions user_role_by_id (a_id: like {CMS_USER_ROLE}.id): detachable CMS_USER_ROLE @@ -88,6 +105,19 @@ feature -- Change: roles and permissions do end + unassign_role_from_user (a_user_role: CMS_USER_ROLE; a_user: CMS_USER) + do + end + + assign_role_to_user (a_user_role: CMS_USER_ROLE; a_user: CMS_USER) + do + end + + delete_role (a_role: CMS_USER_ROLE) + -- + do + end + feature -- Change: User activation save_activation (a_token: READABLE_STRING_32; a_id: INTEGER_64) diff --git a/src/persistence/user/cms_user_storage_sql_i.e b/src/persistence/user/cms_user_storage_sql_i.e index 803b83f..08a63fd 100644 --- a/src/persistence/user/cms_user_storage_sql_i.e +++ b/src/persistence/user/cms_user_storage_sql_i.e @@ -20,10 +20,10 @@ feature -- Access: user has_user: BOOLEAN -- Has any user? do - Result := user_count > 0 + Result := users_count > 0 end - user_count: INTEGER + users_count: INTEGER -- Number of items users. do error_handler.reset @@ -163,6 +163,31 @@ feature -- Access: user end + recent_users (a_lower: INTEGER; a_count: INTEGER): LIST [CMS_USER] + -- + local + l_parameters: STRING_TABLE [detachable ANY] + do + create {ARRAYED_LIST [CMS_USER]} Result.make (0) + + error_handler.reset + write_information_log (generator + ".recent_users") + + from + create l_parameters.make (2) + l_parameters.put (a_count, "rows") + l_parameters.put (a_lower, "offset") + sql_query (sql_select_recent_users, l_parameters) + sql_start + until + sql_after + loop + if attached fetch_user as l_user then + Result.force (l_user) + end + sql_forth + end + end feature -- Change: user new_user (a_user: CMS_USER) @@ -255,6 +280,20 @@ feature -- Change: user end end + delete_user (a_user: CMS_USER) + -- Delete user `a_user'. + local + l_parameters: STRING_TABLE [detachable ANY] + do + error_handler.reset + sql_begin_transaction + write_information_log (generator + ".delete_user") + create l_parameters.make (1) + l_parameters.put (a_user.id, "uid") + sql_change (sql_delete_user, l_parameters) + sql_commit_transaction + end + update_user_roles (a_user: CMS_USER) -- Update roles of `a_user' require @@ -311,9 +350,6 @@ feature -- Change: user end assign_role_to_user (a_role: CMS_USER_ROLE; a_user: CMS_USER) - require - a_user.has_id - a_role.has_id local l_parameters: STRING_TABLE [detachable ANY] do @@ -324,9 +360,6 @@ feature -- Change: user end unassign_role_from_user (a_role: CMS_USER_ROLE; a_user: CMS_USER) - require - a_user.has_id - a_role.has_id local l_parameters: STRING_TABLE [detachable ANY] do @@ -502,12 +535,20 @@ feature -- Change: roles and permissions -- FIXME: check if this is non set permissions,or none ... if l_existing_role /= Void then l_permissions := l_existing_role.permissions - fill_user_role (l_existing_role) +-- fill_user_role (l_existing_role) end if l_permissions = Void or else l_permissions.is_empty then l_permissions := role_permissions_by_id (a_user_role.id) end + a_user_role.permissions.compare_objects + across l_permissions as ic + loop + if not a_user_role.permissions.has (ic.item) then + unset_permission_for_role_id (ic.item, a_user_role.id) + end + end + across a_user_role.permissions as ic loop @@ -531,12 +572,12 @@ feature -- Change: roles and permissions set_permission_for_role_id (p, a_user_role.id) end end - -- Remove other - across - l_permissions as ic - loop - unset_permission_for_role_id (ic.item, a_user_role.id) - end +-- -- Remove other +-- across +-- l_permissions as ic +-- loop +-- unset_permission_for_role_id (ic.item, a_user_role.id) +-- end end else create l_parameters.make (1) @@ -593,6 +634,22 @@ feature -- Change: roles and permissions end + delete_role (a_role: CMS_USER_ROLE) + -- + local + l_parameters: STRING_TABLE [detachable ANY] + do + error_handler.reset + sql_begin_transaction + write_information_log (generator + ".delete_role") + create l_parameters.make (1) + l_parameters.put (a_role.id, "rid") + sql_change (sql_delete_role_permissions_by_role_id, l_parameters) + sql_change (sql_delete_role_by_id, l_parameters) + sql_commit_transaction + end + + feature -- Access: User activation activation_elapsed_time (a_token: READABLE_STRING_32): INTEGER_32 @@ -800,6 +857,9 @@ feature {NONE} -- Sql Queries: USER Select_user_by_name: STRING = "SELECT * FROM users WHERE name =:name;" -- Retrieve user by name if exists. + Sql_select_recent_users: STRING = "SELECT * FROM users ORDER BY uid DESC, created DESC LIMIT :rows OFFSET :offset ;" + -- Retrieve recent users + Select_user_by_email: STRING = "SELECT * FROM users WHERE email =:email;" -- Retrieve user by email if exists. @@ -812,6 +872,8 @@ feature {NONE} -- Sql Queries: USER sql_update_user: STRING = "UPDATE users SET name=:name, password=:password, salt=:salt, email=:email, status=:status WHERE uid=:uid;" -- SQL update to update an existing user. + sql_delete_user: STRING = "DELETE FROM users WHERE uid=:uid;" + feature {NONE} -- Sql Queries: USER ROLE sql_last_insert_user_role_id: STRING = "SELECT MAX(rid) FROM roles;" @@ -849,6 +911,10 @@ feature {NONE} -- Sql Queries: USER ROLE select_role_permissions_by_role_id: STRING = "SELECT permission, module FROM role_permissions WHERE rid=:rid;" -- User role permissions for role id :rid; + sql_delete_role_permissions_by_role_id: STRING = "DELETE FROM role_permissions WHERE rid=:rid;" + + sql_delete_role_by_id: STRING = "DELETE FROM roles WHERE rid=:rid;" + feature {NONE} -- Sql Queries: USER ACTIVATION sql_insert_activation: STRING = "INSERT INTO users_activations (token, uid, created) VALUES (:token, :uid, :utc_date);"