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

Foundations for Warnings System #161

Merged
merged 16 commits into from
Dec 13, 2024
Merged
2 changes: 1 addition & 1 deletion docker-local-setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ to build missing images";echo;
fi

echo;echo "Starting Docker Compose...";echo
CHT_USER_MANAGEMENT_IMAGE=cht-user-management:local CHT_USER_MANAGEMENT_WORKER_IMAGE=cht-user-management-worker:local docker compose up -d
CHT_USER_MANAGEMENT_IMAGE=cht-user-management:local CHT_USER_MANAGEMENT_WORKER_IMAGE=cht-user-management-worker:local docker compose up

echo;echo "Server is now running at http://127.0.0.1:$EXTERNAL_PORT/login";echo
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "cht-user-management",
"version": "1.4.2",
"version": "1.5.0",
"main": "dist/index.js",
"dependencies": {
"@bull-board/api": "^5.17.0",
Expand Down
12 changes: 7 additions & 5 deletions scripts/create-user-managers/create-user-managers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ import { Command } from 'commander';
import { AuthenticationInfo, ContactType } from '../../src/config';
import { createUserWithRetries } from '../../src/lib/retry-logic';
import Place from '../../src/services/place';
import { UserPayload } from '../../src/services/user-payload';
import RemotePlaceCache, { RemotePlace } from '../../src/lib/remote-place-cache';
import { PropertyValues, UnvalidatedPropertyValue } from '../../src/property-value';
import UserManager from './ke_user_manager.json';
import { UserPayload } from '../../src/services/user-payload';

const { ChtApi } = require('../../src/lib/cht-api'); // require is needed for rewire
const ChtSession = require('../../src/lib/cht-session').default; // require is needed for rewire
Expand Down Expand Up @@ -53,8 +55,8 @@ export default async function createUserManagers(argv: string[]) {

async function createUserManager(username: string, placeDocId: string, chtApi: typeof ChtApi, adminUsername: string, passwordOverride?: string) {
const place = new Place(UserManagerContactType);
place.contact.properties.name = `${username} (User Manager)`;
place.userRoleProperties.role = UserManagerContactType.user_role.join(' ');
place.contact.properties.name = new UnvalidatedPropertyValue(`${username} (User Manager)`, 'name');
place.userRoleProperties.role = new UnvalidatedPropertyValue(UserManagerContactType.user_role.join(' '), 'role');

const chtPayload = place.asChtPayload(adminUsername);
chtPayload.contact.role = 'user_manager';
Expand Down Expand Up @@ -96,8 +98,8 @@ function parseCommandlineArguments(argv: string[]): CommandLineArgs {
}

async function getPlaceDocId(county: string | undefined, chtApi: typeof ChtApi) {
const counties = await chtApi.getPlacesWithType('a_county');
const countyMatches = counties.filter((c: any) => !county || c.name === county.toLowerCase());
const counties = await RemotePlaceCache.getPlacesWithType(chtApi, UserManagerContactType, UserManagerContactType.hierarchy[0]);
const countyMatches = counties.filter((c: RemotePlace) => !county || PropertyValues.isMatch(county, c.name));
if (countyMatches.length < 1) {
throw Error(`Could not find county "${county}"`);
}
Expand Down
10 changes: 9 additions & 1 deletion scripts/create-user-managers/ke_user_manager.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,15 @@
"contact_type": "person",
"user_role": ["user_manager", "mm-online"],
"username_from_place": false,
"hierarchy": [],
"hierarchy": [{
"type": "name",
"friendly_name": "County",
"property_name": "name",
"required": false,
"parameter": ["\\sCounty"],
"contact_type": "a_county",
"level": 0
}],
"deactivate_users_on_replace": false,
"replacement_property": {
"friendly_name": "Unused",
Expand Down
2 changes: 1 addition & 1 deletion src/config/config-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import kenyaConfig from './chis-ke';
import togoConfig from './chis-tg';
import civConfig from './chis-civ';

const CONFIG_MAP: { [key: string]: PartnerConfig } = {
export const CONFIG_MAP: { [key: string]: PartnerConfig } = {
'CHIS-KE': kenyaConfig,
'CHIS-UG': ugandaConfig,
'CHIS-TG': togoConfig,
Expand Down
37 changes: 35 additions & 2 deletions src/config/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import _ from 'lodash';
import { ChtApi, PlacePayload } from '../lib/cht-api';
import getConfigByKey from './config-factory';
import Validation from '../validation';

export type ConfigSystem = {
domains: AuthenticationInfo[];
Expand Down Expand Up @@ -28,10 +29,13 @@ export type ContactType = {
hint?: string;
};

const KnownContactPropertyTypes = [...Validation.getKnownContactPropertyTypes()] as const;
export type ContactPropertyType = typeof KnownContactPropertyTypes[number];

export type HierarchyConstraint = {
friendly_name: string;
property_name: string;
type: string;
type: ContactPropertyType;
required: boolean;
parameter? : string | string[] | object;
errorDescription? : string;
Expand All @@ -43,7 +47,7 @@ export type HierarchyConstraint = {
export type ContactProperty = {
friendly_name: string;
property_name: string;
type: string;
type: ContactPropertyType;
required: boolean;
parameter? : string | string[] | object;
errorDescription? : string;
Expand All @@ -55,6 +59,7 @@ export type AuthenticationInfo = {
useHttp?: boolean;
};


const {
CONFIG_NAME,
NODE_ENV,
Expand Down Expand Up @@ -187,6 +192,33 @@ export class Config {
return _.sortBy(domains, 'friendly');
}

// TODO: Joi? Chai?
public static assertValid({ config }: PartnerConfig = partnerConfig) {
for (const contactType of config.contact_types) {
const allHierarchyProperties = [...contactType.hierarchy, contactType.replacement_property];
const allProperties = [
...contactType.place_properties,
...contactType.contact_properties,
...allHierarchyProperties,
Config.getUserRoleConfig(contactType),
];

Config.getPropertyWithName(contactType.place_properties, 'name');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not related to the PR but nothing hints that getPropertyWithName might actually throw an error which i think makes it hard to reason about error handling in functions that call it.

Copy link
Member Author

Choose a reason for hiding this comment

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

true. now that we assert it on startup, maybe we don't need it in the property? we know it is also there

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we don't need it in the property

Yeah i think it would be clearer if getPropertyWithName returned a string or undefined; then the caller can choose to assert if undefined and throw if needed

Copy link
Member Author

Choose a reason for hiding this comment

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

since we assert at startup, we can guarantee it is never undefined at runtime
this allows us to safely make it a string which takes the burden off the calling code
i personally like this since it leads to cleaner calling code. would you rather i not assert in the get and just use typescript operators to cast?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't we get a runtime error if the property name that was being accessed was not asserted at start up like name is? If this function is only used for getting name then maybe it can be renamed and args changed?

Config.getPropertyWithName(contactType.contact_properties, 'name');

allProperties.forEach(property => {
if (!KnownContactPropertyTypes.includes(property.type)) {
throw Error(`Unknown property type "${property.type}"`);
}
});

const generatedHierarchyProperties = allHierarchyProperties.filter(hierarchy => hierarchy.type === 'generated');
if (generatedHierarchyProperties.length) {
throw Error('Hierarchy properties cannot be of type "generated"');
}
}
}

public static getCsvTemplateColumns(placeType: string) {
const placeTypeConfig = Config.getContactType(placeType);
const hierarchy = Config.getHierarchyWithReplacement(placeTypeConfig);
Expand All @@ -205,3 +237,4 @@ export class Config {
return columns;
}
}

9 changes: 3 additions & 6 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
require('dotenv').config();
import { Config } from './config';
import build from './server';
import { env } from 'process';
const {
INTERFACE
} = process.env;

const port: number = env.PORT ? parseInt(env.PORT) : 3500;
Config.assertValid();

(async () => {
const loggerConfig = {
transport: {
target: 'pino-pretty',
},
};
const server = build({
logger: loggerConfig,
logger: true,
});

// in 1.1.0 we allowed INTERFACE to be declared in .env, but let's be
Expand Down
39 changes: 5 additions & 34 deletions src/lib/cht-api.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import _ from 'lodash';
import { AxiosInstance } from 'axios';
import ChtSession from './cht-session';
import { Config, ContactType } from '../config';
import { UserPayload } from '../services/user-payload';
import { AxiosInstance } from 'axios';

export type PlacePayload = {
name: string;
Expand All @@ -18,17 +18,6 @@ export type PlacePayload = {
[key: string]: any;
};

export type RemotePlace = {
id: string;
name: string;
lineage: string[];
ambiguities?: RemotePlace[];

// sadly, sometimes invalid or uncreated objects "pretend" to be remote
// should reconsider this naming
type: 'remote' | 'local' | 'invalid';
};

export class ChtApi {
public readonly chtSession: ChtSession;
private axiosInstance: AxiosInstance;
Expand Down Expand Up @@ -174,26 +163,15 @@ export class ChtApi {
};

getPlacesWithType = async (placeType: string)
: Promise<RemotePlace[]> => {
const url = `medic/_design/medic-client/_view/contacts_by_type_freetext`;
: Promise<any[]> => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this array of type RemotePlace by default?

Copy link
Member Author

@kennsippell kennsippell May 15, 2024

Choose a reason for hiding this comment

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

Not yet. Right now it returns the raw documents from CouchDB. I use any to capture that.
RemotePlaceCache.fetchRemotePlaces now converts the CouchDB doc into a RemotePlace. I didn't want to cache the full object from CouchDB because it could be quite large and I want things to work with our validation/formatting/generation stuff.

const url = `medic/_design/medic-client/_view/contacts_by_type`;
const params = {
startkey: JSON.stringify([ placeType, 'name:']),
endkey: JSON.stringify([ placeType, 'name:\ufff0']),
key: JSON.stringify([placeType]),
include_docs: true,
};
console.log('axios.get', url, params);
const resp = await this.axiosInstance.get(url, { params });

return resp.data.rows
.map((row: any): RemotePlace => {
const nameData = row.key[1];
return {
id: row.id,
name: nameData.substring('name:'.length),
lineage: extractLineage(row.doc),
type: 'remote',
};
});
return resp.data.rows.map((row: any) => row.doc);
};

getDoc = async (id: string): Promise<any> => {
Expand Down Expand Up @@ -228,10 +206,3 @@ function minify(doc: any): any {
};
}

function extractLineage(doc: any): string[] {
if (doc?.parent?._id) {
return [doc.parent._id, ...extractLineage(doc.parent)];
}

return [];
}
2 changes: 1 addition & 1 deletion src/lib/cht-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { AuthenticationInfo } from '../config';
import { AxiosHeaders, AxiosInstance } from 'axios';
import axiosRetry from 'axios-retry';
import { axiosRetryConfig } from './retry-logic';
import { RemotePlace } from './cht-api';
import { RemotePlace } from './remote-place-cache';

const COUCH_AUTH_COOKIE_NAME = 'AuthSession=';
const ADMIN_FACILITY_ID = '*';
Expand Down
52 changes: 52 additions & 0 deletions src/lib/credentials-file.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { Config, ContactType } from '../config';
import SessionCache from '../services/session-cache';
import { stringify } from 'csv/sync';

type File = {
filename: string;
content: string;
};

export default function getCredentialsFiles(sessionCache: SessionCache, contactTypes: ContactType[]): File[] {
const files: File[] = [];
for (const contactType of contactTypes) {
const places = sessionCache.getPlaces({ type: contactType.name });
if (!places.length) {
continue;
}

const rows = places.map((place) => [
...Object.values(place.hierarchyProperties).map(prop => prop.formatted),
place.name,
place.contact.properties.name?.formatted,
place.contact.properties.phone?.formatted,
place.userRoles.join(' '),
place.creationDetails.username,
place.creationDetails.password,
]);

const constraints = Config.getHierarchyWithReplacement(contactType);
const props = Object.keys(places[0].hierarchyProperties)
.map(prop => constraints.find(c => c.property_name === prop)!.friendly_name);
const columns = [
...props,
contactType.friendly,
'name',
'phone',
'role',
'username',
'password',
];

const content = stringify(rows, {
columns,
header: true,
});
files.push({
filename: `${contactType.name}.csv`,
content,
});
}

return files;
}
4 changes: 2 additions & 2 deletions src/lib/move.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default class MoveLib {
}

if (toId === fromLineage[1]?.id) {
throw Error(`Place "${fromLineage[0]?.name}" already has "${toLineage[1]?.name}" as parent`);
throw Error(`Place "${fromLineage[0]?.name.original}" already has "${toLineage[1]?.name.original}" as parent`);
}

const jobName = this.getJobName(fromLineage, toLineage);
Expand Down Expand Up @@ -63,7 +63,7 @@ async function resolve(prefix: string, formData: any, contactType: ContactType,
await RemotePlaceResolver.resolveOne(place, sessionCache, chtApi, { fuzz: true });
place.validate();

const validationError = place.validationErrors && Object.keys(place.validationErrors).find(err => err.startsWith('hierarchy_'));
const validationError = place.validationErrors && Object.keys(place.validationErrors).find(err => err.startsWith(prefix));
if (validationError) {
throw Error(place.validationErrors?.[validationError]);
}
Expand Down
Loading
Loading