Skip to content

Commit

Permalink
Improve responsiveness of app when platinum license changes
Browse files Browse the repository at this point in the history
Previously, routes/apps were going off the static data passed from the server which was only initialized once on page load. hasPlatinumLicense however changes dynamically and in real-time, removing the need for a hard page refresh.

I could have replaced all `canManageMetaEngine` flags with `isPlatinum && canManageEngines`, but I thought baking license checks into the main ability would be more scalable and potentially open the way to other license-based flags also being dynamic.
  • Loading branch information
cee-chen committed Jun 21, 2021
1 parent 27eadce commit f90c3e8
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
*/

import { DEFAULT_INITIAL_APP_DATA } from '../../../common/__mocks__';
import { LogicMounter } from '../__mocks__/kea_logic';
import { LogicMounter } from '../__mocks__/kea_logic/logic_mounter.test_helper';

jest.mock('../shared/licensing', () => ({
LicensingLogic: { selectors: { hasPlatinumLicense: () => false } },
}));

import { AppLogic } from './app_logic';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { kea, MakeLogicType } from 'kea';

import { InitialAppData } from '../../../common/types';

import { LicensingLogic } from '../shared/licensing';

import { ConfiguredLimits, Account, Role } from './types';

import { getRoleAbilities } from './utils/role';
Expand Down Expand Up @@ -43,8 +45,8 @@ export const AppLogic = kea<MakeLogicType<AppValues, AppActions, Required<Initia
}),
selectors: {
myRole: [
(selectors) => [selectors.account],
({ role }) => (role ? getRoleAbilities(role) : {}),
(selectors) => [selectors.account, LicensingLogic.selectors.hasPlatinumLicense],
({ role }, hasPlatinumLicense) => (role ? getRoleAbilities(role, hasPlatinumLicense) : {}),
],
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('EnginesOverview', () => {
metaEnginesLoading: false,
hasPlatinumLicense: false,
// AppLogic
myRole: { canManageEngines: false },
myRole: { canManageEngines: false, canManageMetaEngines: false },
// MetaEnginesTableLogic
expandedSourceEngines: {},
conflictingEnginesSets: {},
Expand Down Expand Up @@ -124,7 +124,7 @@ describe('EnginesOverview', () => {
setMockValues({
...valuesWithEngines,
hasPlatinumLicense: true,
myRole: { canManageEngines: true },
myRole: { canManageMetaEngines: true },
});
const wrapper = shallow(<EnginesOverview />);

Expand All @@ -135,7 +135,7 @@ describe('EnginesOverview', () => {
setMockValues({
...valuesWithEngines,
hasPlatinumLicense: true,
myRole: { canManageEngines: false },
myRole: { canManageMetaEngines: false },
});
const wrapper = shallow(<EnginesOverview />);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { EnginesLogic } from './engines_logic';
export const EnginesOverview: React.FC = () => {
const { hasPlatinumLicense } = useValues(LicensingLogic);
const {
myRole: { canManageEngines },
myRole: { canManageEngines, canManageMetaEngines },
} = useValues(AppLogic);

const {
Expand Down Expand Up @@ -111,7 +111,7 @@ export const EnginesOverview: React.FC = () => {
title={<h2>{META_ENGINES_TITLE}</h2>}
titleSize="s"
action={
canManageEngines && (
canManageMetaEngines && (
<EuiButtonTo
color="secondary"
size="s"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { DEFAULT_INITIAL_APP_DATA } from '../../../../../common/__mocks__';
import { getRoleAbilities } from './';

describe('getRoleAbilities', () => {
const mockRole = DEFAULT_INITIAL_APP_DATA.appSearch.role;
const mockRole = DEFAULT_INITIAL_APP_DATA.appSearch.role as any;

it('transforms server role data into a flat role obj with helper shorthands', () => {
expect(getRoleAbilities(mockRole)).toEqual({
Expand Down Expand Up @@ -53,9 +53,10 @@ describe('getRoleAbilities', () => {

describe('can()', () => {
it('sets view abilities to true if manage abilities are true', () => {
const role = { ...mockRole };
role.ability.view = [];
role.ability.manage = ['account_settings'];
const role = {
...mockRole,
ability: { view: [], manage: ['account_settings'] },
};

const myRole = getRoleAbilities(role);

Expand All @@ -70,4 +71,26 @@ describe('getRoleAbilities', () => {
expect(myRole.can('edit', 'fakeSubject')).toEqual(false);
});
});

describe('canManageMetaEngines', () => {
const canManageEngines = { ability: { manage: ['account_engines'] } };

it('returns true when the user can manage any engines and the account has a platinum license', () => {
const myRole = getRoleAbilities({ ...mockRole, ...canManageEngines }, true);

expect(myRole.canManageMetaEngines).toEqual(true);
});

it('returns false when the user can manage any engines but the account does not have a platinum license', () => {
const myRole = getRoleAbilities({ ...mockRole, ...canManageEngines }, false);

expect(myRole.canManageMetaEngines).toEqual(false);
});

it('returns false when has a platinum license but the user cannot manage any engines', () => {
const myRole = getRoleAbilities({ ...mockRole, ability: { manage: [] } }, true);

expect(myRole.canManageMetaEngines).toEqual(false);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { RoleTypes, AbilityTypes, Role } from './types';
* Transforms the `role` data we receive from the Enterprise Search
* server into a more convenient format for front-end use
*/
export const getRoleAbilities = (role: Account['role']): Role => {
export const getRoleAbilities = (role: Account['role'], hasPlatinumLicense = false): Role => {
// Role ability function helpers
const myRole = {
can: (action: AbilityTypes, subject: string): boolean => {
Expand Down Expand Up @@ -49,7 +49,7 @@ export const getRoleAbilities = (role: Account['role']): Role => {
canViewSettings: myRole.can('view', 'account_settings'),
canViewRoleMappings: myRole.can('view', 'role_mappings'),
canManageEngines: myRole.can('manage', 'account_engines'),
canManageMetaEngines: myRole.can('manage', 'account_meta_engines'),
canManageMetaEngines: hasPlatinumLicense && myRole.can('manage', 'account_engines'),
canManageLogSettings: myRole.can('manage', 'account_log_settings'),
canManageSettings: myRole.can('manage', 'account_settings'),
canManageEngineCrawler: myRole.can('manage', 'engine_crawler'),
Expand Down

0 comments on commit f90c3e8

Please sign in to comment.