Add settings section to configure dataSet #5
Conversation
…feature/generic-config-CU-869bd706j
There was a problem hiding this comment.
Pull request overview
This pull request adds a comprehensive DataSet configuration system that allows administrators to manage dataset approval workflows through a UI. The implementation replaces hardcoded configuration files with a dynamic dataStore-based solution.
Key changes:
- Introduces DataSet configuration management (CRUD operations) accessible only to super admins
- Implements granular permission system for dataset actions (read, complete, incomplete, revoke, submit, approve) based on users and user groups
- Adds custom workflow logic for submit/complete and revoke/incomplete actions
- Migrates from static configuration files to dynamic dataStore-based configuration
Reviewed changes
Copilot reviewed 71 out of 73 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock, package.json | Updates react-router-dom to 6.30.2, adds real-cancellable-promise and typed-immutable-map dependencies |
| src/domain/entities/DataSetConfiguration.ts | New entity representing dataset configuration with validation and permission checking logic |
| src/domain/usecases/GetApprovalConfigurationsUseCase.ts | Use case to retrieve dataset configurations with permissions for current user |
| src/data/DataSetConfigurationD2Repository.ts | Repository implementation for storing/retrieving configurations from dataStore |
| src/webapp/pages/*.tsx | New pages for managing dataset configurations (list, add, edit) |
| src/webapp/components/dataset-config/*.tsx | UI components for dataset configuration table and form |
| src/data/common/Dhis2ConfigRepository.ts | Removes hardcoded approval settings, simplifies configuration loading |
| src/domain/reports/mal-data-approval/usecases/*.ts | Updates use cases to use dynamic configurations instead of static settings |
| src/data/reports/mal-data-approval/MalDataApprovalDefaultRepository.ts | Refactors to use dataSet configurations for approval operations |
| src/webapp/reports/Reports.tsx | Adds routing for dataset settings pages with loader pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/data/UserD2Repository.ts
Outdated
| return new User({ | ||
| id: d2User.id, | ||
| name: d2User.displayName, | ||
| userGroups: d2User.userGroups, | ||
| ...d2User.userCredentials, | ||
| isSuperAdmin: true, |
There was a problem hiding this comment.
The hard-coded value 'true' should be replaced with a proper check for whether the user is a super admin. Currently, all users will be marked as super admin.
| return new User({ | |
| id: d2User.id, | |
| name: d2User.displayName, | |
| userGroups: d2User.userGroups, | |
| ...d2User.userCredentials, | |
| isSuperAdmin: true, | |
| const isSuperAdmin = | |
| !!d2User.userCredentials && | |
| Array.isArray(d2User.userCredentials.userRoles) && | |
| d2User.userCredentials.userRoles.some( | |
| role => Array.isArray(role.authorities) && role.authorities.includes("ALL") | |
| ); | |
| return new User({ | |
| id: d2User.id, | |
| name: d2User.displayName, | |
| userGroups: d2User.userGroups, | |
| ...d2User.userCredentials, | |
| isSuperAdmin, |
| organisationUnits: { id: true }, | ||
| }, | ||
| filter: { code: { in: dataSetCodes } }, | ||
| filter: { code: { in: [] } }, |
There was a problem hiding this comment.
The filter with an empty array filter: { code: { in: [] } } will always return no results. This appears to be a placeholder that should either use the malDataSetCodes or be removed if dataSets are now configured differently.
| filter: { code: { in: [] } }, | |
| filter: { code: { in: malDataSetCodes } }, |
| // const settings = await this.getSettingByDataSet([originalDataSetId]); | ||
| // const dataSetSettings = settings.find(setting => setting.dataSetId === originalDataSetId); | ||
| // if (!dataSetSettings) throw new Error(`Data set settings not found: ${originalDataSetId}`); |
There was a problem hiding this comment.
The commented-out code should be removed rather than left in the codebase. If this logic is no longer needed, it should be deleted to improve code cleanliness.
| // const settings = await this.getSettingByDataSet([originalDataSetId]); | |
| // const dataSetSettings = settings.find(setting => setting.dataSetId === originalDataSetId); | |
| // if (!dataSetSettings) throw new Error(`Data set settings not found: ${originalDataSetId}`); |
| ): Promise<boolean> { | ||
| try { | ||
| const { approvalDataSetId, dataSetId } = await this.getApprovalDataSetId(dataValues); | ||
| // const { approvalDataSetId } = await this.getApprovalDataSetId(dataValues); |
There was a problem hiding this comment.
The commented-out code should be removed rather than left in the codebase. This appears to be old logic that has been replaced.
| // const { approvalDataSetId } = await this.getApprovalDataSetId(dataValues); |
| import { promiseMap } from "../utils/promises"; | ||
| import { WmrDiffReport } from "../domain/reports/WmrDiffReport"; | ||
| import { AppSettingsD2Repository } from "../data/AppSettingsD2Repository"; | ||
| import { dataSetApprovalName, WmrDiffReport } from "../domain/reports/WmrDiffReport"; |
There was a problem hiding this comment.
Unused import dataSetApprovalName.
📌 References
📝 Implementation
📹 Screenshots/Screen capture
List dataSet configurations
Create/Edit dataSet configuration Form
Delete a dataSet configuration
🔥 Notes to the tester
dataset-approval#869bd706j