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.

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

[BUG]Csv report generation had missing nested Fields #502

Merged
merged 11 commits into from
Jan 28, 2025
Merged

[BUG]Csv report generation had missing nested Fields #502

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
47b6b0d
csv report generation added nested fields
sumukhswamy Jan 18, 2025
9880e7c
csv report generation added nested fields
sumukhswamy Jan 18, 2025
472e338
addressed comments for handling multi level nested fields
sumukhswamy Jan 21, 2025
380ea4d
linter changes
sumukhswamy Jan 21, 2025
d800b85
linter changes
sumukhswamy Jan 24, 2025
bf612d0
linter changes
sumukhswamy Jan 24, 2025
10f563b
linter changes
sumukhswamy Jan 24, 2025
92af2c8
flatten hits recursive function and modified nested test
sumukhswamy Jan 27, 2025
0b43116
removed unnecessary loops
sumukhswamy Jan 27, 2025
0c498c2
added test for deeply nested conditions
sumukhswamy Jan 28, 2025
b750dda
linter changes
sumukhswamy Jan 28, 2025
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
30 changes: 17 additions & 13 deletions server/routes/utils/__tests__/savedSearchReportHelper.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import 'regenerator-runtime/runtime';
import { createSavedSearchReport } from '../savedSearchReportHelper';
import { reportSchema } from '../../../model';
import { mockLogger } from '../../../../test/__mocks__/loggerMock';

Check failure on line 9 in server/routes/utils/__tests__/savedSearchReportHelper.test.ts

View workflow job for this annotation

GitHub Actions / Lint 

Mocks should not be manually imported from a __mocks__ directory. Instead use `jest.mock` and import from the original module path
import _ from 'lodash';

/**
Expand Down Expand Up @@ -52,15 +52,18 @@
const maxResultSize = 5;

describe('test create saved search report', () => {
test('create report with valid input', async () => {

Check warning on line 55 in server/routes/utils/__tests__/savedSearchReportHelper.test.ts

View workflow job for this annotation

GitHub Actions / Lint 

Test has no assertions
// Check if the assumption of input is up-to-date
reportSchema.validate(input);
}, 20000);

test('create report with expected file name', async () => {
const hits: Array<{ _source: any }> = [];

Check warning on line 61 in server/routes/utils/__tests__/savedSearchReportHelper.test.ts

View workflow job for this annotation

GitHub Actions / Lint 

Unexpected any. Specify a different type
const client = mockOpenSearchClient(hits);
const { timeCreated, fileName } = await createSavedSearchReport(
const {
timeCreated: _timeCreated,
fileName,
} = await createSavedSearchReport(
input,
client,
mockDateFormat,
Expand Down Expand Up @@ -103,7 +106,7 @@
}, 20000);

test('create report for empty data set', async () => {
const hits: Array<{ _source: any }> = [];

Check warning on line 109 in server/routes/utils/__tests__/savedSearchReportHelper.test.ts

View workflow job for this annotation

GitHub Actions / Lint 

Unexpected any. Specify a different type
const client = mockOpenSearchClient(hits);
const { dataUrl } = await createSavedSearchReport(
input,
Expand Down Expand Up @@ -909,7 +912,7 @@
'geoip.location': { lon: -0.1, lat: 51.5 },
customer_birth_date: '2023-04-26T04:34:32Z',
order_date: '2023-04-26T04:34:32Z',
products: { created_on: '2023-04-26T04:34:32Z' },
products: [{ created_on: '2023-04-26T04:34:32Z' }],
},
{
customer_birth_date: '2023-04-26T04:34:32Z',
Expand All @@ -924,7 +927,7 @@
'geoip.location': { lon: -74, lat: 40.8 },
customer_birth_date: '2023-04-26T04:34:32Z',
order_date: '2023-04-26T04:34:32Z',
products: { created_on: '2023-04-26T04:34:32Z' },
products: [{ created_on: '2023-04-26T04:34:32Z' }],
},
{
customer_birth_date: '2023-04-26T04:34:32Z',
Expand All @@ -947,11 +950,10 @@
mockLogger,
mockTimezone
);

expect(dataUrl).toEqual(
'geoip\\.country_iso_code,geoip\\.location\\.lon,geoip\\.location\\.lat,geoip\\.city_name\n' +
'GB,-0.1,51.5, \n' +
'US,-74,40.8,New York'
'geoip\\.country_iso_code,products\\.created_on,geoip\\.location\\.lon,geoip\\.location\\.lat,geoip\\.city_name\n' +
'GB,"[""2023-04-26T04:34:32Z""]",-0.1,51.5, \n' +
'US,"[""2023-04-26T04:34:32Z""]",-74,40.8,New York'
);
}, 20000);

Expand Down Expand Up @@ -1213,7 +1215,7 @@

test('create report for data set with metadata fields', async () => {
const metadataFields = { _index: 'nameofindex', _id: 'someid' };
let hits = [
const hits = [
hit(
{
category: 'c1',
Expand Down Expand Up @@ -1357,7 +1359,7 @@
true,
undefined,
mockLogger,
"Etc/GMT-2"
'Etc/GMT-2'
);

expect(dataUrl).toEqual(
Expand Down Expand Up @@ -1455,14 +1457,14 @@
* Mock Elasticsearch client and return different mock objects based on endpoint and parameters.
*/
function mockOpenSearchClient(
mockHits: Array<{ _source: any; fields: any }>,

Check warning on line 1460 in server/routes/utils/__tests__/savedSearchReportHelper.test.ts

View workflow job for this annotation

GitHub Actions / Lint 

Unexpected any. Specify a different type

Check warning on line 1460 in server/routes/utils/__tests__/savedSearchReportHelper.test.ts

View workflow job for this annotation

GitHub Actions / Lint 

Unexpected any. Specify a different type
columns = '"category", "customer_gender"'
) {
let call = 0;
const client = jest.fn();
client.callAsInternalUser = jest
.fn()
.mockImplementation((endpoint: string, params: any) => {

Check warning on line 1467 in server/routes/utils/__tests__/savedSearchReportHelper.test.ts

View workflow job for this annotation

GitHub Actions / Lint 

Unexpected any. Specify a different type
switch (endpoint) {
case 'get':
return {
Expand Down Expand Up @@ -1495,7 +1497,9 @@
case 'clearScroll':
return null;
default:
fail('Fail due to unexpected function call on client', endpoint);
throw new Error(
`Fail due to unexpected function call on client: ${endpoint}`
);
}
});
return client;
Expand Down Expand Up @@ -1573,9 +1577,9 @@
`);
}

function hit(source_kv: any, fields_kv = {}) {
function hit(sourceKv: any, fieldsKv = {}) {

Check warning on line 1580 in server/routes/utils/__tests__/savedSearchReportHelper.test.ts

View workflow job for this annotation

GitHub Actions / Lint 

Unexpected any. Specify a different type
return {
_source: source_kv,
fields: fields_kv,
_source: sourceKv,
fields: fieldsKv,
};
}
39 changes: 34 additions & 5 deletions server/routes/utils/dataReportHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,16 +297,45 @@ export const convertToExcel = async (dataset: any) => {
};

//Return only the selected fields
function traverse(data, keys, result = {}) {
function traverse(data: any, keys: string[], result: { [key: string]: any } = {}) {
// Flatten the data if necessary (ensure all nested fields are at the top level)
data = flattenHits(data);
const sourceKeys = Object.keys(data);

keys.forEach((key) => {
const value = _.get(data, key, undefined);
if (value !== undefined) result[key] = value;
else {

if (value !== undefined) {
result[key] = value;
} else {
Object.keys(data)
.filter((sourceKey) => sourceKey.startsWith(key + '.'))
.forEach((sourceKey) => (result[sourceKey] = data[sourceKey]));
.forEach((sourceKey) => {
result[sourceKey] = data[sourceKey];
});

Object.keys(data).forEach((dataKey) => {
const arrayValue = data[dataKey];

if (Array.isArray(arrayValue)) {
const flattenedValues: { [key: string]: any[] } = {};

arrayValue.forEach((item) => {
if (typeof item === 'object' && item !== null) {
Object.keys(item).forEach((subKey) => {
const newKey = `${dataKey}.${subKey}`;
if (!flattenedValues[newKey]) {
flattenedValues[newKey] = [];
}
flattenedValues[newKey].push(item[subKey]);
});
}
});
Copy link
Member

Choose a reason for hiding this comment

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

seems this only does it one level, does it work if the data is more nested like

{
  "a": {
    "b": {
      "c": [
        {
          "d": [{ "e": 1 }, { "e": 2 }]
        },
        {
          "d": [{ "e": 3 }, { "e": 4 }]
        }
      ]
    }
  }
}

and could you add some unit tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have updated it

Copy link
Member

Choose a reason for hiding this comment

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

i don't really see any logic change in the new commits?

Copy link
Member

Choose a reason for hiding this comment

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

Adding more comments here:

  1. This may be still only working with one level of array nesting
  2. Best way to check is by adding multi-level nested array of objects
  3. Nit - Object.keys(data) is looped twice can just keep it one.


Object.keys(flattenedValues).forEach((newKey) => {
result[newKey] = flattenedValues[newKey];
});
}
});
}
});
return result;
Expand Down
Empty file added server/routes/utils/test
View file
Open in desktop
Empty file.
Loading