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

feat(awell): read data point value #570

Merged
merged 2 commits into from
Jan 28, 2025

Conversation

nckhell
Copy link
Contributor

@nckhell nckhell commented Jan 28, 2025

No description provided.

Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Typo

The key for valueNumbersArray is incorrectly set to valueStringsArray. This might lead to unexpected behavior when accessing this data point.

valueNumbersArray: {
  key: 'valueStringsArray',
  valueType: 'strings_array',
Error Handling

The error handling for the onError callback does not seem to account for cases where the SDK query itself fails (e.g., network issues or invalid responses). This could lead to unhandled exceptions.

  const resp = await sdk.orchestration.query({
    pathwayDataPoints: {
      __args: {
        pathway_id: careFlowId,
        data_point_definition_id: dataPointDefinitionId,
      },
      success: true,
      dataPoints: {
        id: true,
        key: true,
        serialized_value: true,
        data_point_definition_id: true,
        valueType: true,
        date: true,
      },
    },
  })

  const dataPoints = resp.pathwayDataPoints.dataPoints

  if (dataPoints.length === 0) {
    await onError({
      events: [
        addActivityEventLog({
          message:
            'No data point values were found for the given data point definition id.',
        }),
      ],
    })
    return
  }

  /**
   * If a single data point definition has multiple values, there will be
   * multiple entries in the dataPoints array. We need to get the most recent
   */
  const mostRecentDataPoint = dataPoints.sort((a, b) => {
    return new Date(b.date).getTime() - new Date(a.date).getTime()
  })[0]

  await onComplete({
    data_points: {
      valueString:
        mostRecentDataPoint.valueType === enumDataPointValueType.STRING
          ? mostRecentDataPoint.serialized_value
          : undefined,
      valueNumber:
        mostRecentDataPoint.valueType === enumDataPointValueType.NUMBER
          ? mostRecentDataPoint.serialized_value
          : undefined,
      valueDate:
        mostRecentDataPoint.valueType === enumDataPointValueType.DATE
          ? mostRecentDataPoint.serialized_value
          : undefined,
      valueJson:
        mostRecentDataPoint.valueType === enumDataPointValueType.JSON
          ? mostRecentDataPoint.serialized_value
          : undefined,
      valueBoolean:
        mostRecentDataPoint.valueType === enumDataPointValueType.BOOLEAN
          ? mostRecentDataPoint.serialized_value
          : undefined,
      valueTelephone:
        mostRecentDataPoint.valueType === enumDataPointValueType.TELEPHONE
          ? mostRecentDataPoint.serialized_value
          : undefined,
      valueStringsArray:
        mostRecentDataPoint.valueType === enumDataPointValueType.STRINGS_ARRAY
          ? mostRecentDataPoint.serialized_value
          : undefined,
      valueNumbersArray:
        mostRecentDataPoint.valueType === enumDataPointValueType.NUMBERS_ARRAY
          ? mostRecentDataPoint.serialized_value
          : undefined,
    },
  })
},
Test Coverage

While the tests cover various data point types, there is no test for the valueStringsArray or valueNumbersArray data point types. Adding these tests would ensure complete coverage.

import { TestHelpers } from '@awell-health/extensions-core'
import { generateTestPayload } from '../../../../../tests'
import { getDataPointValue as action } from './getDataPointValue'
import {
  getDataPointStringValueMock,
  getDataPointBooleanValueMock,
  getDataPointDateValueMock,
  getDataPointJsonValueMock,
  getDataPointNumberValueMock,
  getDataPointTelephoneValueMock,
  getDataPointNumericValueWithMultipleValuesMock,
} from './__testdata__/getDataPointValue.mock'

jest.mock('@awell-health/awell-sdk')

describe('Awell Extension - Get Data Point Value', () => {
  const { onComplete, onError, helpers, extensionAction, clearMocks } =
    TestHelpers.fromAction(action)

  beforeEach(() => {
    clearMocks()
  })

  describe('when at least one data point value is found for the given data point definition id', () => {
    describe('when the data point is a string', () => {
      it('should call the onComplete callback with the data point value', async () => {
        const awellSdkMock = {
          orchestration: {
            query: jest.fn().mockResolvedValue(getDataPointStringValueMock),
          },
        }

        helpers.awellSdk = jest.fn().mockResolvedValue(awellSdkMock)

        await extensionAction.onEvent({
          payload: generateTestPayload({
            fields: {
              careFlowId: 'care-flow-id',
              dataPointDefinitionId: 'data-point-definition-id',
            },
            settings: {},
          }),
          onComplete,
          onError,
          helpers,
        })

        expect(onComplete).toHaveBeenCalledWith({
          data_points: {
            valueString: 'string',
            valueBoolean: undefined,
            valueNumber: undefined,
            valueDate: undefined,
            valueTelephone: undefined,
            valueJson: undefined,
            valueStringsArray: undefined,
            valueNumbersArray: undefined,
          },
        })
        expect(onError).not.toHaveBeenCalled()
      })
    })

    describe('when the data point is a number', () => {
      it('should call the onComplete callback with the data point value', async () => {
        const awellSdkMock = {
          orchestration: {
            query: jest.fn().mockResolvedValue(getDataPointNumberValueMock),
          },
        }

        helpers.awellSdk = jest.fn().mockResolvedValue(awellSdkMock)

        await extensionAction.onEvent({
          payload: generateTestPayload({
            fields: {
              careFlowId: 'care-flow-id',
              dataPointDefinitionId: 'data-point-definition-id',
            },
            settings: {},
          }),
          onComplete,
          onError,
          helpers,
        })

        expect(onComplete).toHaveBeenCalledWith({
          data_points: {
            valueString: undefined,
            valueBoolean: undefined,
            valueNumber: '1',
            valueDate: undefined,
            valueTelephone: undefined,
            valueJson: undefined,
            valueStringsArray: undefined,
            valueNumbersArray: undefined,
          },
        })
        expect(onError).not.toHaveBeenCalled()
      })
    })

    describe('when the data point is a date', () => {
      it('should call the onComplete callback with the data point value', async () => {
        const awellSdkMock = {
          orchestration: {
            query: jest.fn().mockResolvedValue(getDataPointDateValueMock),
          },
        }

        helpers.awellSdk = jest.fn().mockResolvedValue(awellSdkMock)

        await extensionAction.onEvent({
          payload: generateTestPayload({
            fields: {
              careFlowId: 'care-flow-id',
              dataPointDefinitionId: 'data-point-definition-id',
            },
            settings: {},
          }),
          onComplete,
          onError,
          helpers,
        })

        expect(onComplete).toHaveBeenCalledWith({
          data_points: {
            valueString: undefined,
            valueBoolean: undefined,
            valueNumber: undefined,
            valueDate: '2025-01-28T00:00:00.000Z',
            valueTelephone: undefined,
            valueJson: undefined,
            valueStringsArray: undefined,
            valueNumbersArray: undefined,
          },
        })
        expect(onError).not.toHaveBeenCalled()
      })
    })

    describe('when the data point is a boolean', () => {
      it('should call the onComplete callback with the data point value', async () => {
        const awellSdkMock = {
          orchestration: {
            query: jest.fn().mockResolvedValue(getDataPointBooleanValueMock),
          },
        }

        helpers.awellSdk = jest.fn().mockResolvedValue(awellSdkMock)

        await extensionAction.onEvent({
          payload: generateTestPayload({
            fields: {
              careFlowId: 'care-flow-id',
              dataPointDefinitionId: 'data-point-definition-id',
            },
            settings: {},
          }),
          onComplete,
          onError,
          helpers,
        })

        expect(onComplete).toHaveBeenCalledWith({
          data_points: {
            valueString: undefined,
            valueBoolean: 'true',
            valueNumber: undefined,
            valueDate: undefined,
            valueTelephone: undefined,
            valueJson: undefined,
            valueStringsArray: undefined,
            valueNumbersArray: undefined,
          },
        })
        expect(onError).not.toHaveBeenCalled()
      })
    })

    describe('when the data point is a telephone', () => {
      it('should call the onComplete callback with the data point value', async () => {
        const awellSdkMock = {
          orchestration: {
            query: jest.fn().mockResolvedValue(getDataPointTelephoneValueMock),
          },
        }

        helpers.awellSdk = jest.fn().mockResolvedValue(awellSdkMock)

        await extensionAction.onEvent({
          payload: generateTestPayload({
            fields: {
              careFlowId: 'care-flow-id',
              dataPointDefinitionId: 'data-point-definition-id',
            },
            settings: {},
          }),
          onComplete,
          onError,
          helpers,
        })

        expect(onComplete).toHaveBeenCalledWith({
          data_points: {
            valueString: undefined,
            valueBoolean: undefined,
            valueNumber: undefined,
            valueDate: undefined,
            valueTelephone: '+32476581696',
            valueJson: undefined,
            valueStringsArray: undefined,
            valueNumbersArray: undefined,
          },
        })
        expect(onError).not.toHaveBeenCalled()
      })
    })

    describe('when the data point is json', () => {
      it('should call the onComplete callback with the data point value', async () => {
        const awellSdkMock = {
          orchestration: {
            query: jest.fn().mockResolvedValue(getDataPointJsonValueMock),
          },
        }

        helpers.awellSdk = jest.fn().mockResolvedValue(awellSdkMock)

        await extensionAction.onEvent({
          payload: generateTestPayload({
            fields: {
              careFlowId: 'care-flow-id',
              dataPointDefinitionId: 'data-point-definition-id',
            },
            settings: {},
          }),
          onComplete,
          onError,
          helpers,
        })

        expect(onComplete).toHaveBeenCalledWith({
          data_points: {
            valueString: undefined,
            valueBoolean: undefined,
            valueNumber: undefined,
            valueDate: undefined,
            valueTelephone: undefined,
            valueJson: '{"a":"hello"}',
            valueStringsArray: undefined,
            valueNumbersArray: undefined,
          },
        })
        expect(onError).not.toHaveBeenCalled()
      })
    })
  })

  describe('when multiple data point values are found for the given data point definition id', () => {
    it('should call the onComplete callback with the most recent data point value', async () => {
      const awellSdkMock = {
        orchestration: {
          query: jest
            .fn()
            .mockResolvedValue(getDataPointNumericValueWithMultipleValuesMock),
        },
      }

      helpers.awellSdk = jest.fn().mockResolvedValue(awellSdkMock)

      await extensionAction.onEvent({
        payload: generateTestPayload({
          fields: {
            careFlowId: 'care-flow-id',
            dataPointDefinitionId: 'data-point-definition-id',
          },
          settings: {},
        }),
        onComplete,
        onError,
        helpers,
      })

      expect(onComplete).toHaveBeenCalledWith({
        data_points: {
          valueString: undefined,
          valueBoolean: undefined,
          valueNumber: '2',
          valueDate: undefined,
          valueTelephone: undefined,
          valueJson: undefined,
          valueStringsArray: undefined,
          valueNumbersArray: undefined,
        },
      })
      expect(onError).not.toHaveBeenCalled()
    })
  })

  describe('when no data point value is found for the given data point definition id', () => {
    test('should call the onError callback', async () => {
      const awellSdkMock = {
        orchestration: {
          query: jest.fn().mockResolvedValue({
            pathwayDataPoints: {
              success: true,
              dataPoints: [],
            },
          }),
        },
      }

      helpers.awellSdk = jest.fn().mockResolvedValue(awellSdkMock)

      await extensionAction.onEvent({
        payload: generateTestPayload({
          fields: {
            careFlowId: 'care-flow-id',
            dataPointDefinitionId: 'data-point-definition-id',
          },
          settings: {},
        }),
        onComplete,
        onError,
        helpers,
      })

      expect(onComplete).not.toHaveBeenCalled()
      expect(onError).toHaveBeenCalledWith({
        events: expect.arrayContaining([
          {
            date: expect.any(String),
            text: {
              en: 'No data point values were found for the given data point definition id.',
            },
          },
        ]),
      })
    })
  })
})

Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Possible issue
Handle missing or invalid date fields

Add error handling for cases where the date field in dataPoints is missing or
invalid to prevent runtime errors during sorting.

extensions/awell/v1/actions/getDataPointValue/getDataPointValue.ts [65-67]

-const mostRecentDataPoint = dataPoints.sort((a, b) => {
-  return new Date(b.date).getTime() - new Date(a.date).getTime()
-})[0]
+const mostRecentDataPoint = dataPoints
+  .filter(dp => dp.date)
+  .sort((a, b) => new Date(b.date).getTime() - new Date(a.date).getTime())[0]
Suggestion importance[1-10]: 10

Why: Adding error handling for missing or invalid date fields is critical to prevent runtime errors during sorting. This change significantly improves the robustness of the code and ensures it handles edge cases gracefully.

10
Fix incorrect key and type assignment

The valueNumbersArray key is incorrectly assigned the same value type as
valueStringsArray. Update the valueType for valueNumbersArray to correctly reflect
its intended type.

extensions/awell/v1/actions/getDataPointValue/config/dataPoints.ts [32-34]

 valueNumbersArray: {
-  key: 'valueStringsArray',
-  valueType: 'strings_array',
+  key: 'valueNumbersArray',
+  valueType: 'numbers_array',
 },
Suggestion importance[1-10]: 9

Why: The suggestion correctly identifies and fixes an issue where the valueNumbersArray key and type are incorrectly assigned, which could lead to functional errors. This change ensures the data structure is accurate and aligns with its intended purpose.

9
General
Test handling of invalid date fields

Add a test case to verify behavior when the dataPoints array contains entries with
missing or invalid date fields, ensuring the sorting logic handles such cases
gracefully.

extensions/awell/v1/actions/getDataPointValue/getDataPointValue.test.ts [260-268]

 describe('when multiple data point values are found for the given data point definition id', () => {
   it('should call the onComplete callback with the most recent data point value', async () => {
     const awellSdkMock = {
       orchestration: {
         query: jest
           .fn()
-          .mockResolvedValue(getDataPointNumericValueWithMultipleValuesMock),
+          .mockResolvedValue({
+            ...getDataPointNumericValueWithMultipleValuesMock,
+            pathwayDataPoints: {
+              ...getDataPointNumericValueWithMultipleValuesMock.pathwayDataPoints,
+              dataPoints: [
+                ...getDataPointNumericValueWithMultipleValuesMock.pathwayDataPoints.dataPoints,
+                { id: 'invalid', date: null, serialized_value: '3', valueType: 'NUMBER' },
+              ],
+            },
+          }),
       },
     }
Suggestion importance[1-10]: 9

Why: Adding a test case for handling invalid date fields enhances test coverage and ensures the sorting logic is robust against such scenarios. This is a proactive measure to maintain code quality and reliability.

9
Validate serialized_value format consistency

Validate the serialized_value field to ensure it matches the expected format for its
valueType, preventing potential type mismatches or incorrect data handling.

extensions/awell/v1/actions/getDataPointValue/getDataPointValue.ts [75-77]

 valueNumber:
-  mostRecentDataPoint.valueType === enumDataPointValueType.NUMBER
+  mostRecentDataPoint.valueType === enumDataPointValueType.NUMBER &&
+  !isNaN(Number(mostRecentDataPoint.serialized_value))
     ? mostRecentDataPoint.serialized_value
     : undefined,
Suggestion importance[1-10]: 8

Why: Validating the serialized_value field ensures data integrity and prevents potential type mismatches. This is a valuable improvement for maintaining consistency and reliability in data handling.

8

@nckhell nckhell merged commit 1a74ecd into main Jan 28, 2025
2 checks passed
@nckhell nckhell deleted the awell_extension_read_datapoint_value branch January 28, 2025 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant