feat: [#187570392] raffle-bingo-nonessential-ques-license-task #8697

AnJuHyppolite · 2024-09-19T22:37:25Z

Description

Ticket

This pull request resolves #187570392.

Approach

Steps to Test

Enter the app as a Poppy with a nonprofit business structure.
The industry is user's choice since raffle-bingo is industry agnostic.
Once you're directed to your dashboard, select Yes for the Will you be holding, operating, or conducting raffles or bingo games? question and save your response.
Once you're redirected to your dashboard, click the Apply for a Raffle or Bingo Game License.
You can toggle between steps by clicking the numbers in the stepper or by using the Continue and Back buttons.
If you click the Mark as Completed button, the task progress will change from NOT STARTED to COMPLETED and a snack bar advising of such, will appear.

Notes

Code author checklist

AnJuHyppolite · 2024-09-20T00:39:29Z

shared/src/profileData.ts

@@ -150,6 +151,7 @@ export const emptyProfileData: ProfileData = {
  nonEssentialRadioAnswers: {},
  elevatorOwningBusiness: undefined,
  communityAffairsAddress: undefined,
+  raffleBingoGames: undefined,


Context: The raffle-bingo non-essential question is industry-agnostic, so I added the raffleBingoGames property to ProfileData instead of IndustrySpecificData (or its related structures), which we typically use for industry-specific questions.

AnJuHyppolite · 2024-09-20T01:09:53Z

content/src/roadmaps/raffle-bingo-steps/raffle-license-step-1.md

Context: I moved raffle-license-step-1.md and raffle-license-step-2.md from content/src/roadmaps/license-tasks/ into their own directory, content/src/roadmaps/raffle-bingo-steps/, to account for the nuance that the Raffle Bingo Licensure process involves two distinct steps.

As a result, I had to make updates to search.tsx, cmsCollections.ts, loadTasks.ts, and loadTasks.test.ts.

AnJuHyppolite · 2024-09-20T01:10:02Z

content/src/roadmaps/raffle-bingo-steps/raffle-license-step-2.md

Context: I moved raffle-license-step-1.md and raffle-license-step-2.md from content/src/roadmaps/license-tasks/ into their own directory, content/src/roadmaps/raffle-bingo-steps/, to account for the nuance that the Raffle Bingo Licensure process involves two distinct steps.

As a result, I had to make updates to search.tsx, cmsCollections.ts, loadTasks.ts, and loadTasks.test.ts.

jphechter · 2024-09-20T17:41:58Z

api/src/db/migrations/v143_add_raffle_bingo_games.ts

+    ...business,
+    profileData: {
+      ...business.profileData,
+      raffleBingoGames: undefined,


This is fine as a formal indicator that we're adding this to profileData object, but because there's no data actually being set this doesn't do anything.

web/public/mgmt/config.yml

content/src/roadmaps/raffle-bingo-steps/raffle-license-step-2.md

jphechter · 2024-09-23T14:51:47Z

content/src/roadmaps/tasks/raffle-bingo-games-license.md

+summaryDescriptionMd: ""
+---
+
+This task is intentionally blank. The content here is controlled by raffle-step-license-1 and raffle-step-license-2.


[boulder] We should talk to content about this task. Should this actually live with the rest of the license tasks? Right now this will not be sync'd to Webflow, and are they anticipating that it will be? The behavior of the task in-app will not change. This is more of a question of how they want this to work with Webflow.

If that's the case, and they do want it to sync, the content being sync'd to Webflow will be:

This task is intentionally blank. The content here is controlled by raffle-step-license-1 and raffle-step-license-2.

If they have specific content that they'd like to be displayed on Webflow that should be added here. It will not show up in the Navigator. This is exclusively in regards to data being sync'd.

@AnJuHyppolite I know we discussed this with content in Slack and at TLC daily scrum, but did we ever come to a decision here?

@ReginaL1 not mission critical for this piece of work, but just want to make sure it's on your radar.

@jphechter @AnJuHyppolite I believe we punted to decide later what the content for Webflow will be, since it's a complex case. For now, showing the task screens on My Account is the higher priority

jphechter · 2024-09-23T16:03:07Z

web/src/components/tasks/RaffleBingoPaginator.test.tsx

+    );
+  };
+
+  it("renders first page without clicking stepper", async () => {


[sand] The behavior we're trying to capture here is that the first screen is displayed. I don't think referencing the stepper is immediately relevant for this test. Consider renaming this test to better capture the desired behavior.

Suggested change

it("renders first page without clicking stepper", async () => {

it("renders first tab content when the task is opened", async () => {

@AnJuHyppolite Marking this unresolved. This is the same test name.

web/src/components/tasks/RaffleBingoPaginator.test.tsx

jphechter · 2024-09-23T16:37:58Z

web/src/components/tasks/RaffleBingoTask.tsx

+  return (
+    <div>
+      <TaskHeader task={props.task} />
+      <RaffleBingoPaginator task={props.task} />


I know this is a pattern we follow other places in the application, but I'm not totally sold on shoving all of the task logic into the paginator. The only thing the task component is really doing here is adding a TaskHeader. Personally, I'd opt for either keeping all the logic in a single file (e.g. moving everything in RaffleBingoPaginator up one level into RaffleBingoTask), or divvying up logical responsibilities between the paginator component and the task component.

This just feels like RaffleBingoTask doesn't have purpose.

Curious what your thoughts are here.

web/src/lib/roadmap/buildUserRoadmap.test.ts

jphechter · 2024-09-23T16:47:54Z

web/src/lib/roadmap/buildUserRoadmap.test.ts

+      );
+      expect(getLastCalledWith(mockRoadmapBuilder)[0].addOns).not.toContain("raffle-bingo-games");
+    });
+  });


[boulder] We should be covering all our negative test cases here as well. Given that we are fully expecting that a business must be a nonprofit in order to get this addon, there should be a test for the case that raffleBingoGames is true, legalStructureId is not nonprofit and we assert that the addon does not get added.

web/src/lib/roadmap/buildUserRoadmap.test.ts

jphechter · 2024-09-23T16:49:58Z

web/src/lib/roadmap/buildUserRoadmap.test.ts

+    it("adds raffle-bingo-games task if raffle-bingo-games business", () => {
+      const profileData = generateStartingProfile({
+        raffleBingoGames: true,
+        isNonprofitOnboardingRadio: true,


This line isn't impacting the logic in buildUserRoadmap. Strongly advise we leave this out. This is a case where we're staging data that does not actually support the test case, which is something else we've been talking about as a team.

Suggested change

isNonprofitOnboardingRadio: true,

I updated the test names to account for the non-profit legal structure that is needed in order for a business to get a raffle-bingo prompt and added a test for non-profits that do not conduct raffle/bingo games.

web/src/lib/roadmap/buildUserRoadmap.test.ts

jphechter · 2024-09-23T16:55:56Z

web/src/pages/profile.tsx

+  const displayRaffleBingoGameQuestion = (): boolean => {
+    if (!business) return false;
+    return profileData.legalStructureId === "nonprofit";
+  };


[boulder] Should we add a test for this?

Yes, added.

Should we also add tests to assert that this doesn't appear on the Oscar and Dakota profiles as well?

jphechter · 2024-09-30T15:55:07Z

web/public/mgmt/config.yml

@@ -2522,6 +2522,54 @@ collections:
          - { label: "school/course", value: "school-course" }
          - { label: "object or vehicle", value: "object-vehicle" }

+  - name: "raffle-bingo-steps"
+    label: "Raffle Bingo Steps (Navigator with Webflow mappings)"


[sand] This should be removed

Suggested change

label: "Raffle Bingo Steps (Navigator with Webflow mappings)"

label: "Raffle Bingo Steps"

jphechter · 2024-09-30T15:58:05Z

web/public/mgmt/config.yml

+    create: true
+    editor:
+      preview: true
+    fields:


[boulder] Take another look at these fields. It looks like we aren't representing the collection items correctly (Ex: body is missing, we don't need license name, etc)

@AnJuHyppolite body is still missing.

jphechter · 2024-09-30T16:00:08Z

web/public/mgmt/config.yml

+  - name: "raffle-bingo-steps"
+    label: "Raffle Bingo Steps (Navigator with Webflow mappings)"
+    folder: "content/src/roadmaps/raffle-bingo-steps"
+    summary: "{{displayname}} - {{name}}"


[sand] We don't have a displayname for these items, so they're rendering with a leading dash.

Suggested change

summary: "{{displayname}} - {{name}}"

summary: "{{name}}"

I had a different DX. raffle-bingo-step-1 rendered in the CMS, and I found that raffle-bingo-step-1 had a displayname and raffle-bingo-step-2 did not. I addeddisplayname to raffle-bingo-step-2 and now both steps render with their display names.

This approach follows the existing pattern in the CMS, where the display name of the task precedes the license task (name).

jphechter · 2024-09-30T16:01:26Z

web/public/mgmt/config.yml

+    editor:
+      preview: true


If you want to make this feel a little more complete you could implement the CMS preview, but not necessary for the ticket to be complete.

Let me know when you rebase

alexander-littleton · 2024-10-01T20:39:01Z

web/src/components/tasks/RaffleBingoPaginator.test.tsx

+    await waitFor(() => {
+      expect(screen.getByText("Application Requirements")).toBeInTheDocument();
+    });


Suggested change

await waitFor(() => {

expect(screen.getByText("Application Requirements")).toBeInTheDocument();

});

expect(screen.findByText("Application Requirements")).toBeInTheDocument();

[dust] No need to make this change but findByText will actually do the same thing waitFor() does in a less verbose way for this case.

web/src/contexts/configContext.ts

alexander-littleton · 2024-10-01T21:20:46Z

web/src/components/tasks/RaffleBingoSteps.ts

[sand] This file of helper functions is a bit of a new pattern in this directory. I think it would be fine to just unpack these declarations into the component files where they are used since they are each quite small but open to other thoughts.

I like the way u think

Whole-heartedly agree.

web/src/components/tasks/RaffleBingoSteps.ts

alexander-littleton · 2024-10-01T21:26:02Z

web/src/lib/async-content-fetchers/fetchTaskByFilename.ts

-        file = await import(`@businessnjgovnavigator/content/roadmaps/municipal-tasks/${filename}.md`);
+        try {
+          file = await import(`@businessnjgovnavigator/content/roadmaps/municipal-tasks/${filename}.md`);
+        } catch {
+          file = await import(`@businessnjgovnavigator/content/roadmaps/raffle-bingo-steps/${filename}.md`);
+        }


[dust] This might actually be a case where Promise chaining via .catch() would be preferable to prevent nesting.

alexander-littleton · 2024-10-01T22:02:07Z

web/src/components/tasks/RaffleBingoSteps.ts

+export const shouldDisplayContinueButton = (stepIndex: number): boolean => {
+  return stepIndex !== RaffleBingoSteps.length - 1;
+};
+
+export const shouldDisplayBackButton = (stepIndex: number): boolean => {
+  return stepIndex !== 0;
+};
+
+export const shouldDisplayMarkAsCompleteButton = (stepIndex: number): boolean => {
+  return stepIndex === RaffleBingoSteps.length - 1;
+};


[dust] Tiny thing here but I find it a better practice to name things based on what values they represent instead of where they are used as it makes them more flexible and increases readability. One way you can directly see the benefit of this is that we could replace the two shouldDisplayContinueButton and shouldDisplayMarkAsCompleteButton functions with just one isLastStep function. In the other file where that function is used, you can tell exactly what that new name represents without having to jump into this file. I would go with isNotFirstStep or something similar for the other one.

content/src/fieldConfig/profile.json

jphechter · 2024-10-02T17:21:42Z

content/src/roadmaps/raffle-bingo-steps/raffle-license-step-1.md

@@ -1,17 +1,12 @@
 ---
 notesMd: "[2526: Add non-essential question tasks: raffle task screen](https://dev.azure.com/NJInnovation/Business%20First%20Stop/_workitems/edit/2526)"
 id: "raffle-license-step-1"
-webflowId: "66be3c5382fc36dffc2ce314"


When you merge this, remind me so that we can remove these from Webflow. I think the sync job will take care of this, but just incase we can proactively delete them.

web/public/mgmt/config.yml

jphechter · 2024-10-02T17:44:33Z

web/src/components/njwds-extended/cta/SingleCtaLink.tsx

@@ -33,6 +35,11 @@ export const SingleCtaLink = (props: Props): ReactElement => {
          }}
        >
          {props?.text || Config.taskDefaults.defaultCallToActionText}
+          {props.iconName && (
+            <Icon className="text-green usa-icon--size-3 margin-left-1 padding-bottom-05">
+              {props.iconName}


Not something you chose, but I think it's super odd that we specify the icon type by passing a string as a child. I kinda hate this.

jphechter · 2024-10-02T17:54:50Z

web/src/components/tasks/RaffleBingoPaginator.tsx

+
+export const RaffleBingoPaginator = (props: Props): ReactElement => {
+  const [stepIndex, setStepIndex] = useState(0);
+  const [task, setTask] = useState<Task>();


[sand] It might be helpful to rename this state variable to better differentiate this from the task prop being passed into this component.

These objects are Tasks but the way we're treating them is really steps in the context of a larger task.

Suggested change

const [task, setTask] = useState<Task>();

const [step, setStep] = useState<Task>();

jphechter · 2024-10-02T17:55:26Z

web/src/components/tasks/RaffleBingoPaginator.tsx

+    getRaffleBingoTask(stepIndex).then((task) => {
+      setTask(task);
+    });


[sand] For the same reason as above.

Suggested change

getRaffleBingoTask(stepIndex).then((task) => {

setTask(task);

});

getRaffleBingoTask(stepIndex).then((step) => {

setStep(step);

});

web/src/components/tasks/RaffleBingoSteps.ts

jphechter · 2024-10-02T18:00:57Z

web/src/components/tasks/RaffleBingoSteps.ts

+  {
+    stepLabel: "Get an ID Number",
+    fileName: "raffle-license-step-1",
+    agencyId: "games-of-chance-control-commission",
+  },
+  { stepLabel: "From Your NJ Municipality", fileName: "raffle-license-step-2", agencyId: "nj-municipality" },


[pebble] Things like stepLabel and agencyId should probably be content driven. I suggest moving these values into the CMS. Then the 2 filenames representing the steps could be stored in an array.

In RaffleBingoPaginator, instances where you're referencing RaffleBingoSteps to get this content could look at the current step.

jphechter · 2024-10-02T18:06:55Z

web/src/components/tasks/RaffleBingoSteps.ts

+export const getAgencyName = (agencyId: string): string => {
+  const issuingAgency = arrayOfTaskAgencies.find((agency) => agency.id === agencyId);
+  return issuingAgency!.name;
+};


[boulder] We have a function LookupTaskAgencyById that's available in taskAgency.ts which does exactly this. Opt for that function over re-writing the same logic.

I understand the thought here. In the future, if you're adding something that you think would be beneficial elsewhere in the application, it could be an opportunity to write a helper function.

jphechter · 2024-10-02T18:09:27Z

web/src/lib/roadmap/buildUserRoadmap.test.ts

+      buildUserRoadmap(
+        generateStartingProfile({
+          raffleBingoGames: false,
+          legalStructureId: "limited-liability-company",


[dust] If we wanted to be really thorough here, we could be grabbing a random legalStructureId that is not nonprofit. This is a common pattern we use in our tests.

web/src/components/tasks/RaffleBingoPaginator.tsx

AnJuHyppolite force-pushed the raffle-bingo-nonessential-ques-license-task-#187570392 branch from f1758e6 to 88eaf68 Compare September 20, 2024 00:37

AnJuHyppolite commented Sep 20, 2024

View reviewed changes

AnJuHyppolite force-pushed the raffle-bingo-nonessential-ques-license-task-#187570392 branch 3 times, most recently from 5bb07cf to 919e2ca Compare September 20, 2024 00:57

AnJuHyppolite commented Sep 20, 2024

View reviewed changes

AnJuHyppolite force-pushed the raffle-bingo-nonessential-ques-license-task-#187570392 branch 2 times, most recently from 40ffc05 to 6cb2d0f Compare September 20, 2024 01:26

jphechter reviewed Sep 20, 2024

View reviewed changes