Skip to content

Commit

Permalink
fix: address PR#52 comments round 4
Browse files Browse the repository at this point in the history
Simplify getLogFormatDate, naming and casing.
Fix userRoles repeating in userCredentials.
  • Loading branch information
nshandra committed Sep 24, 2024
1 parent 11f770c commit 3d1e216
Show file tree
Hide file tree
Showing 17 changed files with 77 additions and 74 deletions.
6 changes: 3 additions & 3 deletions src/data/user-monitoring/common/MessageMSTeamsRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@ export class MessageMSTeamsRepository implements MessageRepository {

async sendMessage(messageType: string, message: string): Async<boolean> {
const httpProxy = this.webhook.proxy;
const url = this.webhook.ms_url;
const server_name = this.webhook.server_name;
const url = this.webhook.msUrl;
const serverName = this.webhook.serverName;

if (!isEmpty(httpProxy)) {
process.env["http_proxy"] = httpProxy;
process.env["https_proxy"] = httpProxy;
}

const postData = JSON.stringify({
text: `[*${messageType}* - ${server_name}] - ${message}`,
text: `[*${messageType}* - ${serverName}] - ${message}`,
});

const requestOptions = {
Expand Down
4 changes: 2 additions & 2 deletions src/data/user-monitoring/entities/MSTeamsWebhookOptions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export interface MSTeamsWebhookOptions {
ms_url: string;
msUrl: string;
proxy: string;
server_name: string;
serverName: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,25 @@ export class UserD2Repository implements UserRepository {
.getData();

return users.objects.map((user): User => {
// used to avoid repeating userRoles in userCredentials
const { userRoles, ...userCredentials } = user.userCredentials;

return {
...user,
username: user.userCredentials.username,
disabled: user.userCredentials.disabled,
twoFA: user.userCredentials.twoFA,
userRoles: user.userCredentials.userRoles,
invitation: user.userCredentials.invitation,
externalAuth: user.userCredentials.externalAuth,
selfRegistered: user.userCredentials.selfRegistered,
catDimensionConstraints: user.userCredentials.catDimensionConstraints,
cogsDimensionConstraints: user.userCredentials.cogsDimensionConstraints,
username: userCredentials.username,
disabled: userCredentials.disabled,
twoFA: userCredentials.twoFA,
userRoles: userRoles,
invitation: userCredentials.invitation,
externalAuth: userCredentials.externalAuth,
selfRegistered: userCredentials.selfRegistered,
catDimensionConstraints: userCredentials.catDimensionConstraints,
cogsDimensionConstraints: userCredentials.cogsDimensionConstraints,
attributeValues: user.attributeValues.map(attributeValue => ({
attribute: attributeValue.attribute.name,
value: attributeValue.value,
})),
userCredentials: userCredentials,
};
});
}
Expand Down
19 changes: 9 additions & 10 deletions src/domain/usecases/user-monitoring/GetLogFormatDate.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
export class GetLogFormatDate {
private logFormatDate(date: Date): string {
const isoString = date.toISOString();
/**
* Formats a given date into a DHIS2 log timestamp.
*
* @param date - The date to be formatted.
* @returns The timestamp as a string.
*/

return isoString.replace("Z", "");
}
export function getLogFormatDate(date: Date): string {
const isoString = date.toISOString();

constructor() {}

execute(date: Date): string {
return this.logFormatDate(date);
}
return isoString.replace("Z", "").replace(".", ",");
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import _ from "lodash";
import { Async } from "../../../entities/Async";
import { UsersByAuthority } from "../../../entities/user-monitoring/authorities-monitoring/AuthoritiesMonitoringOptions";

export class CheckUserByAuthoritiesChangesUseCase {
export class CheckUserByAuthoritiesChanges {
constructor() {}

private compareDicts(dict1: UsersByAuthority, dict2: UsersByAuthority) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { User } from "domain/entities/user-monitoring/authorities-monitoring/Use
import { AuthoritiesMonitoringOptions } from "domain/entities/user-monitoring/authorities-monitoring/AuthoritiesMonitoringOptions";

import { GetUsersByAuthoritiesUseCase } from "./GetUsersByAuthoritiesUseCase";
import { CheckUserByAuthoritiesChangesUseCase } from "./CheckUserByAuthoritiesChangesUseCase";
import { CheckUserByAuthoritiesChanges } from "./CheckUserByAuthoritiesChanges";
import { GetAuthoritiesMonitoringConfigUseCase } from "./GetAuthoritiesMonitoringConfigUseCase";
import { SaveAuthoritiesMonitoringConfigUseCase } from "./SaveAuthoritiesMonitoringConfigUseCase";

Expand Down Expand Up @@ -61,8 +61,8 @@ export class MonitorUsersByAuthorityUseCase {
this.debugJSON("Users by authority:", usersByAuthority);

if (!setDataStore) {
const checkUsersChangesUseCase = new CheckUserByAuthoritiesChangesUseCase();
const { newUsers, usersLosingAuth } = await checkUsersChangesUseCase.execute(
const checkUsersChanges = new CheckUserByAuthoritiesChanges();
const { newUsers, usersLosingAuth } = await checkUsersChanges.execute(
options.usersByAuthority,
usersByAuthority
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ import {
UsersByAuthority,
} from "domain/entities/user-monitoring/authorities-monitoring/AuthoritiesMonitoringOptions";

import { GetLogFormatDate } from "../GetLogFormatDate";
import { getLogFormatDate } from "../GetLogFormatDate";

export class SaveAuthoritiesMonitoringConfigUseCase {
constructor(private configRepository: AuthoritiesMonitoringConfigRepository) {}

async execute(options: AuthoritiesMonitoringOptions, usersByAuthority: UsersByAuthority): Async<void> {
const new_options = {
const newOptions = {
...options,
lastExecution: new GetLogFormatDate().execute(new Date()),
lastExecution: getLogFormatDate(new Date()),
usersByAuthority: usersByAuthority,
};

await this.configRepository.save(new_options);
await this.configRepository.save(newOptions);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,49 +8,49 @@ import {
noneUsers,
} from "./AuthoritiesMonitoringTests.data";

import { CheckUserByAuthoritiesChangesUseCase } from "../CheckUserByAuthoritiesChangesUseCase";
import { CheckUserByAuthoritiesChanges } from "../CheckUserByAuthoritiesChanges";

describe("CheckUserByAuthoritiesChangesUseCase", () => {
describe("CheckUserByAuthoritiesChanges", () => {
it("Should find no changes in the same UsersByAuthority object", async () => {
const useCase = new CheckUserByAuthoritiesChangesUseCase();
const checkChanges = new CheckUserByAuthoritiesChanges();

const { newUsers, usersLosingAuth } = await useCase.execute(allAuthUsers, allAuthUsers);
const { newUsers, usersLosingAuth } = await checkChanges.execute(allAuthUsers, allAuthUsers);

expect(newUsers).toEqual(noChanges);
expect(usersLosingAuth).toEqual(noChanges);
});

it("Should find users with new authorities", async () => {
const useCase = new CheckUserByAuthoritiesChangesUseCase();
const checkChanges = new CheckUserByAuthoritiesChanges();

const { newUsers, usersLosingAuth } = await useCase.execute(noneUsers, allAuthUsers);
const { newUsers, usersLosingAuth } = await checkChanges.execute(noneUsers, allAuthUsers);

expect(newUsers).toEqual(allAuthUsers);
expect(usersLosingAuth).toEqual(noChanges);
});

it("Should find users losing authorities", async () => {
const useCase = new CheckUserByAuthoritiesChangesUseCase();
const checkChanges = new CheckUserByAuthoritiesChanges();

const { newUsers, usersLosingAuth } = await useCase.execute(auth2Users, noneUsers);
const { newUsers, usersLosingAuth } = await checkChanges.execute(auth2Users, noneUsers);

expect(newUsers).toEqual(noChanges);
expect(usersLosingAuth).toEqual(auth2Users);
});

it("Should find new and losing types of changes", async () => {
const useCase = new CheckUserByAuthoritiesChangesUseCase();
const checkChanges = new CheckUserByAuthoritiesChanges();

const { newUsers, usersLosingAuth } = await useCase.execute(auth2Users, auth1Users);
const { newUsers, usersLosingAuth } = await checkChanges.execute(auth2Users, auth1Users);

expect(newUsers).toEqual(auth1Users);
expect(usersLosingAuth).toEqual(auth2Users);
});

it("Should find new, no change and losing types of changes", async () => {
const useCase = new CheckUserByAuthoritiesChangesUseCase();
const checkChanges = new CheckUserByAuthoritiesChanges();

const { newUsers, usersLosingAuth } = await useCase.execute(twoAuthUsers, auth1and2Users);
const { newUsers, usersLosingAuth } = await checkChanges.execute(twoAuthUsers, auth1and2Users);

expect(newUsers).toEqual(auth1Users);
expect(usersLosingAuth).toEqual(auth3Users);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export class MonitorUserGroupsUseCase {
log.info(`Get user groups with ids: ${options.groupsToMonitor.join(", ")}`);

const getGroupsUseCase = new GetUserGroupsUseCase(this.userGroupRepository);
const compareUserGroupsUseCase = new CompareUserGroups();
const compareUserGroups = new CompareUserGroups();

const userGroups: UserGroup[] = await getGroupsUseCase.execute(options.groupsToMonitor);
log.info(`Retrieved user groups: ${userGroups.map(g => g.id).join(", ")}`);
Expand All @@ -86,7 +86,7 @@ export class MonitorUserGroupsUseCase {
return [];
}

const changes = compareUserGroupsUseCase.execute(orig, group);
const changes = compareUserGroups.execute(orig, group);

if (
_.isEmpty(changes.changedPropsAdded) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@ import { Async } from "domain/entities/Async";
import { UserGroupsMonitoringOptions } from "domain/entities/user-monitoring/user-groups-monitoring/UserGroupsMonitoringOptions";
import { UserGroup } from "domain/entities/user-monitoring/user-groups-monitoring/UserGroups";

import { GetLogFormatDate } from "../GetLogFormatDate";
import { getLogFormatDate } from "../GetLogFormatDate";

export class SaveUserGroupsMonitoringConfigUseCase {
constructor(private configRepository: UserGroupsMonitoringConfigRepository) {}

async execute(options: UserGroupsMonitoringOptions, monitoredUserGroups: UserGroup[]): Async<void> {
const new_options = {
const newOptions = {
...options,
lastExecution: new GetLogFormatDate().execute(new Date()),
lastExecution: getLogFormatDate(new Date()),
monitoredUserGroups: monitoredUserGroups,
};

await this.configRepository.save(new_options);
await this.configRepository.save(newOptions);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,28 @@ import {
minimalUserGroup,
userGroup1,
userGroup1Updated,
} from "./CompareUserGroupsUseCase.data";
} from "./CompareUserGroups.data";

describe("CompareUserGroupsUseCase", () => {
describe("CompareUserGroups", () => {
it("Should return empty array when comparing the same objects", () => {
const useCase = new CompareUserGroups();
const compareUserGroups = new CompareUserGroups();

const minUserGroup2 = _.cloneDeep(minimalUserGroup);

const result = useCase.execute(minimalUserGroup, minUserGroup2);
const result = compareUserGroups.execute(minimalUserGroup, minUserGroup2);

const userGroup2 = _.cloneDeep(userGroup1);

const result2 = useCase.execute(userGroup1, userGroup2);
const result2 = compareUserGroups.execute(userGroup1, userGroup2);

expect(result).toEqual(emptyDiff);
expect(result2).toEqual(emptyDiff);
});

it("Should return the differences between two user groups", () => {
const useCase = new CompareUserGroups();
const compareUserGroups = new CompareUserGroups();

const result = useCase.execute(userGroup1, userGroup1Updated);
const result = compareUserGroups.execute(userGroup1, userGroup1Updated);

expect(result).toEqual(userGroup1Diff);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class MonitorUserTemplatesUseCase {
log.info(`Get user groups with usernames: ${options.templatesToMonitor.join(", ")}`);

const getTemplatesUseCase = new GetUserTemplatesUseCase(this.usersRepository);
const compareUserTemplatesUseCase = new CompareUserTemplates();
const compareUserTemplates = new CompareUserTemplates();

const userTemplates: User[] = await getTemplatesUseCase.execute(options.templatesToMonitor);
log.info(`Retrieved user templates: ${userTemplates.map(g => g.username).join(", ")}`);
Expand All @@ -111,7 +111,7 @@ export class MonitorUserTemplatesUseCase {
return [];
}

const changes = compareUserTemplatesUseCase.execute(orig, user);
const changes = compareUserTemplates.execute(orig, user);

if (
_.isEmpty(changes.changedPropsAdded) &&
Expand All @@ -128,7 +128,7 @@ export class MonitorUserTemplatesUseCase {
return changes;
});

log.info(`userGroupsChanges: ${this.stringifyObject(userGroupsChanges)}`);
log.debug(`userGroupsChanges: ${this.stringifyObject(userGroupsChanges)}`);

if (_.isEmpty(userGroupsChanges)) {
log.info("Report: No changes.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@ import { Async } from "domain/entities/Async";
import { UserTemplatesMonitoringOptions } from "domain/entities/user-monitoring/user-templates-monitoring/UserTemplatesMonitoringOptions";
import { User } from "domain/entities/user-monitoring/user-templates-monitoring/Users";

import { GetLogFormatDate } from "../GetLogFormatDate";
import { getLogFormatDate } from "../GetLogFormatDate";

export class SaveUserTemplatesMonitoringConfigUseCase {
constructor(private configRepository: UserTemplatesMonitoringConfigRepository) {}

async execute(options: UserTemplatesMonitoringOptions, monitoredUserTemplates: User[]): Async<void> {
const new_options = {
const newOptions = {
...options,
lastExecution: new GetLogFormatDate().execute(new Date()),
lastExecution: getLogFormatDate(new Date()),
monitoredUserTemplates: monitoredUserTemplates,
};

await this.configRepository.save(new_options);
await this.configRepository.save(newOptions);
}
}
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
import { describe, it, expect, beforeEach } from "vitest";

import { user1, user1Updated, noChangesDiff, expectedDiff } from "./CompareUserTemplatesUseCase.data";
import { user1, user1Updated, noChangesDiff, expectedDiff } from "./CompareUserTemplates.data";

import { CompareUserTemplates } from "../CompareUserTemplates";

describe("CompareUserTemplatesUseCase", () => {
let useCase: CompareUserTemplates;
describe("CompareUserTemplates", () => {
let compareUserTemplates: CompareUserTemplates;

beforeEach(() => {
useCase = new CompareUserTemplates();
compareUserTemplates = new CompareUserTemplates();
});

it("should not fild differences between the same user templates", () => {
const diff = useCase.execute(user1, user1);
const diff = compareUserTemplates.execute(user1, user1);

expect(diff).toEqual(noChangesDiff);
});

it("should compare user templates and return the differences", () => {
const diff = useCase.execute(user1, user1Updated);
const diff = compareUserTemplates.execute(user1, user1Updated);

expect(diff).toEqual(expectedDiff);
});
Expand Down
8 changes: 4 additions & 4 deletions src/scripts/commands/userMonitoring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,14 +232,14 @@ function getAuthFromFile(configFile: string): UserMonitoringAuth {
function getWebhookConfFromFile(configFile: string): MSTeamsWebhookOptions {
const fs = require("fs");
const configJSON = JSON.parse(fs.readFileSync("./" + configFile, "utf8"));
const ms_url = configJSON["WEBHOOK"]["ms_url"];
const msUrl = configJSON["WEBHOOK"]["ms_url"];
const proxy = configJSON["WEBHOOK"]["proxy"];
const server_name = configJSON["WEBHOOK"]["server_name"];
const serverName = configJSON["WEBHOOK"]["server_name"];

return {
ms_url: ms_url,
msUrl: msUrl,
proxy: proxy,
server_name: server_name,
serverName: serverName,
};
}

Expand Down

0 comments on commit 3d1e216

Please sign in to comment.