Skip to content

Add data util method to allow adding locally stored data #584

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

Open
clintonium-119 wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Add data util method to allow adding locally stored data #584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2373f62
Merge pull request #575 from tidepool-org/release-1.53.1
henry-tp Jan 16, 2026
0ec041c
Add dataUtil.addLocalData method to allow adding locally downloaded d…
clintonium-119 Jan 21, 2026
17e3e6d
v1.54.0-local-data-source.1
clintonium-119 Jan 21, 2026
85121a8
Update src/utils/DataUtil.js
clintonium-119 Jan 22, 2026
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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"node": "20.8.0"
},
"packageManager": "yarn@3.6.4",
"version": "1.54.0-web-4274-copy-billing.6",
"version": "1.54.0-local-data-source.1",
"description": "Tidepool data visualization for diabetes device data.",
"keywords": [
"data visualization"
Expand Down
27 changes: 27 additions & 0 deletions src/utils/DataUtil.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,33 @@ export class DataUtil {
this.endTimer('init total');
};

Copilot AI Jan 22, 2026

Copy link

Choose a reason for hiding this comment

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

The new addLocalData method lacks JSDoc documentation. Given that other methods in the DataUtil class have JSDoc comments (as seen in the constructor), this method should include documentation explaining its parameters (especially the localDataSource parameter which has specific behavior), return value, and purpose. This is particularly important since this is a new public API.

Suggested change
/**
* Loads local raw Tidepool data from a JSON fixture and forwards it to {@link addData}
* for normalization and indexing.
*
* The source of the local data can affect how basal durations are interpreted:
* when {@code localDataSource} is set to {@code 'export'}, basal records with a
* duration less than {@code 1000} are treated as minutes and converted to milliseconds.
* For any other value (or when omitted), durations are assumed to already be in
* milliseconds and are left unchanged.
*
* @param {string} patientId - Identifier of the patient the local data belongs to.
* @param {boolean} [returnData=false] - When {@code true}, returns the normalized data
* from {@link addData}; when {@code false}, performs the load as a side effect only.
* @param {('export'|undefined)} [localDataSource] - Indicates the origin/format of the
* local data. Use {@code 'export'} when loading data from a Tidepool export so basal
* durations shorter than {@code 1000} are interpreted as minutes.
* @returns {*} The value returned by {@link addData}, typically the normalized dataset
* when {@code returnData} is {@code true}; otherwise may be {@code undefined}.
*/

Copilot uses AI. Check for mistakes.
addLocalData = (patientId, returnData = false, localDataSource) => {

Copilot AI Jan 22, 2026

Copy link

Choose a reason for hiding this comment

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

The localDataSource parameter name is ambiguous and doesn't clearly indicate its purpose. The parameter only accepts the value 'export' to enable special export-related processing, but the name suggests it might specify the source location of local data. Consider renaming to something more descriptive like isExportFormat or exportDataFormat to better communicate its purpose.

Copilot uses AI. Check for mistakes.
let data;
try {
// eslint-disable-next-line global-require, import/no-unresolved
data = require('../../local/rawData.json');
let dataSource = localDataSource === 'export' ? 'the Tidepool export service' : 'the Tidepool API';

Copilot AI Jan 22, 2026

Copy link

Choose a reason for hiding this comment

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

The dataSource variable initialized on line 117 is declared but then immediately overwritten in the conditional blocks on lines 121 and 124. This makes the initial assignment on line 117 unnecessary and potentially confusing. Consider removing the initial assignment and declaring dataSource with let without initialization, or restructure the logic to avoid the unnecessary assignment.

Copilot uses AI. Check for mistakes.

if (data?.data?.current?.data) {
data = _.flatten(_.values(data.data.current.data));
dataSource = 'a Tidepool Web console export';
} else if (data?.[0].dataset) {
data = _.flatten(_.map(data, v => v.data));
dataSource = 'a Tidepool Account Tool export';
}

this.log(`Loading dataset provided by ${dataSource}`);
} catch (e) {
data = { data: [] };

Copilot AI Jan 22, 2026

Copy link

Choose a reason for hiding this comment

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

The error handling sets data to an object { data: [] } instead of an array. This will cause issues when the data is processed later, as the code expects data to be an array. The subsequent check data?.[0].dataset on line 122 will fail (since data[0] would be undefined for this object), and the conditional on line 132 expects data to be an array for _.map. The catch block should set data = [] instead to match the expected type.

Suggested change
data = { data: [] };
data = [];

Copilot uses AI. Check for mistakes.
}

if (localDataSource === 'export') {
data = _.map(data, d => ({ ...d, duration: d.type === 'basal' && d.duration < 1000 ? d.duration * MS_IN_MIN : d.duration }));
}

return this.addData(data, patientId, returnData);
};
Comment on lines +112 to +137

Copilot AI Jan 22, 2026

Copy link

Choose a reason for hiding this comment

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

The new addLocalData method lacks test coverage. The repository has comprehensive tests for DataUtil (as seen in test/utils/DataUtil.test.js with 6454 lines), and other methods like addData are thoroughly tested. This new method should have corresponding test cases covering different scenarios: successful data loading from different formats (Tidepool Web console export, Tidepool Account Tool export, direct API format), error handling when the file doesn't exist, and the export format duration conversion logic.

Copilot uses AI. Check for mistakes.

addData = (rawData = [], patientId, returnData = false) => {
this.startTimer('addData');

Expand Down
21 changes: 1 addition & 20 deletions storybook/preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,9 @@ import React from 'react';

import DataUtil from '../src/utils/DataUtil';

let data;
try {
// eslint-disable-next-line global-require, import/no-unresolved
data = require('../local/rawData.json');
let dataSource = 'the Tidepool API';

if (data?.data?.current?.data) {
data = _.flatten(_.values(data.data.current.data));
dataSource = 'a Tidepool Web console export';
} else if (data?.[0].dataset) {
data = _.flatten(_.map(data, v => v.data));
dataSource = 'a Tidepool Account Tool export';
}

console.log(`Loading dataset provided by ${dataSource}`);
} catch (e) {
data = { data: [] };
}

const patientId = 'abc123';
const dataUtil = new DataUtil();
dataUtil.addData(data, patientId);
dataUtil.addLocalData(patientId);

const props = {
dataUtil,
Expand Down