Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
21 changes: 0 additions & 21 deletions src/data/scheduler/SchedulerD2ApiRepository.ts

This file was deleted.

27 changes: 27 additions & 0 deletions src/data/scheduler/SchedulerExecutionInfoD2ApiRepository.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { Instance } from "../../domain/instance/entities/Instance";
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we add a decorator for all refactored files? So that we can keep track of all the files that are pending refactor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added comments on the refactored files and TODOs commenting on what they need to be fully refactored. Also in PR I have added some common reactoring tasks

import { SchedulerExecutionInfo } from "../../domain/scheduler/entities/SchedulerExecutionInfo";
import { SchedulerExecutionInfoRepository } from "../../domain/scheduler/repositories/SchedulerExecutionInfoRepositoryConstructor";
import { Namespace } from "../storage/Namespaces";
import { StorageDataStoreClient } from "../storage/StorageDataStoreClient";
import { SchedulerExecutionInfoModel } from "./models/SchedulerExecutionInfoModel";

export class SchedulerExecutionInfoD2ApiRepository implements SchedulerExecutionInfoRepository {
private dataStoreClient: StorageDataStoreClient;

constructor(private instance: Instance) {
this.dataStoreClient = new StorageDataStoreClient(this.instance);
}

public async updateExecutionInfo(execution: SchedulerExecutionInfo): Promise<void> {
const data = SchedulerExecutionInfoModel.encode<SchedulerExecutionInfo>(execution);
return this.dataStoreClient.saveObject<SchedulerExecutionInfo>(Namespace.SCHEDULER_EXECUTIONS, data);
}

public async getLastExecutionInfo(): Promise<SchedulerExecutionInfo> {
const data = await this.dataStoreClient.getOrCreateObject<SchedulerExecutionInfo>(
Namespace.SCHEDULER_EXECUTIONS,
{}
);
return SchedulerExecutionInfoModel.unsafeDecode(data);
}
}
55 changes: 55 additions & 0 deletions src/data/scheduler/SyncRuleJobConfigD2ApiRepository.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { SynchronizationRule, SynchronizationRuleData } from "../../domain/rules/entities/SynchronizationRule";
import { SyncRuleJobConfigRepository } from "../../domain/scheduler/repositories/SyncRuleJobConfigRepository";
import { User } from "../../domain/user/entities/User";
import { SyncRuleJobConfig } from "../../domain/scheduler/entities/SyncRuleJobConfig";
import { Namespace } from "../storage/Namespaces";
import { StorageDataStoreClient } from "../storage/StorageDataStoreClient";
import { Instance } from "../../domain/instance/entities/Instance";

export class SyncRuleJobConfigD2ApiRepository implements SyncRuleJobConfigRepository {
private dataStoreClient: StorageDataStoreClient;

constructor(private instance: Instance) {
Copy link
Contributor

Choose a reason for hiding this comment

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

currently in our projects, we are pass private api: D2Api to all repo constructors and then get the dataStoreClient from it. I am not sure why that is used instead of instance. We can check with Arnau and if that is recommended, shall we start using that pattern in all refactored repos?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code comment and task added for future implementation

this.dataStoreClient = new StorageDataStoreClient(this.instance);
}

public async getAll(currentUser: User): Promise<SyncRuleJobConfig[]> {
const synchRulesToBeScheduled = await this.getAllSynchronizationRules(currentUser);

const validSyncRuleJobConfigs = this.mapSynchronizationRulesToSyncRuleJobConfigs(synchRulesToBeScheduled);

return validSyncRuleJobConfigs;
}

private async getAllSynchronizationRules(currentUser: User): Promise<SynchronizationRule[]> {
const storedSynchronizationRuleData =
await this.dataStoreClient.listObjectsInCollection<SynchronizationRuleData>(Namespace.RULES);

const syncRules = storedSynchronizationRuleData.map(data => SynchronizationRule.build(data));

const synchRulesToBeScheduled = syncRules.filter(rule => rule.enabled);

const allowedSynchRulesToBeScheduled = synchRulesToBeScheduled.filter(
rule => currentUser.isGlobalAdmin || rule.isVisibleToUser(currentUser)
);

return allowedSynchRulesToBeScheduled;
}

private mapSynchronizationRulesToSyncRuleJobConfigs(syncRule: SynchronizationRule[]): SyncRuleJobConfig[] {
return syncRule.reduce((acc: SyncRuleJobConfig[], syncRule: SynchronizationRule): SyncRuleJobConfig[] => {
if (syncRule.frequency) {
return [
...acc,
{
id: syncRule.id,
name: syncRule.name,
frequency: syncRule.frequency,
},
];
} else {
return acc;
}
}, []);
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { SchedulerExecution } from "../../../domain/scheduler/entities/SchedulerExecution";
import { SchedulerExecutionInfo } from "../../../domain/scheduler/entities/SchedulerExecutionInfo";
import { Codec, Schema } from "../../../utils/codec";

export const SchedulerExecutionModel: Codec<SchedulerExecution> = Schema.object({
export const SchedulerExecutionInfoModel: Codec<SchedulerExecutionInfo> = Schema.object({
lastExecutionDuration: Schema.optional(Schema.number),
lastExecution: Schema.optional(Schema.date),
nextExecution: Schema.optional(Schema.date),
Expand Down
18 changes: 13 additions & 5 deletions src/domain/common/factories/RepositoryFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ import { GitHubRepositoryConstructor } from "../../packages/repositories/GitHubR
import { ReportsRepositoryConstructor } from "../../reports/repositories/ReportsRepository";
import { FileRulesRepositoryConstructor } from "../../rules/repositories/FileRulesRepository";
import { RulesRepositoryConstructor } from "../../rules/repositories/RulesRepository";
import { SchedulerRepositoryConstructor } from "../../scheduler/repositories/SchedulerRepository";
import { SchedulerExecutionInfoRepositoryConstructor } from "../../scheduler/repositories/SchedulerExecutionInfoRepositoryConstructor";
import { SyncRuleJobConfigRepositoryConstructor } from "../../scheduler/repositories/SyncRuleJobConfigRepository";
import { SettingsRepositoryConstructor } from "../../settings/SettingsRepository";
import { DownloadRepositoryConstructor } from "../../storage/repositories/DownloadRepository";
import { StoreRepositoryConstructor } from "../../stores/repositories/StoreRepository";
Expand Down Expand Up @@ -190,9 +191,10 @@ export class RepositoryFactory {
}

@cache()
public schedulerRepository(instance: Instance) {
const config = this.configRepository(instance);
return this.get<SchedulerRepositoryConstructor>(Repositories.SchedulerRepository, [config]);
public schedulerExecutionInfoRepository(instance: Instance) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to use this concept of repository-factory? I dont see it in our standard apps. We can create a separate PR to handle these "common" refactors, just making a note here for us.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code comment and task added for future implementation

return this.get<SchedulerExecutionInfoRepositoryConstructor>(Repositories.SchedulerExecutionInfoRepository, [
instance,
]);
}

@cache()
Expand All @@ -205,6 +207,11 @@ export class RepositoryFactory {
public dhisReleasesRepository() {
return this.get<DhisReleasesRepositoryConstructor>(Repositories.DhisReleasesRepository, []);
}

@cache()
public syncRuleJobConfigRepository(instance: Instance) {
return this.get<SyncRuleJobConfigRepositoryConstructor>(Repositories.SyncRuleJobConfigRepository, [instance]);
}
}

type RepositoryKeys = typeof Repositories[keyof typeof Repositories];
Expand All @@ -231,8 +238,9 @@ export const Repositories = {
UserRepository: "userRepository",
MappingRepository: "mappingRepository",
SettingsRepository: "settingsRepository",
SchedulerRepository: "schedulerRepository",
SchedulerExecutionInfoRepository: "schedulerExecutionInfoRepository",
DataStoreMetadataRepository: "dataStoreMetadataRepository",
DhisReleasesRepository: "dhisReleasesRepository",
TableColumnsRepository: "tableColumnsRepository",
SyncRuleJobConfigRepository: "syncRuleJobConfigRepository",
} as const;
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export interface SchedulerExecution {
export interface SchedulerExecutionInfo {
lastExecutionDuration?: number;
lastExecution?: Date;
nextExecution?: Date;
Expand Down
7 changes: 7 additions & 0 deletions src/domain/scheduler/entities/SyncRuleJobConfig.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export type SyncRuleJobConfig = {
id: string;
name: string;
frequency: string;
};

export const EVERY_MINUTE_FREQUENCY = "0 * * * * *";
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { Instance } from "../../instance/entities/Instance";
import { SchedulerExecutionInfo } from "../entities/SchedulerExecutionInfo";

export interface SchedulerExecutionInfoRepositoryConstructor {
Copy link
Contributor

Choose a reason for hiding this comment

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

Again this comment in for separate PR for "common" refactor - should we used this concept of Constructor ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code comment and task added for future implementation

new (instance: Instance): SchedulerExecutionInfoRepository;
}

export interface SchedulerExecutionInfoRepository {
updateExecutionInfo(executionInfo: SchedulerExecutionInfo): Promise<void>;
getLastExecutionInfo(): Promise<SchedulerExecutionInfo>;
}
11 changes: 0 additions & 11 deletions src/domain/scheduler/repositories/SchedulerRepository.ts

This file was deleted.

11 changes: 11 additions & 0 deletions src/domain/scheduler/repositories/SyncRuleJobConfigRepository.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { SyncRuleJobConfig } from "../entities/SyncRuleJobConfig";
import { User } from "../../user/entities/User";
import { Instance } from "../../instance/entities/Instance";

export interface SyncRuleJobConfigRepositoryConstructor {
new (instance: Instance): SyncRuleJobConfigRepository;
}

export interface SyncRuleJobConfigRepository {
getAll(currentUser: User): Promise<SyncRuleJobConfig[]>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { UseCase } from "../../common/entities/UseCase";
import { RepositoryFactory } from "../../common/factories/RepositoryFactory";
import { Instance } from "../../instance/entities/Instance";
import { SchedulerExecutionInfo } from "../entities/SchedulerExecutionInfo";

export class GetLastSchedulerExecutionInfoUseCase implements UseCase {
constructor(private repositoryFactory: RepositoryFactory, private localInstance: Instance) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

common refactor note - we dont pass the local instance to Use cases in our standard apps. Should we do away with this type of usage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code comment and task added for future implementation


public execute(): Promise<SchedulerExecutionInfo> {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use Future instead of Promise? If its too big a change, we can add a note for future refactor and take care of it in separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added new Future entity and changed from Promise to Future

return this.repositoryFactory.schedulerExecutionInfoRepository(this.localInstance).getLastExecutionInfo();
}
}
12 changes: 0 additions & 12 deletions src/domain/scheduler/usecases/GetLastSchedulerExecutionUseCase.ts

This file was deleted.

17 changes: 17 additions & 0 deletions src/domain/scheduler/usecases/GetSyncRuleJobConfigsUseCase.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { UseCase } from "../../common/entities/UseCase";
import { RepositoryFactory } from "../../common/factories/RepositoryFactory";
import { Instance } from "../../instance/entities/Instance";
import { SyncRuleJobConfig } from "../entities/SyncRuleJobConfig";

export class GetSyncRuleJobConfigsUseCase implements UseCase {
constructor(private repositoryFactory: RepositoryFactory, private localInstance: Instance) {}

public async execute(): Promise<SyncRuleJobConfig[]> {
const currentUser = await this.repositoryFactory.userRepository(this.localInstance).getCurrent();
const syncRuleJobConfigs = await this.repositoryFactory
.syncRuleJobConfigRepository(this.localInstance)
.getAll(currentUser);

return syncRuleJobConfigs;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { UseCase } from "../../common/entities/UseCase";
import { RepositoryFactory } from "../../common/factories/RepositoryFactory";
import { Instance } from "../../instance/entities/Instance";
import { SchedulerExecutionInfo } from "../entities/SchedulerExecutionInfo";

export class UpdateSchedulerExecutionInfoUseCase implements UseCase {
constructor(private repositoryFactory: RepositoryFactory, private localInstance: Instance) {}

public execute(executionInfo: SchedulerExecutionInfo): Promise<void> {
return this.repositoryFactory
.schedulerExecutionInfoRepository(this.localInstance)
.updateExecutionInfo(executionInfo);
}
}
19 changes: 13 additions & 6 deletions src/presentation/CompositionRoot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { GitHubOctokitRepository } from "../data/packages/GitHubOctokitRepositor
import { ReportsD2ApiRepository } from "../data/reports/ReportsD2ApiRepository";
import { FileRulesDefaultRepository } from "../data/rules/FileRulesDefaultRepository";
import { RulesD2ApiRepository } from "../data/rules/RulesD2ApiRepository";
import { SchedulerD2ApiRepository } from "../data/scheduler/SchedulerD2ApiRepository";
import { SchedulerExecutionInfoD2ApiRepository } from "../data/scheduler/SchedulerExecutionInfoD2ApiRepository";
Copy link
Contributor

Choose a reason for hiding this comment

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

common refactor note - We need to move to current way of implementing Composition root

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code comment and task added for future implementation

import { SettingsD2ApiRepository } from "../data/settings/SettingsD2ApiRepository";
import { DownloadWebRepository } from "../data/storage/DownloadWebRepository";
import { StoreD2ApiRepository } from "../data/stores/StoreD2ApiRepository";
Expand Down Expand Up @@ -100,8 +100,8 @@ import { GetSyncRuleUseCase } from "../domain/rules/usecases/GetSyncRuleUseCase"
import { ListSyncRuleUseCase } from "../domain/rules/usecases/ListSyncRuleUseCase";
import { ReadSyncRuleFilesUseCase } from "../domain/rules/usecases/ReadSyncRuleFilesUseCase";
import { SaveSyncRuleUseCase } from "../domain/rules/usecases/SaveSyncRuleUseCase";
import { GetLastSchedulerExecutionUseCase } from "../domain/scheduler/usecases/GetLastSchedulerExecutionUseCase";
import { UpdateLastSchedulerExecutionUseCase } from "../domain/scheduler/usecases/UpdateLastSchedulerExecutionUseCase";
import { GetLastSchedulerExecutionInfoUseCase } from "../domain/scheduler/usecases/GetLastSchedulerExecutionInfoUseCase";
import { UpdateSchedulerExecutionInfoUseCase } from "../domain/scheduler/usecases/UpdateSchedulerExecutionInfoUseCase";
import { GetSettingsUseCase } from "../domain/settings/GetSettingsUseCase";
import { SaveSettingsUseCase } from "../domain/settings/SaveSettingsUseCase";
import { DownloadFileUseCase } from "../domain/storage/usecases/DownloadFileUseCase";
Expand All @@ -125,6 +125,8 @@ import { DhisReleasesLocalRepository } from "../data/dhis-releases/DhisReleasesL
import { GetColumnsUseCase } from "../domain/table-columns/usecases/GetColumnsUseCase";
import { SaveColumnsUseCase } from "../domain/table-columns/usecases/SaveColumnsUseCase";
import { TableColumnsDataStoreRepository } from "../data/table-columns/TableColumnsDataStoreRepository";
import { GetSyncRuleJobConfigsUseCase } from "../domain/scheduler/usecases/GetSyncRuleJobConfigsUseCase";
import { SyncRuleJobConfigD2ApiRepository } from "../data/scheduler/SyncRuleJobConfigD2ApiRepository";
import { getD2APiFromInstance } from "../utils/d2-utils";
import { RoleD2ApiRepository } from "../data/role/RoleD2ApiRepository";
import { ValidateRolesUseCase } from "../domain/role/ValidateRolesUseCase";
Expand Down Expand Up @@ -155,11 +157,15 @@ export class CompositionRoot {
this.repositoryFactory.bind(Repositories.MetadataRepository, MetadataJSONRepository, "json");
this.repositoryFactory.bind(Repositories.TransformationRepository, TransformationD2ApiRepository);
this.repositoryFactory.bind(Repositories.MappingRepository, MappingD2ApiRepository);
this.repositoryFactory.bind(Repositories.SchedulerRepository, SchedulerD2ApiRepository);
this.repositoryFactory.bind(
Repositories.SchedulerExecutionInfoRepository,
SchedulerExecutionInfoD2ApiRepository
);
this.repositoryFactory.bind(Repositories.SettingsRepository, SettingsD2ApiRepository);
this.repositoryFactory.bind(Repositories.DataStoreMetadataRepository, DataStoreMetadataD2Repository);
this.repositoryFactory.bind(Repositories.DhisReleasesRepository, DhisReleasesLocalRepository);
this.repositoryFactory.bind(Repositories.TableColumnsRepository, TableColumnsDataStoreRepository);
this.repositoryFactory.bind(Repositories.SyncRuleJobConfigRepository, SyncRuleJobConfigD2ApiRepository);
}

@cache()
Expand Down Expand Up @@ -405,8 +411,9 @@ export class CompositionRoot {
@cache()
public get scheduler() {
return getExecute({
getLastExecution: new GetLastSchedulerExecutionUseCase(this.repositoryFactory, this.localInstance),
updateLastExecution: new UpdateLastSchedulerExecutionUseCase(this.repositoryFactory, this.localInstance),
getLastExecutionInfo: new GetLastSchedulerExecutionInfoUseCase(this.repositoryFactory, this.localInstance),
updateExecutionInfo: new UpdateSchedulerExecutionInfoUseCase(this.repositoryFactory, this.localInstance),
getSyncRuleJobConfigs: new GetSyncRuleJobConfigsUseCase(this.repositoryFactory, this.localInstance),
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import FiberManualRecordIcon from "@material-ui/icons/FiberManualRecord";
import React, { useCallback, useEffect, useState } from "react";
import styled from "styled-components";
import { SchedulerExecution } from "../../../../../domain/scheduler/entities/SchedulerExecution";
import { SchedulerExecutionInfo } from "../../../../../domain/scheduler/entities/SchedulerExecutionInfo";
import i18n from "../../../../../locales";
import { useAppContext } from "../../contexts/AppContext";

Expand All @@ -15,10 +15,10 @@ export const SchedulerInfo: React.FC<SchedulerInfoProps> = React.memo(props => {
const [status, setStatus] = useState<boolean>(false);

const getSchedulerInfo = useCallback(() => {
compositionRoot.scheduler.getLastExecution().then(execution => {
const timestamp = execution?.lastExecution?.toISOString() ?? "";
compositionRoot.scheduler.getLastExecutionInfo().then(lastExecutionInfo => {
const timestamp = lastExecutionInfo?.lastExecution?.toISOString() ?? "";
if (onSchedulerRun) onSchedulerRun(timestamp);
return setStatus(isRunning(execution));
return setStatus(isRunning(lastExecutionInfo));
});
}, [compositionRoot, onSchedulerRun]);

Expand Down Expand Up @@ -49,6 +49,6 @@ const SchedulerContainer = styled.div`
gap: 10px;
`;

function isRunning(info?: SchedulerExecution): boolean {
function isRunning(info?: SchedulerExecutionInfo): boolean {
return !!info?.nextExecution && info.nextExecution >= new Date();
}
2 changes: 1 addition & 1 deletion src/presentation/webapp/WebApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ const App = () => {
</div>

<Share visible={showShareButton} />
{appConfig && <Feedback options={appConfig.feedback} username={username} />}
{appConfig?.feedback && <Feedback options={appConfig.feedback} username={username} />}
</SnackbarProvider>
</LoadingProvider>
</MuiThemeProvider>
Expand Down
Loading
Loading