From 5b64aeb14d3b868ac3f65c8254786ca6002e8b47 Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Thu, 28 Feb 2019 12:42:05 +0100 Subject: [PATCH] Implement `@current` method in App.User, and replace throughout codebase where appropriate. --- .../_application_controller.coffee | 6 +--- .../_application_controller_generic.coffee | 6 ++-- .../sidebar_customer.coffee | 3 +- .../app/controllers/integrations.coffee | 4 +-- .../app/controllers/navigation.coffee | 18 +++++------- .../ticket_zoom/article_action/delete.coffee | 17 +++++------ .../ticket_zoom/sidebar_customer.coffee | 5 ++-- .../app/controllers/user_profile.coffee | 4 +-- .../widget/user_signup_check.coffee | 5 ++-- .../app/lib/app_post/task_manager.coffee | 6 +--- .../javascripts/app/models/ticket.coffee | 8 +++--- app/assets/javascripts/app/models/user.coffee | 4 +++ public/assets/tests/session.js | 28 +++++++++++++------ 13 files changed, 53 insertions(+), 61 deletions(-) diff --git a/app/assets/javascripts/app/controllers/_application_controller.coffee b/app/assets/javascripts/app/controllers/_application_controller.coffee index 81918642e..acd02c1b9 100644 --- a/app/assets/javascripts/app/controllers/_application_controller.coffee +++ b/app/assets/javascripts/app/controllers/_application_controller.coffee @@ -249,11 +249,7 @@ class App.Controller extends Spine.Controller false permissionCheck: (key) -> - userId = App.Session.get('id') - return false if !userId - user = App.User.findNative(userId) - return false if !user - user.permission(key) + App.User.current()?.permission(key) authenticateCheckRedirect: -> return true if @authenticateCheck() diff --git a/app/assets/javascripts/app/controllers/_application_controller_generic.coffee b/app/assets/javascripts/app/controllers/_application_controller_generic.coffee index 6ce648472..f5c9a09a3 100644 --- a/app/assets/javascripts/app/controllers/_application_controller_generic.coffee +++ b/app/assets/javascripts/app/controllers/_application_controller_generic.coffee @@ -439,8 +439,6 @@ class App.ControllerNavSidbar extends App.Controller if @authenticateRequired @authenticateCheckRedirect() - @user = App.User.find(App.Session.get('id')) - @render(true) @bind('ui:rerender', @@ -501,7 +499,7 @@ class App.ControllerNavSidbar extends App.Controller else match = false for permissionName in item.permission - if !match && @user.permission(permissionName) + if !match && @permissionCheck(permissionName) match = true groupsUnsorted.push item _.sortBy(groupsUnsorted, (item) -> return item.prio) @@ -520,7 +518,7 @@ class App.ControllerNavSidbar extends App.Controller else match = false for permissionName in item.permission - if !match && @user && @user.permission(permissionName) + if !match && @permissionCheck(permissionName) match = true itemsUnsorted.push item diff --git a/app/assets/javascripts/app/controllers/agent_ticket_create/sidebar_customer.coffee b/app/assets/javascripts/app/controllers/agent_ticket_create/sidebar_customer.coffee index 811e0013e..ea1171637 100644 --- a/app/assets/javascripts/app/controllers/agent_ticket_create/sidebar_customer.coffee +++ b/app/assets/javascripts/app/controllers/agent_ticket_create/sidebar_customer.coffee @@ -11,8 +11,7 @@ class SidebarCustomer extends App.Controller } if App.User.exists(@params.customer_id) customer = App.User.find(@params.customer_id) - currentUser = App.User.find(App.Session.get('id')) - if customer.isAccessibleBy(currentUser, 'change') + if customer.isAccessibleBy(App.User.current(), 'change') @item.sidebarActions.push { title: 'Edit Customer' name: 'customer-edit' diff --git a/app/assets/javascripts/app/controllers/integrations.coffee b/app/assets/javascripts/app/controllers/integrations.coffee index b47bb5bf2..996e6de51 100644 --- a/app/assets/javascripts/app/controllers/integrations.coffee +++ b/app/assets/javascripts/app/controllers/integrations.coffee @@ -33,8 +33,6 @@ class Index extends App.ControllerSubContent render: => return if @initRender && @integration - @user = App.User.find(App.Session.get('id')) - @initRender = true integrations = [] for key, value of @integrationItems @@ -44,7 +42,7 @@ class Index extends App.ControllerSubContent else match = false for permissionName in value.permission - if !match && @user.permission(permissionName) + if !match && @permissionCheck(permissionName) match = true value.key = key integrations.push value diff --git a/app/assets/javascripts/app/controllers/navigation.coffee b/app/assets/javascripts/app/controllers/navigation.coffee index 6f11e4507..512b58fd4 100644 --- a/app/assets/javascripts/app/controllers/navigation.coffee +++ b/app/assets/javascripts/app/controllers/navigation.coffee @@ -312,7 +312,7 @@ class App.Navigation extends App.ControllerWidgetPermanent @searchContainer.addClass('open') @globalSearch.search(query: @query) - filterNavbar: (values, user, parent = null) -> + filterNavbar: (values, parent = null) -> return _.filter values, (item) => if typeof item.callback is 'function' data = item.callback() || {} @@ -320,16 +320,16 @@ class App.Navigation extends App.ControllerWidgetPermanent item[key] = value if !parent? && !item.parent || item.parent is parent - return @filterNavbarPermissionOk(item, user) && + return @filterNavbarPermissionOk(item) && @filterNavbarSettingOk(item) else return false - filterNavbarPermissionOk: (item, user) -> + filterNavbarPermissionOk: (item) -> return true unless item.permission - return _.any item.permission, (permissionName) -> - return user && user.permission(permissionName) + return _.any item.permission, (permissionName) => + return @permissionCheck(permissionName) filterNavbarSettingOk: (item) -> return true unless item.setting @@ -343,15 +343,11 @@ class App.Navigation extends App.ControllerWidgetPermanent level1 = [] dropdown = {} - user = undefined - if App.Session.get('id') - user = App.User.find(App.Session.get('id')) - - level1 = @filterNavbar(navbar, user) + level1 = @filterNavbar(navbar) for item in navbar if item.parent && !dropdown[ item.parent ] - dropdown[ item.parent ] = @filterNavbar(navbar, user, item.parent) + dropdown[ item.parent ] = @filterNavbar(navbar, item.parent) for itemLevel1 in level1 if itemLevel1.target is item.parent diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee index 7436c15f6..830f10c2b 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/delete.coffee @@ -3,16 +3,13 @@ class Delete return actions if ui.permissionCheck('ticket.customer') if article.type.name is 'note' - user = undefined - if App.Session.get('id') == article.created_by_id - user = App.User.find(App.Session.get('id')) - if user.permission('ticket.agent') - actions.push { - name: 'delete' - type: 'delete' - icon: 'trash' - href: '#' - } + if App.User.current()?.id == article.created_by_id && ui.permissionCheck('ticket.agent') + actions.push { + name: 'delete' + type: 'delete' + icon: 'trash' + href: '#' + } actions diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_customer.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_customer.coffee index 239b36105..6189ea78a 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_customer.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/sidebar_customer.coffee @@ -15,10 +15,11 @@ class SidebarCustomer extends App.Controller ] } return @item if @ticket && @ticket.customer_id == 1 - currentUser = App.User.find(App.Session.get('id')) + + # prevent exceptions if customer model is no available if @ticket.customer_id && App.User.exists(@ticket.customer_id) customer = App.User.find(@ticket.customer_id) - if customer.isAccessibleBy(currentUser, 'change') + if customer?.isAccessibleBy(App.User.current(), 'change') @item.sidebarActions.push { title: 'Edit Customer' name: 'customer-edit' diff --git a/app/assets/javascripts/app/controllers/user_profile.coffee b/app/assets/javascripts/app/controllers/user_profile.coffee index 3a2614c72..7653d6a2a 100644 --- a/app/assets/javascripts/app/controllers/user_profile.coffee +++ b/app/assets/javascripts/app/controllers/user_profile.coffee @@ -107,8 +107,6 @@ class ActionRow extends App.ObserverActionRow @navigate("ticket/create/customer/#{user.id}") actions: (user) => - currentUser = App.User.find(App.Session.get('id')) - actions = [ { name: 'history' @@ -122,7 +120,7 @@ class ActionRow extends App.ObserverActionRow } ] - if user.isAccessibleBy(currentUser, 'change') + if user.isAccessibleBy(App.User.current(), 'change') actions.unshift { name: 'edit' title: 'Edit' diff --git a/app/assets/javascripts/app/controllers/widget/user_signup_check.coffee b/app/assets/javascripts/app/controllers/widget/user_signup_check.coffee index d1f69529c..e42e3ba6d 100644 --- a/app/assets/javascripts/app/controllers/widget/user_signup_check.coffee +++ b/app/assets/javascripts/app/controllers/widget/user_signup_check.coffee @@ -12,9 +12,8 @@ class Widget extends App.Controller @verifyLater(user.id) 'user_signup_verify' ) - currentUserId = App.Session.get('id') - return if !currentUserId - @verifyLater(currentUserId) + user = App.User.current() + @verifyLater(user.id) if user? verifyLater: (userId) => delay = => diff --git a/app/assets/javascripts/app/lib/app_post/task_manager.coffee b/app/assets/javascripts/app/lib/app_post/task_manager.coffee index 6dcd1c987..152ec2db8 100644 --- a/app/assets/javascripts/app/lib/app_post/task_manager.coffee +++ b/app/assets/javascripts/app/lib/app_post/task_manager.coffee @@ -651,15 +651,11 @@ class _taskManagerSingleton extends App.Controller App.Event.trigger 'taskbar:init' # initial load of permanent tasks - user_id = App.Session.get('id') - user = undefined - if user_id - user = App.User.find(user_id) permanentTask = App.Config.get('permanentTask') taskCount = 0 if permanentTask for key, config of permanentTask - if !config.permission || (user && user.permission(config.permission)) + if !config.permission || @permissionCheck(config.permission) taskCount += 1 do (key, config, taskCount) => App.Delay.set( diff --git a/app/assets/javascripts/app/models/ticket.coffee b/app/assets/javascripts/app/models/ticket.coffee index 0b72415e3..6cfc91de0 100644 --- a/app/assets/javascripts/app/models/ticket.coffee +++ b/app/assets/javascripts/app/models/ticket.coffee @@ -247,10 +247,10 @@ class App.Ticket extends App.Model result editable: (permission = 'change') -> - user_id = App.Session.get('id') - return true if user_id is @customer_id - return false if !App.User.exists(user_id) - group_ids = App.User.find(user_id).allGroupIds(permission) + user = App.User.current() + return false if !user? + return true if user.id is @customer_id + group_ids = user.allGroupIds(permission) for local_group_id in group_ids if local_group_id.toString() is @group_id.toString() return true diff --git a/app/assets/javascripts/app/models/user.coffee b/app/assets/javascripts/app/models/user.coffee index 5ce2b6d85..31578811d 100644 --- a/app/assets/javascripts/app/models/user.coffee +++ b/app/assets/javascripts/app/models/user.coffee @@ -346,3 +346,7 @@ class App.User extends App.Model return false if @organization_id is null return false if requester.organization_id is null @organization_id == requester.organization_id + + # Do NOT modify the return value of this method! + # It is a direct reference to a value in the App.User.irecords object. + @current: App.Session.get diff --git a/public/assets/tests/session.js b/public/assets/tests/session.js index e31eff281..0e166eefd 100644 --- a/public/assets/tests/session.js +++ b/public/assets/tests/session.js @@ -2,7 +2,8 @@ window.onload = function() { test('test current user behaviour by updating session user via assets', function() { - // load user + // Wenn App.User updated through asset and set as session user + // expect App.Session.get with new values App.User.refresh([{ "login": "hh@example.com", "firstname": "Harald", @@ -18,11 +19,7 @@ test('test current user behaviour by updating session user via assets', function "asdf": "", "id": 6 }]); - - // set session user App.Session.set(6) - - // verify attributes equal(App.Session.get('id'), 6) equal(App.Session.get('login'), 'hh@example.com') equal(App.Session.get('vip'), false) @@ -32,7 +29,8 @@ test('test current user behaviour by updating session user via assets', function equal(App.Session.get().custom_key, undefined) equal(App.Session.get().not_existing, undefined) - // update session user via assets + // Wenn App.User updated through asset + // expect App.Session.get with new values App.User.refresh([{ "login": "hh_new@example.com", "firstname": "Harald", @@ -48,8 +46,6 @@ test('test current user behaviour by updating session user via assets', function "asdf": "", "id": 6 }]); - - // verify attributes equal(App.Session.get('id'), 6) equal(App.Session.get('login'), 'hh_new@example.com') equal(App.Session.get('vip'), false) @@ -59,7 +55,8 @@ test('test current user behaviour by updating session user via assets', function equal(App.Session.get().custom_key, undefined) equal(App.Session.get().not_existing, undefined) - // clear session + // Wenn App.Session is reseted to inital + // expect undefined for all App.Session.init() equal(App.Session.get(), undefined) equal(App.Session.get('id'), undefined) @@ -67,6 +64,19 @@ test('test current user behaviour by updating session user via assets', function equal(App.Session.get('vip'), undefined) equal(App.Session.get('custom_key'), undefined) + // When App.Session is set and set to undefined or null, + // expect @current() to return null + App.Session.set(6) + App.Session.set(undefined) + equal(App.User.current(), null, 'with no active session') + App.Session.set(null) + equal(App.User.current(), null, 'with no active session') + + // When App.Session is set with an invalid (not existing) user ID, + // expect @current() to return null + App.Session.set(100) + equal(App.User.current(), null, 'with invalid session user ID') + }); } \ No newline at end of file