Benchmark creation #298

codetheweb · 2024-03-03T23:04:57Z

Screen.Recording.2024-03-03.at.8.27.03.PM.mov

Future improvements:

fix breadcrumbs
finish benchmark listing page (/goals/[goal_id])
input validation
re-use same form for editing benchmarks

tessathornberry

This looks great, @codetheweb. Other than maybe cleaning up any minor "this variable was defined but never used" github messages which are noted in your changed files here, this is good as is for this step. Nice work!
OF COURSE, me writing that includes that I agree with the "future fixes" you mentioned in the notes on your PR. :D

src/backend/db/migrations/1_initial-migrations.sql

tessathornberry · 2024-03-06T00:53:49Z

src/components/goal/Goal.tsx

@@ -51,7 +52,7 @@ const Goals = ({ goal }: GoalProps) => {
              gap: "8px",
            }}
          >
-            <button
+            <Link
              // standin for tertiary button style
              style={{
                display: "inline-flex",


☀️ We may add to a future ticket to move all this CSS into the sibling CSS file rather than having it in the components, but that goes for the whole app. Plus behavior may vary when combined with MUI components, so we can put that off a bit longer.

src/components/goal/Goal.tsx

tessathornberry · 2024-03-06T01:01:24Z

src/pages/students/[student_id]/goals/[goal_id].tsx

+    <div>
+      {goal && (
+        <GoalHeader
+          name="[placeholder] 1st Goal"


☀️ Future functions may track which goal and change wording to "Goal 1" etc.

tessathornberry · 2024-03-06T01:08:37Z

src/pages/students/[student_id]/goals/[goal_id]/create.tsx

+  const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => {
+    e.preventDefault();
+    const form = e.currentTarget as HTMLFormElement;
+    const formData = new FormData(form);


☀️ This is cool

src/pages/students/[student_id]/goals/[goal_id]/create.tsx

tessathornberry · 2024-03-06T01:11:44Z

src/pages/students/[student_id]/goals/[goal_id]/create.tsx

+          </Stack>
+        </Stack>
+
+        <Divider />


☀️ ok, these are cool

src/pages/students/[student_id]/goals/[goal_id]/create.tsx

KCCPMG

Mild conflict with the [goal_id] page in PR #272 which was just merged. Quick summary of those differences from the perspective of what was just merged:

Clicking on a goal goes to goals/[goal_id] instead of students/[student_id]/goals/[goal_id]. What you have here seems much better to me, especially considering that breadcrumbs are now working.
The [goal_id] page recently merged in was largely following this design, which contains the goal editing functionality. Nothing in the benchmark section has been implemented yet aside from the selectable tabs, but according to that original design this is where that add benchmark button should go.

…compass into feat-create-benchmark

…als/[goal_id] and students/[student_id]/goals/[goal_id], edit Iep component to temporarily redirect to new combination page

… temp_goals and goals pages folder

src/backend/lib/db_helpers/case_manager.ts

katconnors · 2024-05-10T21:53:02Z

There's a bit of a bug on github's end where I can only view the file changes for the 7 total commits (the 3 original ones, plus the 4 recent ones). Alternatively, is it possible to submit the PR to only include the changes that are within the scope of the issue? (I believe there are 9 commits for other PR's, since this PR references 16 commits)

KCCPMG · 2024-05-10T22:03:47Z

There's a bit of a bug on github's end where I can only view the file changes for the 7 total commits (the 3 original ones, plus the 4 recent ones). Alternatively, is it possible to submit the PR to only include the changes that are within the scope of the issue? (I believe there are 9 commits for other PR's, since this PR references 16 commits)

I'm not quite sure I understand....it sounds like you can see Max's original 3 commits, can't see the commits from merging main back into this branch to re-sync, and then you can see the 4 commits that I made since then?

katconnors · 2024-05-10T22:10:37Z

On the functionality side, I haven't been able to add a benchmark.
On my first attempt, I filled in all fields except for "metric to track", and I received this error:

I've since been unable to replicate the error, but the benchmarks don't appear when I submit the form (even with all fields completed).

katconnors · 2024-05-10T22:12:26Z

There's a bit of a bug on github's end where I can only view the file changes for the 7 total commits (the 3 original ones, plus the 4 recent ones). Alternatively, is it possible to submit the PR to only include the changes that are within the scope of the issue? (I believe there are 9 commits for other PR's, since this PR references 16 commits)

I'm not quite sure I understand....it sounds like you can see Max's original 3 commits, can't see the commits from merging main back into this branch to re-sync, and then you can see the 4 commits that I made since then?

For my review, I wanted to isolate the file changes to just the 4 commits you've made, because Max's commits already had reviews. I would assume it's a bug, because I can do a number of other combinations within the selections.

KCCPMG · 2024-05-10T22:18:09Z

On the functionality side, I haven't been able to add a benchmark. On my first attempt, I filled in all fields except for "metric to track", and I received this error: I've since been unable to replicate the error, but the benchmarks don't appear when I submit the form (even with all fields completed).

You may need to re-run the migrations file. I believe that Max's earlier commits included an alternation of the schema in initial migrations.

katconnors · 2024-05-13T01:00:18Z

I re-ran the migrations file and the feature worked for me. I took a look at the three latest commits, and don't have any suggestions.

… to prevent clicking through two pages

codetheweb added 3 commits March 3, 2024 15:02

Create benchmark page

8018624

Add goals placeholder page

e9efb21

Add mutation

b3fe5cb

codetheweb force-pushed the feat-create-benchmark branch from c8d3907 to b3fe5cb Compare March 4, 2024 04:30

codetheweb linked an issue Mar 4, 2024 that may be closed by this pull request

Benchmark creation page #281

Closed

codetheweb marked this pull request as ready for review March 4, 2024 04:33

codetheweb requested review from tessathornberry and KCCPMG and removed request for tessathornberry March 4, 2024 04:33

tessathornberry approved these changes Mar 6, 2024

View reviewed changes

KCCPMG requested changes Apr 9, 2024

View reviewed changes

KCCPMG requested review from hieungo89 and removed request for hieungo89 April 23, 2024 01:48

KCCPMG added 4 commits April 22, 2024 21:18

Merge branch 'feat-create-benchmark' of https://github.com/sfbrigade/…

9a08c0f

…compass into feat-create-benchmark

create students/[student_id]/temp_goals[goal_id] as combination of go…

eabec95

…als/[goal_id] and students/[student_id]/goals/[goal_id], edit Iep component to temporarily redirect to new combination page

move changes safely into students/[student_id]/goals[goal_id], delete…

369c2b3

… temp_goals and goals pages folder

import cleanup in [goal_id], add comment in [goal_id]/create

30a4440

KCCPMG requested review from KCCPMG, hieungo89 and tessathornberry and removed request for KCCPMG April 25, 2024 01:26

tessathornberry requested a review from katconnors May 9, 2024 02:56

tessathornberry mentioned this pull request May 9, 2024

Ds checkboxes #321

Merged

katconnors reviewed May 10, 2024

View reviewed changes

src/backend/lib/db_helpers/case_manager.ts Outdated Show resolved Hide resolved

KCCPMG removed their request for review May 13, 2024 22:08

KCCPMG approved these changes May 13, 2024

View reviewed changes

KCCPMG added 2 commits May 13, 2024 15:11

Merge branch 'main' into feat-create-benchmark

8793728

add stop propagation to 'Add Benchmark' Link components/goal/Goal.tsx…

a7c1b70

… to prevent clicking through two pages

KCCPMG merged commit 798e040 into main May 14, 2024
3 checks passed

KCCPMG deleted the feat-create-benchmark branch May 14, 2024 00:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Benchmark creation #298

Benchmark creation #298

codetheweb commented Mar 3, 2024 •

edited

Loading

tessathornberry left a comment

tessathornberry Mar 6, 2024

tessathornberry Mar 6, 2024

tessathornberry Mar 6, 2024

tessathornberry Mar 6, 2024

KCCPMG left a comment

katconnors commented May 10, 2024 •

edited

Loading

KCCPMG commented May 10, 2024

katconnors commented May 10, 2024

katconnors commented May 10, 2024

KCCPMG commented May 10, 2024

katconnors commented May 13, 2024

Benchmark creation #298

Benchmark creation #298

Conversation

codetheweb commented Mar 3, 2024 • edited Loading

tessathornberry left a comment

Choose a reason for hiding this comment

tessathornberry Mar 6, 2024

Choose a reason for hiding this comment

tessathornberry Mar 6, 2024

Choose a reason for hiding this comment

tessathornberry Mar 6, 2024

Choose a reason for hiding this comment

tessathornberry Mar 6, 2024

Choose a reason for hiding this comment

KCCPMG left a comment

Choose a reason for hiding this comment

katconnors commented May 10, 2024 • edited Loading

KCCPMG commented May 10, 2024

katconnors commented May 10, 2024

katconnors commented May 10, 2024

KCCPMG commented May 10, 2024

katconnors commented May 13, 2024

codetheweb commented Mar 3, 2024 •

edited

Loading

katconnors commented May 10, 2024 •

edited

Loading