feat: [188623677] add environmental questionnaire for air #9242
+1,441
−32
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SirSamuelJoseph
commented
Dec 6, 2024
web/src/components/tasks/environment-questionnaire/air/CheckAirPermits.tsx
Outdated
Show resolved
Hide resolved
somabadri
force-pushed
the
eco-air-waste-permit-task
branch
9 times, most recently
from
71596a2 to
e2a2a00
Compare
somabadri
changed the title
somabadri
force-pushed
the
eco-air-waste-permit-task
branch
from
e2a2a00 to
ff82192
Compare
somabadri
force-pushed
the
eco-air-waste-permit-task
branch
from
ff82192 to
d48587f
Compare
jphechter
reviewed
Jan 14, 2025
| @@ -80,7 +80,6 @@ describe("v152_add_land_to_environment_data", () => { | |||
| it("adds land section to environment data", () => { | |||
| const id = "biz-1"; | |||
| const v151Business = generatev151Business({ | |||
| taskProgress: { "waste-permitting": "IN_PROGRESS" }, | |||
There was a problem hiding this comment.
@jphechter I took it out because it's not necessary for the test that the waste task is in progress, it's not relevant to what we are trying to test. We just want to ensure that the land section gets added.
somabadri
force-pushed
the
eco-air-waste-permit-task
branch
from
d48587f to
023ec86
Compare
jphechter
reviewed
Jan 20, 2025
Comment on lines
+10
to
+78
| it("migrates environment data when land task is COMPLETED", () => { | ||
| const id = "biz-1"; | ||
| const v152Business = generatev152Business({ | ||
| taskProgress: { "land-permitting": "COMPLETED" }, | ||
| environmentData: { | ||
| land: generatev152LandData({ | ||
| questionnaireData: generatev152LandQuestionnaireData({ | ||
| takeOverExistingBiz: false, | ||
| propertyAssessment: true, | ||
| constructionActivities: true, | ||
| siteImprovementWasteLands: false, | ||
| noLand: false, | ||
| }), | ||
| submitted: true, | ||
| }), | ||
| }, | ||
| id, | ||
| }); | ||
| const v152User = generatev152UserData({ | ||
| businesses: { "biz-1": v152Business }, | ||
| }); | ||
|
|
||
| const v153User = migrate_v152_to_v153(v152User); | ||
|
|
||
| expect(v153User.businesses[id].environmentData?.land?.questionnaireData).toEqual({ | ||
| takeOverExistingBiz: false, | ||
| propertyAssessment: true, | ||
| constructionActivities: true, | ||
| siteImprovementWasteLands: false, | ||
| noLand: false, | ||
| }); | ||
| expect(v153User.businesses[id].environmentData?.land?.submitted).toEqual(true); | ||
| expect(v153User.businesses[id].taskProgress["land-permitting"]).toEqual("COMPLETED"); | ||
| }); | ||
|
|
||
| it("migrates environment data when land task is IN_PROGRESS", () => { | ||
| const id = "biz-1"; | ||
| const v152Business = generatev152Business({ | ||
| taskProgress: { "land-permitting": "IN_PROGRESS" }, | ||
| environmentData: { | ||
| land: generatev152LandData({ | ||
| questionnaireData: generatev152LandQuestionnaireData({ | ||
| takeOverExistingBiz: false, | ||
| propertyAssessment: true, | ||
| constructionActivities: true, | ||
| siteImprovementWasteLands: false, | ||
| noLand: false, | ||
| }), | ||
| submitted: false, | ||
| }), | ||
| }, | ||
| id, | ||
| }); | ||
| const v152User = generatev152UserData({ | ||
| businesses: { "biz-1": v152Business }, | ||
| }); | ||
|
|
||
| const v153User = migrate_v152_to_v153(v152User); | ||
|
|
||
| expect(v153User.businesses[id].environmentData?.land?.questionnaireData).toEqual({ | ||
| takeOverExistingBiz: false, | ||
| propertyAssessment: true, | ||
| constructionActivities: true, | ||
| siteImprovementWasteLands: false, | ||
| noLand: false, | ||
| }); | ||
| expect(v153User.businesses[id].environmentData?.land?.submitted).toEqual(false); | ||
| expect(v153User.businesses[id].taskProgress["land-permitting"]).toEqual("IN_PROGRESS"); | ||
| }); |
There was a problem hiding this comment.
I'm a little confused what the point of these tests are. Based on the v153 migration, it looks like the only thing we're doing is adding a key to environmentData. I'm reading these tests as though this is indicating some conditional logic should be happening. Are we missing functionality in the migration, or are these tests supposed to be checking for something else?
There was a problem hiding this comment.
@jphechter I added these tests as a suggestion from Aaron on the previous migration. The intention was to check that the existing data within environmentData gets transferred over properly to the new version regardless of its state. In hindsight, they are a bit exhaustive, but that was the reason for their addition.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Ticket
This pull request resolves #188623677.
Approach
environmentDataSteps to Test
All other businessesfor industryNotes
Code author checklist
userData(includingprofileData,formationDataetc), then I added a new migration filecmsCollections.ts(see CMS Additions in Engineering Reference/FAQ on the engineering documentation site).envvalues in both.env-templateand in Bitwarden