Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(feat): Add implicit conditions to configuration schema #475

Merged
merged 7 commits into from
Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions packages/framework/esm-api/src/shared-api-objects/current-user.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/** @module @category API */
import { reportError } from "@openmrs/esm-error-handling";
import { createGlobalStore } from "@openmrs/esm-state";
import isUndefined from "lodash-es/isUndefined";
import { Observable } from "rxjs";
import { openmrsFetch, sessionEndpoint } from "../openmrs-fetch";
import type {
Expand Down Expand Up @@ -122,15 +123,28 @@ function setUserLanguage(data: Session) {
}

function userHasPrivilege(
requiredPrivilege: string,
requiredPrivilege: string | string[],
user: { privileges: Array<Privilege> }
) {
return user.privileges.find((p) => requiredPrivilege === p.display);
if (typeof requiredPrivilege === "string") {
return !isUndefined(
user.privileges.find((p) => requiredPrivilege === p.display)
);
} else if (Array.isArray(requiredPrivilege)) {
return requiredPrivilege.every(
(rp) => !isUndefined(user.privileges.find((p) => rp === p.display))
);
} else {
console.error(`Could not understand privileges "${requiredPrivilege}"`);
}

return true;
}

function isSuperUser(user: { roles: Array<Role> }) {
const superUserRole = "System Developer";
return user.roles.find((role) => role.display === superUserRole);
return !isUndefined(
user.roles.find((role) => role.display === "System Developer")
);
}

/**
Expand Down Expand Up @@ -179,7 +193,7 @@ export function clearCurrentUser() {
}

export function userHasAccess(
requiredPrivilege: string,
requiredPrivilege: string | string[],
user: { privileges: Array<Privilege>; roles: Array<Role> }
) {
return userHasPrivilege(requiredPrivilege, user) || isSuperUser(user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,20 @@ describe("defineConfigSchema", () => {
Config.defineConfigSchema("mod-mod", schema);
expect(console.error).not.toHaveBeenCalled();
});

it("logs an error if the schema attempts to include a key named 'Display conditions'", () => {
const schema = {
"Display conditions": {
_type: Type.Array,
_default: [],
},
};

Config.defineConfigSchema("mod-mod", schema);
expect(console.error).toHaveBeenCalledWith(
expect.stringMatching(/mod-mod.*\bDisplay conditions\b/)
);
});
});

describe("getConfig", () => {
Expand Down Expand Up @@ -301,7 +315,11 @@ describe("getConfig", () => {
Config.provide(goodConfig);
const result = await Config.getConfig("foo-module");
expect(result).toStrictEqual({
bar: { a: { b: 0 }, c: 2, diff: 2 },
bar: {
a: { b: 0 },
c: 2,
diff: 2,
},
});
});

Expand Down Expand Up @@ -485,16 +503,14 @@ describe("getConfig", () => {
Config.defineConfigSchema("object-def", {
furi: {
_type: Type.Object,
_elements: {
gohan: { _type: Type.String },
},
_default: {
kake: { gohan: "ok" },
},
_elements: { gohan: { _type: Type.String } },
_default: { kake: { gohan: "ok" } },
},
});
const config = await Config.getConfig("object-def");
expect(config).toStrictEqual({ furi: { kake: { gohan: "ok" } } });
expect(config).toStrictEqual({
furi: { kake: { gohan: "ok" } },
});
});

it("interpolates freeform object element defaults", async () => {
Expand Down Expand Up @@ -823,9 +839,29 @@ describe("implementer tools config", () => {
_description: "All the foo",
_validators: [],
},
"Display conditions": {
privileges: {
_default: [],
_description:
"The privilege(s) the user must have to use this extension",
_source: "default",
_type: Type.Array,
_value: [],
},
},
},
"bar-module": {
bar: { _value: "baz", _source: "my config source", _default: "quinn" },
"Display conditions": {
privileges: {
_default: [],
_description:
"The privilege(s) the user must have to use this extension",
_source: "default",
_type: Type.Array,
_value: [],
},
},
},
});
});
Expand Down Expand Up @@ -975,6 +1011,16 @@ describe("extension slot config", () => {
remove: { _value: ["bar"], _source: "provided" },
},
},
"Display conditions": {
privileges: {
_default: [],
_description:
"The privilege(s) the user must have to use this extension",
_source: "default",
_type: Type.Array,
_value: [],
},
},
},
});
});
Expand Down
42 changes: 36 additions & 6 deletions packages/framework/esm-config/src/module-config/module-config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/** @module @category Config */
import { clone, reduce, mergeDeepRight, equals } from "ramda";
import { clone, reduce, mergeDeepRight, equals, omit } from "ramda";
import {
Config,
ConfigObject,
Expand Down Expand Up @@ -27,7 +27,6 @@ import {
implementerToolsConfigStore,
temporaryConfigStore,
getExtensionSlotsConfigStore,
ExtensionsConfigStore,
} from "./state";
import type {} from "@openmrs/esm-globals";
import { TemporaryConfigStore } from "..";
Expand Down Expand Up @@ -192,9 +191,14 @@ function computeExtensionConfigs(
*/
export function defineConfigSchema(moduleName: string, schema: ConfigSchema) {
validateConfigSchema(moduleName, schema);
const enhancedSchema = mergeDeepRight(
schema,
displayConditionsSchema
) as ConfigSchema;

const state = configInternalStore.getState();
configInternalStore.setState({
schemas: { ...state.schemas, [moduleName]: schema },
schemas: { ...state.schemas, [moduleName]: enhancedSchema },
});
}

Expand All @@ -220,14 +224,20 @@ export function defineExtensionConfigSchema(
schema: ConfigSchema
) {
validateConfigSchema(extensionName, schema);
const enhancedSchema = mergeDeepRight(
schema,
displayConditionsSchema
) as ConfigSchema;

const state = configInternalStore.getState();
if (state.schemas[extensionName]) {
console.warn(
`Config schema for extension ${extensionName} already exists. If there are multiple extensions with this same name, one will probably crash.`
);
}

configInternalStore.setState({
schemas: { ...state.schemas, [extensionName]: schema },
schemas: { ...state.schemas, [extensionName]: enhancedSchema },
});
}

Expand Down Expand Up @@ -256,7 +266,8 @@ export function getConfig(moduleName: string): Promise<Config> {
const store = getConfigStore(moduleName);
function update(state: ConfigStore) {
if (state.loaded && state.config) {
resolve(state.config);
const config = omit(["Display conditions"], state.config);
resolve(config);
unsubscribe && unsubscribe();
}
}
Expand Down Expand Up @@ -362,7 +373,7 @@ function getSchemaWithValuesAndSources(schema) {
return obj;
}, {});
} else {
// Schema is bad; error will have been logged during schema validation
// at this point, the schema is bad and an error will have been logged during schema validation
return {};
}
}
Expand Down Expand Up @@ -492,6 +503,12 @@ function validateConfigSchema(
const thisKeyPath = keyPath + (keyPath && ".") + key;
const schemaPart = schema[key] as ConfigSchema;

if (thisKeyPath === "Display conditions") {
console.error(
`${moduleName} declares a configuration option called "Display conditions"; the "Display conditions" option is a reserved name. ${updateMessage}`
);
}

if (!isOrdinaryObject(schemaPart)) {
console.error(
`${moduleName} has bad config schema definition for key '${thisKeyPath}'. ${updateMessage}`
Expand Down Expand Up @@ -863,3 +880,16 @@ function getExtensionNameFromId(extensionId: string) {
const [extensionName] = extensionId.split("#");
return extensionName;
}

/**
* The displayConditionsSchema is implicitly included in every configuration schema
*/
const displayConditionsSchema: ConfigSchema = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite different from how we handle the extension config. But it is a cool idea, and might actually be a better approach than what we currently have implemented. The only tricky part, at first glance, is that IMO these special keys (extensionSlots, Display conditions) should not be included in the resolved config object returned by useConfig/getConfig.

"Display conditions": {
privileges: {
_description: "The privilege(s) the user must have to use this extension",
_type: Type.Array,
_default: [],
},
},
};
14 changes: 10 additions & 4 deletions packages/framework/esm-config/src/module-config/state.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/** @module @category Config */
import { createGlobalStore, getGlobalStore } from "@openmrs/esm-state";
import { omit } from "ramda";
import {
Config,
ConfigObject,
Expand Down Expand Up @@ -199,11 +200,16 @@ export function getExtensionsConfigStore() {

/** @internal */
export function getExtensionConfig(slotName: string, extensionId: string) {
return getExtensionConfigFromStore(
getExtensionsConfigStore().getState(),
slotName,
extensionId
const extensionConfig = Object.assign(
{},
getExtensionConfigFromStore(
getExtensionsConfigStore().getState(),
slotName,
extensionId
)
);
extensionConfig.config = omit(["Display conditions"], extensionConfig.config);
return extensionConfig;
}

/** @internal */
Expand Down
5 changes: 5 additions & 0 deletions packages/framework/esm-config/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ export interface Config extends Object {

export interface ConfigObject extends Object {
[key: string]: any;
"Display conditions"?: DisplayConditionsConfigObject;
}

export interface DisplayConditionsConfigObject {
privileges?: string[];
}

export type ConfigValue =
Expand Down
10 changes: 10 additions & 0 deletions packages/framework/esm-extensions/src/extensions.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
import { createGlobalStore } from "@openmrs/esm-state";
import { attach, registerExtensionSlot } from "./extensions";

const mockSessionStore = createGlobalStore("mock-session-store", {
loaded: false,
session: null,
});

jest.mock("@openmrs/esm-api", () => ({
getSessionStore: jest.fn(() => mockSessionStore),
}));

describe("extensions system", () => {
it("shouldn't crash when a slot is registered before the extensions that go in it", () => {
attach("mario-slot", "mario-hat");
Expand Down
31 changes: 31 additions & 0 deletions packages/framework/esm-extensions/src/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* - connected (computed from assigned using connectivity and online / offline)
*/

import { getSessionStore, LoggedInUser, userHasAccess } from "@openmrs/esm-api";
import {
ExtensionsConfigStore,
ExtensionSlotConfigObject,
Expand All @@ -19,6 +20,7 @@ import {
getExtensionSlotsConfigStore,
} from "@openmrs/esm-config";
import isEqual from "lodash-es/isEqual";
import isUndefined from "lodash-es/isUndefined";
import {
getExtensionInternalStore,
ExtensionSlotState,
Expand Down Expand Up @@ -301,6 +303,8 @@ function getAssignedExtensionsFromSlotData(
const attachedIds = internalState.slots[slotName].attachedIds;
const assignedIds = calculateAssignedIds(config, attachedIds);
const extensions: Array<AssignedExtension> = [];
let user: LoggedInUser | undefined = undefined;

for (let id of assignedIds) {
const { config: extensionConfig } = getExtensionConfigFromStore(
extensionConfigStoreState,
Expand All @@ -311,6 +315,27 @@ function getAssignedExtensionsFromSlotData(
const extension = internalState.extensions[name];
// if the extension has not been registered yet, do not include it
if (extension) {
const requiredPrivileges =
extensionConfig?.["Display conditions"]?.privileges ??
extension.privileges;
if (
requiredPrivileges &&
(typeof requiredPrivileges === "string" ||
(Array.isArray(requiredPrivileges) && requiredPrivileges.length > 0))
) {
if (isUndefined(user)) {
user = getSessionStore().getState().session?.user;
}

if (!user) {
continue;
}

if (!userHasAccess(requiredPrivileges, user)) {
continue;
}
}

extensions.push({
id,
name,
Expand All @@ -325,6 +350,12 @@ function getAssignedExtensionsFromSlotData(
return extensions;
}

/**
* Gets the list of extensions assigned to a given slot
*
* @param slotName The slot to load the assigned extensions for
* @returns An array of extensions assigned to the named slot
*/
export function getAssignedExtensions(
slotName: string
): Array<AssignedExtension> {
Expand Down
1 change: 1 addition & 0 deletions packages/framework/esm-extensions/src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface ExtensionRegistration {
order?: number;
online?: boolean | object;
offline?: boolean | object;
privileges?: string | string[];
}

export interface ExtensionInfo extends ExtensionRegistration {
Expand Down
Loading