theunitedeffort / theunitedeffort.org

The United Effort Organization website
https://theunitedeffort.org
3 stars 5 forks source link

Add Clipper Start to eligibility tool #603

Closed trevorshannon closed 2 months ago

trevorshannon commented 2 months ago

https://www.clipperstartcard.com/s/

DairyProducts commented 2 months ago

What file(s) contains the logic for determining what programs someone is eligible for, and recording which eligibility conditions someone satisfies? I think it might be in eligibility.js but I'm not sure how it works.

trevorshannon commented 2 months ago

Yes, the logic for eligibility is in eligibility.js and also the list of programs is defined in benefits-eligibility.json

In eligibility.js, you will need to create a new function called something like clipperStartResult() and map it to the page DOM in mapResultFunctions(). The ID of the DOM element to map to will be based on the program id you choose in benefits-eligibility.json.

When writing your result function, you can look at the other *Result() functions for guidance (such as adsaResult). That's where the actual eligibility determination is made. Note that you should not use standard boolean operators such as &&, ||, ! etc but instead use the methods and, or, not, etc. This is so we can seamlessly handle cases where users do not fill in certain form questions.

The form questions are defined in eligibility.liquid in case you need to add any additional questions. All the data we already use is put into an input object in buildInputObject

There will be some ramp-up time to get oriented with this eligibility code, but once you get a feel for the code, it should not be too bad to add this program. I'm happy to answer questions as they come up.

DairyProducts commented 2 months ago

I'm not sure how the input.existingX system works. I understand that it is to check for existing benefits from the checkbox list on the final page of the form, and I added a row to that checkbox list in eligibility.liquid with an ID of rtc-clipper. I see plenty of functions referencing existing benefits such as in adsaResult():

  const isProgramQualified = or(
    input.existingSsiMe,
    input.existingSsdiMe,
    input.existingIhssMe,
    input.existingCapiMe,

But I can't find where input.existingSsiMe is declared for example. I'm wondering what needs to be done in order to create an input.existingRtcClipper.

trevorshannon commented 2 months ago

The input object is created and populated in buildInputObj. Within that function, there is some code that iterates through the "existing assistance" checkboxes and makes an object property for each one: https://github.com/theunitedeffort/theunitedeffort.org/blob/d3a4a6a743ba6626cde1b62d5dda1660438b43c2/src/site/_includes/js/eligibility.js#L2727

The property is named the same as the checkbox element id, except it is converted to camelCase. So you should automatically have access to input.existingRtcClipperMe and input.existingRtcClipperHousehold just by adding a row in the Liquid template with a key of rtc-clipper

DairyProducts commented 2 months ago

I'm trying to write tests for the Clipper START program in eligibility-programs-test.js but I'm getting an error when running them. The following errors are thrown:

TypeError: elig.clipperStartResult is not a function
at Object.clipperStartResult (site/_includes/js/test/eligibility-programs.test.js:2148:19)

TypeError: this.program is not a function
at Object.program [as is] (site/_includes/js/test/eligibility-programs.test.js:111:15)
at Object.is (site/_includes/js/test/eligibility-programs.test.js:2153:85)

This is despite the fact that in eligibility.js there exists a function called clipperStartResult. I looked at previous tests in eligibility-programs-test.js as a guide for writing my tests and I don't see other result functions being referenced elsewhere in eligibility-programs-test.js.

Do the result functions need to be initialized somewhere for use in eligibility-programs-test.js?

trevorshannon commented 2 months ago

Ah yes for tests you will have to export the new function. Sorry for not mentioning it earlier! Hopefully adding a line to the module.exports in eligibility.js will fix it.

DairyProducts commented 2 months ago

That seemed to have solved the TypeError but now a new error is thrown for all of the checks: A logic input value was undefined

The test code is below: https://github.com/theunitedeffort/theunitedeffort.org/blob/84dadc0cac8d5ee5ff221f1b32816fa152d5816f/src/site/_includes/js/test/eligibility-programs.test.js#L2146-L2166

I tried to recreate what previous describe blocks did for similar checks but it doesn't seem to be working. I'm guessing statements like check(elig.clipperStartResult, input).isEligibleIf('existing-rtc-clipper-me').is(false) contain invalid boolean expressions, but I'm not quite sure how the check function works either.

trevorshannon commented 2 months ago

That error gets thrown not by the test code, but by the custom boolean functions via: https://github.com/theunitedeffort/theunitedeffort.org/blob/d3a4a6a743ba6626cde1b62d5dda1660438b43c2/src/site/_includes/js/eligibility.js#L270 The main use of the error is for testing though, as we want to be sure we actually set all the inputs in the test and haven't forgotten any.

You may have to ensure all new input properties have been properly initialized for the tests. This is done in the Jest beforeEach method here.

The check function is really just a convenience that allows you to execute various groups of Jest expect statements with a sort of easy-to-read chaining. For example, you can see exactly what's checked by check(...).isEligibleIf(value).isAtMost(limit) in the definition for isAtMost()

DairyProducts commented 2 months ago

I got the test to run, but I don't think it's running properly. The following is returned:

 FAIL  src/site/_includes/js/test/eligibility-programs.test.js
  ● Program eligibility › Clipper START Program › Cannot have RTC Clipper Card
    Custom message:
      Checking clipperStartResult with modified value of existing-rtc-clipper-me: undefined
    clipperStartResult returns:
    {
      "enrolled": false,
      "eligible": null,
      "conditions": [
        {
          "desc": "Be 19-64 years old",
          "met": null
        },
        {
          "desc": "Have a household income below $2,510 per month (200% federal poverty level)",
          "met": null
        },
        {
          "desc": "Not have an RTC Clipper Card for people with disabilities",
          "met": true
        }
      ],
      "flags": [
        "MORE_INFO_NEEDED"
      ]
    }

It appears that the conditions "Be 19-64 years old" and "Have a household income below $2510 per month" are being checked for the test "Cannot have RTC Clipper Card", despite those conditions being parts of separate tests. These conditions when checked result in a null return value.

I'm not sure what's wrong with the way I set up the tests such that they would return null, especially when only the age is checked:

      check(elig.clipperStartResult, input).isEligibleIf('age').isAtLeast(elig.cnst.clipper.MIN_ELIGIBLE_AGE);
      check(elig.clipperStartResult, input).isEligibleIf('age').isAtMost(elig.cnst.clipper.MAX_ELIGIBLE_AGE);

Full error log: log (2).txt

trevorshannon commented 2 months ago

The tests will always call clipperStartResult with input and that means every eligibility condition in that function will be exercised. The goal is to set all your inputs to a known state and then twiddle one at a time for each test case. If you want to verify the age portion of your logic, you would set the RTC and income inputs such that they satisfy their eligibility conditions, and then vary the age to see that the result is what you expect.

Each eligibility condition can be true, false, or null (we need more info).

I'm not 100% sure without looking at your code, but I suspect the income eligibility condition is null because input.income.valid == false in the input object you are using in your test. Without valid income input, your program will never return an eligible result, no matter what you do with the age value. Here's an example you can look at. Similarly, when checking one of the other conditions (for example, the RTC Clipper condition) you'll have to be sure you set input.age to some valid value so that you can perturb the condition under test. Perhaps something like input.age = elig.cnst.clipper.MIN_ELIGIBLE_AGE

I also see that somewhere you are twiddling the value of existing-rtc-clipper-me but that looks more like an HTML element ID.

Checking clipperStartResult with modified value of existing-rtc-clipper-me: undefined

I believe you are looking to set existingRtcClipperMe

DairyProducts commented 2 months ago

Adding a valid income input breaks throws the logic input undefined error for the check function:

    test('Age must be from 19 to 64 years', () => {
      input.income.valid = true;
      input.age = elig.cnst.clipper.MIN_ELIGIBLE_AGE;
      check(elig.clipperStartResult, input).isEligibleIf('age').isAtLeast(elig.cnst.clipper.MIN_ELIGIBLE_AGE);
      check(elig.clipperStartResult, input).isEligibleIf('age').isAtMost(elig.cnst.clipper.MAX_ELIGIBLE_AGE);
    });

Removing input.income.valid = true; solves the error but causes the income condition to be null. The check function in this case only concerns 'age', so I don't know why the logic error would be thrown.

trevorshannon commented 2 months ago

This was a tricky one! I think it comes down to a lowercase l versus and uppercase L in cnst.clipper.MAX_ELIGIBLE_AGE.

The constant is defined with a lowercase l but you are setting the age value with the uppercase L in the test. So elig.cnst.clipper.MAX_ELIGIBLE_AGE is the undefined value since only elig.cnst.clipper.MAX_ElIGIBLE_AGE is defined. The eligibility.js logic also uses the lowercase l. Changing your test line to read the following (note the lowercase l) fixes the test (though the proper fix would be to make the constant name all caps):

check(elig.clipperStartResult, input).isEligibleIf('age').isAtMost(elig.cnst.clipper.MAX_ElIGIBLE_AGE);

Note that in the example code you pasted above, this line is not needed:

input.age = elig.cnst.clipper.MIN_ELIGIBLE_AGE;

The age will be set by the check statements so it immediately gets overwritten. You only need to ensure all the other inputs are set properly (i.e. that income is below the limit and valid plus RTC clipper is false. Since RTC clipper is false by default, you should only need to set the income as valid.). In test cases that are testing something other than age, you will need to initialize the age to an appropriate value.

DairyProducts commented 2 months ago

I fixed the typo in eligibility.js but the following code still throws the logic input value undefined error:

  describe('Clipper START Program', () => { // Broken!
    test('Not eligible with default input', () => {
      expect(elig.clipperStartResult(input).eligible).not.toBe(true);
    });

    test('Cannot have RTC Clipper Card', () => {
      input.existingRtcClipperMe = false;
      input.income.valid = true;
      check(elig.clipperStartResult, input).isEligibleIf('existing-rtc-clipper-me').is(false);
    });

    test('Age must be from 19 to 64 years', () => {
      input.income.valid = true;
      check(elig.clipperStartResult, input).isEligibleIf('age').isAtLeast(elig.cnst.clipper.MIN_ELIGIBLE_AGE);
      check(elig.clipperStartResult, input).isEligibleIf('age').isAtMost(elig.cnst.clipper.MAX_ELIGIBLE_AGE);
    });

    test('Income must be at or below the limit', () => {
      input.income.valid = true;
      check(elig.clipperStartResult, input).isEligibleIf('income.wages').isAtMost(elig.cnst.clipper.ANNUAL_INCOME_LIMITS[0] / 12);
    });
  });

But more peculiar is that the console also shows a new uncaught exception (with a log so long I put it into a separate text file). It concerns eligibility-integration.test.js but I haven't modified that yet.

error log.txt

trevorshannon commented 2 months ago

The typo existed in two places in eligibility.js and only one has been fixed, so one still remains.

The long error is indeed triggered by the eligibility integration test, but really the error is coming from eligibility.js (A logic input was undefined)

The integration test basically tests clicking around on the eligibility form to see that the whole thing is working correctly. I think you will find that the integration test is correctly finding an issue here--in other words, if you open a web browser and try going through the whole eligibility tool yourself, you'll also find that there is a problem when you submit. While you did not change eligibility-integration.test.js, you did change the code it's testing (eligibility.js)

The error is incredibly verbose, but hidden in there is this section:

detail: Error: A logic input value was undefined
            at throwIfUndefined (C:\Users\Derek\Desktop\theunitedeffort.org\src\site\_includes\js\eligibility.js:288:11)
            at throwIfUndefined (C:\Users\Derek\Desktop\theunitedeffort.org\src\site\_includes\js\eligibility.js:399:3)
            at HTMLLIElement.le [as result] (C:\Users\Derek\Desktop\theunitedeffort.org\src\site\_includes\js\eligibility.js:2670:5)
            at result (C:\Users\Derek\Desktop\theunitedeffort.org\src\site\_includes\js\eligibility.js:3036:28)
            at HTMLButtonElement.computeEligibility (C:\Users\Derek\Desktop\theunitedeffort.org\src\site\_includes\js\eligibility.js:1098:3)

which points you to line 2670 in eligibility.js (the lowercase l typo)

by the way, I saw in your latest commit that the Cannot have RTC Clipper Card test is still trying to tweak the value of existing-rtc-clipper-me instead of existingRtcClipperMe

DairyProducts commented 2 months ago

Thanks! I don't know how I didn't catch that.

Another problem with the following test:

    test('Cannot have RTC Clipper Card', () => {
      input.existingRtcClipperMe = false;
      input.income.valid = true;
      input.age = elig.cnst.clipper.MIN_ELIGIBLE_AGE;
      check(elig.clipperStartResult, input).isEligibleIf('existingRtcClipperMe').is(false);
    });

This test fails because of the expect(received).not.toBe(expected) expression, where the expected value is not true while the actual value is true (all conditions are met). For the other tests I created, the expected value has been true with the applicable expression being expect(received).toBe(expected). As a whimsical solution I changed the last line to check(elig.clipperStartResult, input).isEligibleIf('existingRtcClipperMe').is(true); but the same result occurs. I'm not quite sure what makes the expected value be not true for eligible while for all the other tests the expected value is true.

trevorshannon commented 2 months ago

When the check(...).isEligibleIf(foo).is(false) stuff runs, it first checks that the program is ineligible so that the test can verify that when input.foo is changed to false the program does indeed become eligible. If the input already leads to an eligible output when the check is done, then the test can't be sure that tweaking the value of interest is what makes the output eligible.

The problem here is that you are setting up your initial conditions in such a way that the result is already eligible, then checking that the result becomes eligible when setting a certain input value to false. But it's already eligible so the test can't really test what you want.

There are two solutions here. The first is to keep your initial conditions as-is and instead check for a switch to ineligibility:

check(elig.clipperStartResult, input).isNotEligibleIf('existingRtcClipperMe').is(true);

The second solution is to keep your check as-is and instead initialize the RTC clipper input to true:

input.existingRtcClipperMe = true;
DairyProducts commented 2 months ago

Thanks for the explanation! I implemented the first solution suggested, and all checks relating to eligibility-programs.test.js now pass. There are still two existing failed checks in eligibility-integration.test.js, however.

The first one has to do with the eligibility results page, as it states the following:

Expected substring: "checked 19 programs"
    Received string:    "·······
            We checked 20 programs and found 1 you may qualify for and are not yet receiving., but based on your responses we were not able to find any matching programs for you.·······
          No-Fee ID Card
          We need additional information from you to assess 5 programs. There are also 13 programs you likely do not qualify for and 1 program you're already enrolled in.  You can see the whole list of programs we checked below.
        "

I suspect this has something to do with the fact I added a new program but didn't update the number of programs/register the program in a list of some sort, and thus the mismatched numbers. I'm not sure how to fix this though.

The second failed test has to do with the following: buildInputObj › Sets all data from page elements Which seems to be upset by the addition of the two programs and some value relating to them being set to false:

    expect(received).toEqual(expected) // deep equality

    - Expected  - 0
    + Received  + 2

    @@ -58,10 +58,12 @@
       ...
        "existingPhaHousehold": true,
        "existingPhaMe": true,
    +   "existingRtcClipperHousehold": false,
    +   "existingRtcClipperMe": false,
        "existingSchipHousehold": true,
        "existingSchipMe": true,
       ...
trevorshannon commented 2 months ago

Yes, you are on the right track here. eligibility-integration.test.js has a hardcoded string that it compares against for that particular test. This is sort of a "change detector" test in that when a certain type of change is made, the test will start failing. Since you did add a program, the fact that it went from 19 to 20 is expected, so you simply need to update the expected string.

As for the other test, it is checking whether the buildInputObj function works as expected. We expect all input values to be correctly stored by that function, and if any are missed (or if there are any extras) the test will fail. In this case the test got back an object with two extra (as far as it's concerned) properties: existingRtcClipperHousehold and existingRtcClipperMe. You will need to update the concept of "what's expected" by the test by adding those parameters to the expected output object. You should also update the body of that test to set those two values as true by checking the associated checkboxes. You can use one of the other properties to guide you.