feat: [#188645598] add environmental questionnaire for land #9207

somabadri · 2024-12-03T22:57:02Z

Description

Ticket

This pull request resolves #188645598.

Approach

this builds off the work done for environmental questionnaire for waste
created components that dynamically render content and update userData based on media area
added migration v151 which adds land to environmentData

Steps to Test

Onboard as a new business
Select All other businesses for industry
You should now see the Waste Task under step 3 of the roadmap

Notes

Code author checklist

api/src/db/migrations/v151_add_land_to_environment_data.test.ts

content/src/fieldConfig/env-questionnaire.json

adidner

Man this was complicated. So great work in that regard parsing through it all.

I have a few dust food for comment thoughts and a few minor refactor sand things but honestly they're pretty nit picky and I think you're largely good to go. Nice job

adidner · 2024-12-19T15:44:26Z

web/src/components/tasks/environment-questionnaire/EnvPermitsResults.tsx

+      selected === true && optionsMarkedTrue.push(text);
+    }
+
+    return optionsMarkedTrue;


dust - so this feels a little convoluted, I'm not sure if its a changable thing and I did spend some time really thinking about it.

It kinda feels to me like somehow we should have like a combined state object that has both the string values and their checked state all together as one object. But I'm not sure how much less convoluted that actually because you have to build it and I think this is fine. Mostly just food for thought

I see what you're saying, AA. At the very least we should rename the variable (optionsMarkedTrue) so it's not the same as the function.

adidner · 2024-12-19T15:50:37Z

web/src/components/tasks/environment-questionnaire/EnvPermitsResults.test.tsx

+                takeOverExistingBiz: true,
+                propertyAssessment: false,
+                constructionActivities: false,
+              }),


dust - my inclination here would have been to cover ever variable in both states just to make sure its possible. I think there are a lot of ways to do that and this is mostly nit picky BS at this point and you can ignore this if you like.

adidner · 2024-12-19T15:59:24Z

web/src/components/tasks/environment-questionnaire/EnvQuestionnaire.test.tsx

+      expect(currentBusiness().taskProgress[taskId]).toEqual("COMPLETED");
+    });
+
+    it("calls scrollToTop when saved", async () => {


sand - I believe all these tests are repeated verbatum with different values so you can use an it.each to just pass ing 2 different values and half your total test code volume. Lmk if you want to talk about how to do that,.

adidner · 2024-12-19T16:04:20Z

web/src/components/tasks/environment-questionnaire/results/ResultsSectionAccordion.tsx

@@ -11,7 +11,7 @@ interface Props {
  isDividerDisabled?: boolean;
 }


sand - I don't think those props above (specifically mini and isDividerDisabled are actually used ever?)

AnJuHyppolite · 2024-12-19T15:26:19Z

content/src/fieldConfig/env-questionnaire.json

+    "editInfo": "Your answers were successfully completed!",
+    "editText": "Edit.",
+    "summary": {
+      "partOne": "Based on your answers, your business may be legally required to have a Department of Environmental Protection’s (DEP)",


I know we may have to check with the Content team, but we don't need the apostrophe and s. It should just read: Protection.

AnJuHyppolite · 2024-12-19T15:31:16Z

content/src/fieldConfig/env-questionnaire.json

+    "editText": "Edit.",
+    "summary": {
+      "partOne": "Based on your answers, your business may be legally required to have a Department of Environmental Protection’s (DEP)",
+      "partTwo": "license or permit. Find out your business' requirements below.",


This may be another one for the Content team: business' should either be business or business's

AnJuHyppolite · 2024-12-19T16:01:35Z

content/src/roadmaps/env-tasks/land-permitting.md

@@ -0,0 +1,8 @@
+---
+summaryDescriptionMd: The New Jersey Department of Environmental Protection's (DEP) has environmental regulations for businesses in the State.


Same as before: We should update this to Protection.

AnJuHyppolite · 2024-12-19T16:50:35Z

web/src/components/tasks/environment-questionnaire/EnvPermitsResults.tsx

+      selected === true && optionsMarkedTrue.push(text);
+    }
+
+    return optionsMarkedTrue;


I see what you're saying, AA. At the very least we should rename the variable (optionsMarkedTrue) so it's not the same as the function.

SirSamuelJoseph · 2024-12-20T18:35:22Z

web/public/mgmt/config.yml

-      - label: "Waste Questionnaire Task"
-        name: "waste-questionnaire-task"
-        file: content/src/fieldConfig/waste-questionnaire-page.json
+      - label: "Environment Questionnaire Tasks"


Great condensing here

somabadri force-pushed the check-env-req-land branch 3 times, most recently from 4eb035b to 0e176b3 Compare December 14, 2024 00:18

somabadri force-pushed the check-env-req-land branch 7 times, most recently from c6eff32 to f1f077d Compare December 18, 2024 01:40

somabadri marked this pull request as ready for review December 18, 2024 02:04

somabadri force-pushed the check-env-req-land branch from f1f077d to 52c483f Compare December 18, 2024 14:50

adidner reviewed Dec 18, 2024

View reviewed changes

api/src/db/migrations/v151_add_land_to_environment_data.test.ts Outdated Show resolved Hide resolved

adidner reviewed Dec 18, 2024

View reviewed changes

content/src/fieldConfig/env-questionnaire.json Outdated Show resolved Hide resolved

somabadri force-pushed the check-env-req-land branch 2 times, most recently from a16c64a to d9cac7b Compare December 19, 2024 00:22

feat: [#188645598] add environmental questionnaire for land

4302ca6

somabadri force-pushed the check-env-req-land branch from d9cac7b to 4302ca6 Compare December 19, 2024 01:11

adidner approved these changes Dec 19, 2024

View reviewed changes

AnJuHyppolite reviewed Dec 19, 2024

View reviewed changes

SirSamuelJoseph reviewed Dec 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [#188645598] add environmental questionnaire for land #9207

feat: [#188645598] add environmental questionnaire for land #9207

somabadri commented Dec 3, 2024 •

edited

Loading

adidner left a comment

adidner Dec 19, 2024

AnJuHyppolite Dec 19, 2024

adidner Dec 19, 2024

adidner Dec 19, 2024

adidner Dec 19, 2024

AnJuHyppolite Dec 19, 2024

AnJuHyppolite Dec 19, 2024

AnJuHyppolite Dec 19, 2024

AnJuHyppolite Dec 19, 2024

SirSamuelJoseph Dec 20, 2024

		@@ -11,7 +11,7 @@ interface Props {
		isDividerDisabled?: boolean;
		}

		@@ -0,0 +1,8 @@
		---
		summaryDescriptionMd: The New Jersey Department of Environmental Protection's (DEP) has environmental regulations for businesses in the State.

feat: [#188645598] add environmental questionnaire for land #9207

Are you sure you want to change the base?

feat: [#188645598] add environmental questionnaire for land #9207

Conversation

somabadri commented Dec 3, 2024 • edited Loading

Description

Ticket

Approach

Steps to Test

Notes

Code author checklist

adidner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

somabadri commented Dec 3, 2024 •

edited

Loading