Skip to content

BN-37 | Add. Clinical Config Loader #8

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Apr 22, 2025
Merged

BN-37 | Add. Clinical Config Loader #8

merged 12 commits into from
Apr 22, 2025

Conversation

@rahu1ramesh
Copy link
Contributor

@rahu1ramesh rahu1ramesh commented Apr 15, 2025 •
edited
Loading

JIRA -> BN-37

This PR adds functionality to load clinical configuration from the pre configured source of configuration.

Key Features

  • Fetches clinical Configuration
  • Validates the clinical configuration against a json schema.

Implementation Details

  • useClinicalConfig Hook: Fetches config data from the config folder based on specified endpoint.
  • Config Service: Fetch configuration from endpoint and validate it against a schema.
  • Clinical Config Provider: ClinicalConfigProvider provides application configuration data throughout the application via React Context, handling loading states, errors, and making the configuration accessible to all child components.
  • Testing & Type Definitions: Includes unit tests, integration tests, and TypeScript type definitions.

Next Steps

  • A separate card will be created to use the configuration in the dashboard.

Preview
Screenshot 2025-04-21 at 11 30 03 PM

@rahu1ramesh rahu1ramesh changed the title BN_ BN-37 | Add. Config Loader Apr 15, 2025
@rahu1ramesh rahu1ramesh requested a review from mohan-13 April 15, 2025 14:17
rahu1ramesh
rahu1ramesh commented Apr 18, 2025
View reviewed changes
src/schemas/appConfig.schema.json Outdated
"properties": {
"patientInformation": {
"type": "object",
"required": ["translationKey", "type"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove required and types

src/schemas/appConfig.schema.json Outdated
"type": "object",
"required": ["id", "name", "description", "url", "requiredPrivilege"],
"properties": {
"id": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove

src/schemas/appConfig.schema.json Outdated
"type": "string",
"description": "URL to access the dashboard data"
},
"requiredPrivilege": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove string and add array

mohan-13
mohan-13 reviewed Apr 18, 2025
View reviewed changes
src/constants/dashboard.ts Outdated
export const DEPARTMENT_CONFIG_URL = (departmentURL: string) =>
`/bahmni_config/openmrs/apps/clinical/dashboards/${departmentURL}`;

export const DASHBOARD_CONFIG_URL =
Copy link
Member

Choose a reason for hiding this comment

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

Please call this as CLINICAL_CONFIG_URL and the above one as DASHBOARD_CONFIG_URL

src/constants/errors.ts Outdated
export const ERROR_TITLES = {
CONFIG_ERROR: 'Configuration Error',
VALIDATION_ERROR: 'Validation Error',
DEPARTMENT_ERROR: 'Department Configuration Error',
Copy link
Member

Choose a reason for hiding this comment

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

Department Configuration error could be confusing

src/hooks/__tests__/useConfig.test.tsx Outdated
import React from 'react';
import { renderHook } from '@testing-library/react';
import { useConfig } from '../useConfig';
import { ConfigContextType } from '../../types/config';
Copy link
Member

Choose a reason for hiding this comment

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

Use @types ??

src/providers/ConfigProvider.tsx Outdated
) {
const configMap: Record<string, string> = {};
dashboardConfig.dashboards.forEach((dashboard) => {
const departmentID = dashboard.id;
Copy link
Member

Choose a reason for hiding this comment

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

there is no id for a dashboard

src/providers/ConfigProvider.tsx Outdated
const departmentURL = dashboard.url;
configMap[departmentID] = departmentURL;
});
setConfig(configMap);
Copy link
Member

Choose a reason for hiding this comment

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

why set only dashboard URL references only? why not have our entire config in context ?

src/types/config.ts Outdated
};
actions: Array<Record<string, unknown>>;
dashboards: Array<{
id: string;
Copy link
Member

Choose a reason for hiding this comment

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

Id to be removed

src/providers/ConfigProvider.tsx Outdated
});
setConfig(configMap);
} else {
const error = new Error(CONFIG_ERROR_MESSAGES.NO_DASHBOARDS);
Copy link
Member

Choose a reason for hiding this comment

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

NO dashboards should not be an error by ConfigProvider, it should be the responsibility of the reading component to display error or an info message to the user

src/__mocks__/configMocks.ts Outdated

// Happy Path Mocks
export const validFullConfig = {
section: [
Copy link
Member

Choose a reason for hiding this comment

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

The mocks seems to be different from our actual config which doesn't have sections. please update

@rahu1ramesh rahu1ramesh requested a review from mohan-13 April 21, 2025 17:57
@rahu1ramesh rahu1ramesh changed the title BN-37 | Add. Config Loader BN-37 | Add. Dashboard Config Loader Apr 21, 2025
@rahu1ramesh rahu1ramesh changed the title BN-37 | Add. Dashboard Config Loader BN-37 | Add. Clinical Config Loader Apr 21, 2025
@mohan-13 mohan-13 merged commit 02c27b7 into main Apr 22, 2025
2 checks passed
@mohan-13 mohan-13 deleted the BN-37 branch April 22, 2025 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mohan-13 mohan-13 mohan-13 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rahu1ramesh @mohan-13