US-EPA-CAMD / easey-ui

Project Management repo for EPA Clean Air Markets Division (CAMD) Business Suite of applications
MIT License
0 stars 0 forks source link

Schema issues with APPE data #5992

Open esaber76 opened 11 months ago

esaber76 commented 11 months ago

Found during regression testing in ecmps-tst.app.cloud.gov v2.0.90.

Logged in as saber_ds (Submitter) and imported Appendix E test from historical. Exported test to json file, re-imported/ re-evaluated and received APPE-42-A, APPE-43-E, and AETB-10-A (5985) errors.

The json file includes "appendixECorrelationTestSummary Data" and "appECorrelationTestSummaryData" fields. The data is in the latter so most of the test data is missing after the import.

Schema only includes "appECorrelationTestSummary".

Per 12/14/2023 regression testing review meeting with EPA, update the schema to "appendixECorrelationTestSummaryData". The export should only include "appendixECorrelationTestSummaryData".

QA reporting instructions needed updates added to EC-3959.

annalbrecht commented 8 months ago

From #5985:

Found during regression testing in ecmps-tst.app.cloud.gov v2.0.90.

Logged in as saber_ds (Submitter) and imported RATA test from historical. Exported test to json file, re-imported and re-evaluated and received a critical 2 AETB-10-A error.

The exported data has "airEmissionTestingData" and "airEmissionTestData" fields with the AETB data located in "airEmissionTestingData".

The schema includes "airEmissionTestData". "airEmissionTestingData" is also defined in the MP Reporting Instructions.

Per 12/14/2023 regression testing review meeting with EPA, update the schema to "airEmissionTestingData". The export should only include "airEmissionTestingData".

The export of submitted AETB data also includes the following database fields. Is there a reason for this?

id testSumId userId addDate updateDate This affects APPE, RATA, and UNITDEF QA test exports.

maxdiebold-erg commented 7 months ago

Are the extra database fields in the scope of this issue, or are those being addressed in #6182?

lgiannini1 commented 7 months ago

Verified schema change on dev using ORIS 6137, unit 3 APPE test #13. JSON export of test only contains "appendixECorrelationTestSummaryData" and "airEmissionTestingData". Currently not able to test evaluations on dev due to bypass

lgiannini1 commented 6 months ago

@maxdiebold-erg Not sure if this was changed with the typeORM updates, but when importing the test from my comment above for ORIS 6137, unit 3 from historical data and exporting the test, a componentId is added in the testSummaryData record. Trying to reimport this test results in an IMPORT-18-C error.

Image

maxdiebold-erg commented 6 months ago

@lgiannini1 That doesn't sound like a TypeORM issue, were you able to re-import it previously? Again, that sounds like the issue being addressed in #6182.

lgiannini1 commented 6 months ago

@maxdiebold-erg Yes, previously this componentId field was null in the exports and I was able to reimport the test.

maxdiebold-erg commented 6 months ago

My mistake, I had the fields mixed up, componentId is in the schema. I'll look into that issue more.

maxdiebold-erg commented 6 months ago

@lgiannini1 Good call! I looked more into this issue, and found that the problem is due to the issue described here. null values in queries are simply dropped by TypeORM unless the IsNull helper is used. In this case, the test summary is assigned a component with:

testSumRecord.component = await this.componentWorkspaceRepository.findOne({
  where: {
    componentID: testSummary.componentId,
    locationId,
  },
});

which, in the case of a null componentId value, is translated by TypeORM to:

SELECT
    component_id,
    mon_loc_id,
    component_identifier,
    component_type_cd
FROM
    camdecmpswks.component
WHERE (mon_loc_id = $1)
LIMIT 1

which will return multiple rows. Instead, we should check for a null componentId before performing the query.

lgiannini1 commented 6 months ago

@maxdiebold-erg Was this componentId issue fixed on dev? I'm still seeing a componentId for APPE and RATA tests. Linearities also have a monitoringSystemId when they shouldn't.

maxdiebold-erg commented 6 months ago

@lgiannini1 I am working on the underlying fix for that across all repositories, and I'll have those fixes in code review today.

lgiannini1 commented 6 months ago

ComponentId issue has been resolved on dev.

ibarra-michelle commented 5 months ago

Verified in the test environment with ORIS 6137, Location 3.

  1. In the QA module, I imported from historical, exported and imported with no imports errors. Then, I evaluated with no errors.
  2. Also, airEmissiontestData has been removed and airEmissionTestingData is now available in the exported json file.
  3. Lastly, in the exported file, the componentID field is null again.