Added editIEP to the front and back end and wrote test. Future builds will include these in a modal. #305

tessathornberry · 2024-03-29T00:19:59Z

This closes ticket #212

Iep layout is closer to design systems layout
Button for archive is moved out to Student instead of inside edit.
Edit items have been put in order of creation
Edit IEP options entered into form (CSS will be finalized/updated with Modal creating in next ticket)
Edit IEP trpc function created
Ava test for editIep created and passed

… will include these in a modal. Co-authored-by: katconnors <[email protected]>

hieungo89 · 2024-03-30T00:07:15Z

src/components/design_system/button/Button.module.css

+  cursor: pointer;
+  text-decoration: underline;
+  text-align: center;
+  font-size: 16px;


I would advise having font size as either 1 rem or the global size of var(--h6) so that it's scalable with smaller screens.

Great call, @hieungo89 ! I reused the code for the secondary button and don't know why that became pixels. The secondary button font size is var(--base-size). Would that work?

hieungo89 · 2024-03-30T00:36:19Z

src/backend/routers/student.ts

+      const { userId } = req.ctx.auth;
+
+      // Check if the student exists and if the case manager is assigned to the student
+      const existingStudent = req.ctx.db


I think you might need to also add .execute() to run this method.

hieungo89 · 2024-03-30T00:38:55Z

src/backend/routers/student.ts

+        .where("student_id", "=", student_id)
+        .returningAll()
+        .executeTakeFirstOrThrow();
+      return result;


Do we need to retrieve data directly back from this editIep method? If not, then a return statement is not needed or a result variable. It seems like the frontend method is already retrieving the new data after the edition is saved.

Great call, it works well without it and I agree, thank you!

hieungo89 · 2024-03-30T00:43:05Z

src/backend/routers/student.test.ts

+  const start_date = new Date("2023-01-01");
+  const end_date = new Date("2023-12-31");
+
+  const added = await trpc.student.addIep.mutate({


Lint advised removing the const since added is not being used elsewhere.

hieungo89 · 2024-03-30T00:47:46Z

src/backend/routers/student.test.ts

+
+  t.is(got.length, 1);
+  t.is(got[0].student_id, seed.student.student_id);
+  t.deepEqual(got[0].start_date, updated.start_date);


I would suggest doing a check for the start & end dates before the updated dates, and then showing the updated start & end dates are not the same as the previously added dates. You also don't need a const for updated since it shouldn't return anything. You should get your data with the .getIeps.query() instead.

Just came here to say the same thing. I was playing around with this and tried to compare updated.start_date with updated_start_date and got the following difference showing in the test log:

Difference (- actual, + expected):

Date 2023-03-01 08:00:00 UTC {}

Date 2023-03-02 00:00:00 UTC {}

@hieungo89 , "updated" inserts the data into the iep keys, it doesn't retrieve anything. I can make "got" a let variable and reuse it after the data has been edited. re: your previous comment, if I make a test for checking the starting values, I will use "added" for that test. I can take out "updated" and simply have the await portion of the mutation of editIep, re-calling the "getIeps" with the "got" variable so I can use the new value in "notDeepEqual" tests.
Let me know if what I do is close to what you had in mind? I'll push it up once done with all comments.

KCCPMG · 2024-04-02T00:54:42Z

src/backend/routers/student.ts

+        .selectAll()
+        .where("student_id", "=", student_id)
+        .where("assigned_case_manager_id", "=", userId);
+      if (!existingStudent) {


I could be wrong and I haven't checked this, but if you're using selectAll, then doesn't this query return an array? If so, then I believe that existingStudent will always be truthy as an array with length of 0. I think you want to check if existingStudent.length === 0 rather than the current check, but either way I'd recommend adding tests to make sure that the error is being thrown for a missing or invalid student.

selectAll() would be selecting all the key:value pairs from all the matches in the db. However, I mentioned above that there might not be any returned data due to not having the execute() statement. I do agree with you that checking the length is more accurate.

Good call, @KCCPMG and @hieungo89! I admit that I snagged quite a bit of this from the case_manager.editStudent API, which has the same format.

FYI,

how about

if (!existingStudent[0].student_id) { throw new Error("Student not found under this Case Manager"); }

since existingStudent =

[ { student_id: '9eaa9500-a2a9-4704-b917-7b8c2f28f104', first_name: 'student', last_name: 'one', email: '[email protected]', assigned_case_manager_id: 'a5663b61-691b-49d6-8322-1b3e3f4ba79f', grade: 5 } ]

Length checks would be dicey on undefined properties since it may cause a different kind of error.

I noted that the major issue was how the seed data was populating the student table, ensuring the assigned_case_manager_id was NOT null.

selectAll() would be selecting all the key:value pairs from all the matches in the db. However, I mentioned above that there might not be any returned data due to not having the execute() statement. I do agree with you that checking the length is more accurate.

RE: Kysley:

const persons = await db .selectFrom('person') .selectAll() .execute()

translates to:

SELECT * FROM "person";

You have to have a selectFrom and a select WHAT, even if you're not returning anything. Also, the execute() can be any of the execute commands and it will return something, even if you do not need anything returned.

I ended up doing the conditional on !existingStudent[0]: please let me know if this works for you.

I checked, and the return from existingStudent is either array with 0-1 data point. I believe this should be fine. 👍

…and student test files

… so that asserts were done against the correct data, based on authenticated procedures for the case manager

hieungo89 · 2024-04-05T03:58:26Z

src/backend/routers/student.ts

+          end_date: end_date,
+        })
+        .where("student_id", "=", student_id)
+        .executeTakeFirstOrThrow();


I think just .execute() is enough since this is not returning anything either.

src/pages/students/[student_id].tsx

hieungo89 · 2024-04-05T04:47:31Z

Test looks good!

src/backend/routers/student.test.ts

KCCPMG · 2024-04-05T05:51:08Z

src/backend/routers/student.test.ts

+  t.notDeepEqual(got[0].start_date, iep.start_date);
+  t.notDeepEqual(got[0].end_date, iep.end_date);
+});
+


I think that it would be ideal to add a few other tests here to make sure that errors are being thrown in the right circumstances, specifically when an edit attempt is made on a non-existent student, and when an edit attempt is made on a student that is not assigned to the case manager. This would be beyond the standard of what I believe has been done in similar situations though, so I wouldn't consider it to be essential by any stretch.

Sure, I have added an isDeepEqual back in as well as wrapping the new Date strings with a parseISO from date-fns, which adds more specific time-zone-related information to the date string and handles the date mismatch in the test environment. (It is already correct in the frontend handling)

As for the tests re: the existing student, that is already handled in the editIep function itself and as you say, is outside of the scope of this test.

Right, the thing that I'm suggesting is adding an additional test to make sure that the error is being thrown, rather than expanding the editIep test. If you wanted to do that, here's one that I believe works:

`test("editIep correctly throws for non-existent student", async (t) => {
const { trpc, seed } = await getTestServer(t, {
authenticateAs: "case_manager",
});

const updated_start_date = new Date(parseISO("2023-03-02"));
const updated_end_date = new Date(parseISO("2024-03-01"));

await t.throwsAsync(
trpc.student.editIep.mutate({
student_id: randomUUID(), //random uuid not matching any valid document
start_date: updated_start_date,
end_date: updated_end_date
}), {message: "Student not found under this Case Manager"}
);
})`
Like I said above, not essential, so I'll drop this here and mark it as resolved.

Also the date fix looks good!

…routes/case_manager.editStudent similarly to routes/student.editIep to pass tests. Updated pages/students/[student_id] to preload any current iep dates, for endDate to have the startDate as a minimum value, and for that to adjust as the startDate changes

Added editIEP to the front and back end and wrote test. Future builds…

9b16b77

… will include these in a modal. Co-authored-by: katconnors <[email protected]>

tessathornberry requested review from hieungo89, codetheweb and BeeSeeWhy March 29, 2024 00:20

hieungo89 reviewed Mar 30, 2024

View reviewed changes

KCCPMG reviewed Apr 2, 2024

View reviewed changes

sfbrigade deleted a comment from tessathornberrypadg Apr 4, 2024

tessathornberrypadg added 2 commits April 4, 2024 16:08

updated per code review suggestions: revised editIep API for student …

4fb3049

…and student test files

updated how editIep test was functioning, populating the student data…

a0f033d

… so that asserts were done against the correct data, based on authenticated procedures for the case manager

tessathornberry requested review from hieungo89 and KCCPMG April 5, 2024 00:51

hieungo89 reviewed Apr 5, 2024

View reviewed changes

src/pages/students/[student_id].tsx Show resolved Hide resolved

KCCPMG requested changes Apr 5, 2024

View reviewed changes

tessathornberry requested review from hieungo89 and KCCPMG April 5, 2024 18:27

KCCPMG approved these changes Apr 6, 2024

View reviewed changes

hieungo89 approved these changes Apr 7, 2024

View reviewed changes

tessathornberry merged commit 3ac6b58 into main Apr 8, 2024
3 checks passed

tessathornberry deleted the editIep branch April 8, 2024 23:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added editIEP to the front and back end and wrote test. Future builds will include these in a modal. #305

Added editIEP to the front and back end and wrote test. Future builds will include these in a modal. #305

tessathornberry commented Mar 29, 2024 •

edited

Loading

hieungo89 Mar 30, 2024

tessathornberry Apr 4, 2024

hieungo89 Mar 30, 2024

tessathornberry Apr 4, 2024

hieungo89 Mar 30, 2024

tessathornberry Apr 4, 2024

hieungo89 Mar 30, 2024

hieungo89 Mar 30, 2024

KCCPMG Apr 2, 2024

tessathornberry Apr 4, 2024

KCCPMG Apr 2, 2024

hieungo89 Apr 3, 2024

tessathornberry Apr 4, 2024 •

edited

Loading

tessathornberry Apr 4, 2024

tessathornberry Apr 5, 2024

tessathornberry Apr 5, 2024

hieungo89 Apr 5, 2024

hieungo89 Apr 5, 2024

hieungo89 commented Apr 5, 2024

KCCPMG Apr 5, 2024

tessathornberry Apr 5, 2024 •

edited

Loading

KCCPMG Apr 6, 2024

Added editIEP to the front and back end and wrote test. Future builds will include these in a modal. #305

Added editIEP to the front and back end and wrote test. Future builds will include these in a modal. #305

Conversation

tessathornberry commented Mar 29, 2024 • edited Loading

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

Choose a reason for hiding this comment

tessathornberry Apr 4, 2024 • edited Loading

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

hieungo89 commented Apr 5, 2024

Choose a reason for hiding this comment

tessathornberry Apr 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tessathornberry commented Mar 29, 2024 •

edited

Loading

tessathornberry Apr 4, 2024 •

edited

Loading

tessathornberry Apr 5, 2024 •

edited

Loading