Update coaching sessions #96

calebbourg · 2025-04-02T11:01:16Z

Description

This PR adds the ability to update an existing Coaching Session.

It also adds a new type of menu to the coaching session component. Open to suggestion on if this is what we want.

Changes

Updated the add-new-coaching-session dialog component to be re-usable for creating new coaching session and updating existing coaching sessions
Adds a "3 dot" menu to the coaching sessions component
Adds a "Delete Coaching Session" link that will be used later

Screenshots / Videos Showing UI Changes (if applicable)

Testing Strategy

tested locally

Concerns

describe any concerns that might be worth mentioning or discussing

zgavin1 · 2025-04-02T15:53:51Z

src/components/ui/dashboard/coaching-session-dialog.tsx

+        <form onSubmit={handleSubmit} className="space-y-4">
+          <div className="space-y-2">
+            <Label htmlFor="session-date">Session Date</Label>
+            <Calendar
+              mode="single"
+              selected={sessionDate}
+              onSelect={(date) => setSessionDate(date)}
+            />
+          </div>
+          <div className="space-y-2">
+            <Label htmlFor="session-time">Session Time</Label>
+            <input
+              type="time"
+              id="session-time"
+              value={sessionTime}
+              onChange={(e) => setSessionTime(e.target.value)}
+              className="w-full border rounded p-2"
+              required
+            />
+          </div>
+          <Button type="submit" disabled={!sessionDate || !sessionTime}>
+            {mode === "create" ? "Create Session" : "Update Session"}
+          </Button>
+        </form>


We probably want the form itself (and its submit logic) moved into it's own react component and then composed into the dialog component. Separates concerns of the dialog UI vs the form pretty nicely that way.

<CoachingSessionDialog ...props > <CoachingSessionForm ...props /> </CoachingSessionDialog>

I think that's a sensible suggestion Zach.

Makes total sense. This is my pass at separating the two dea2db8

zgavin1 · 2025-04-02T15:56:13Z

src/lib/api/actions.ts

+   * @param actionId The ID of the action to delete
+   * @returns Promise resolving to the deleted Action object
+   */
+  deleteNested: async (_entityId: Id, _actionId: Id): Promise<Action> => {


forgive me I just haven't noticed this until now - what are the underscore prefixes on the params? Just acknowledging the api method isn't implemented yet?

That's a convention that I see in other languages that just means that the arguments aren't actually going to be used in the function (because, like you said, it's not implemented)

zgavin1 · 2025-04-02T15:56:48Z

src/lib/api/coaching-sessions.ts

+   */
+  deleteNested: async (
+    _entityId: Id,
+    coachingSessionId: Id


does this need the underscore?

jhodapp · 2025-04-02T19:07:46Z

@calebbourg Thanks for this, it's nice to see this functionality in place!!

Can you move the "Join Session" back out into a primary button that sits just to the left of the new vertical "..." menu and change the text to "Join"? And then change "Edit Session" and "Delete Session" to "Edit" and "Delete"?

jhodapp

Looking great, a few inline suggestions.

jhodapp · 2025-04-03T16:28:36Z

src/components/ui/coaching-session.tsx

  coachingSession: {
    id: Id;
    date: string;
+    coaching_relationship_id: Id;
+    created_at: DateTime;
+    updated_at: DateTime;
  };


Change this to use the type defined in src/types/coaching-session.ts

yup e0074c6

jhodapp · 2025-04-03T16:30:31Z

src/components/ui/coaching-session.tsx

+        onOpenChange={setUpdateDialogOpen}
+        onCoachingSessionUpdated={() => {
+          // Refresh the list of coaching sessions
+          window.location.reload();


Is there a hook for the coaching sessions that we can call refresh() on instead of this? I may be confused on what this is for otherwise.

Yes this was something that AI added that I didn't see. Good catch! dea2db8

jhodapp · 2025-04-03T16:33:22Z

src/components/ui/dashboard/coaching-session-dialog.tsx

+        <form onSubmit={handleSubmit} className="space-y-4">
+          <div className="space-y-2">
+            <Label htmlFor="session-date">Session Date</Label>
+            <Calendar
+              mode="single"
+              selected={sessionDate}
+              onSelect={(date) => setSessionDate(date)}
+            />
+          </div>
+          <div className="space-y-2">
+            <Label htmlFor="session-time">Session Time</Label>
+            <input
+              type="time"
+              id="session-time"
+              value={sessionTime}
+              onChange={(e) => setSessionTime(e.target.value)}
+              className="w-full border rounded p-2"
+              required
+            />
+          </div>
+          <Button type="submit" disabled={!sessionDate || !sessionTime}>
+            {mode === "create" ? "Create Session" : "Update Session"}
+          </Button>
+        </form>


I think that's a sensible suggestion Zach.

src/components/ui/dashboard/coaching-session-dialog.tsx

calebbourg · 2025-04-04T12:33:05Z

@jhodapp I moved the join session button back out in e18bfed

Let me know how this works for now.

jhodapp · 2025-04-04T17:20:14Z

@jhodapp I moved the join session button back out in e18bfed

Let me know how this works for now.

@calebbourg

Closer, but now I see the "Create New Coaching Session" button repeated above every session. Are you seeing this too?

jhodapp · 2025-04-04T17:24:35Z

@calebbourg Your latest changes also seem to degrade the AddEntity component up top on the Dashboard page as well:

calebbourg · 2025-04-04T17:24:49Z

@jhodapp oh haha for some reason I thought that had something to do with changes you were making on the dashboard being in an in between state.. I see now that that would be really weird :) I'll fix it

jhodapp · 2025-04-04T17:26:04Z

@jhodapp oh haha for some reason I thought that had something to do with changes you were making on the dashboard being in an in between state.. I see now that that would be really weird :) I'll fix it

Not yet, I haven't made any changes yet. Thanks brother!

calebbourg · 2025-04-05T12:13:15Z

@jhodapp @zgavin1 I took another crack at this. I realized that we were using the CoachingSessionDialog component in multiple places throughout the larger dashboard page and so attempted to declare it once and provide hooks for the various other components to call into it's functionality. I defined the hooks in the top level dashboard page. In order to do that I needed to separate some of the code into a client rendered function. that is called by page.tsx which remains server-rendered. I think that's a reasonable approach but open to thoughts.

Another spin-off of this was that I removed the "Create Coaching Session" button from the actual list of coaching sessions as It seemed redundant given the add-entities buttons above.

jhodapp · 2025-04-05T16:29:27Z

Another spin-off of this was that I removed the "Create Coaching Session" button from the actual list of coaching sessions as It seemed redundant given the add-entities buttons above.

@calebbourg this is awesome, it was something I was going to do anyway when I moved the coaching sessions list down in the page for exactly the reason you mentioned. Thanks for doing this!

jhodapp

Such an awesome change set, thank you Caleb. Just a few more suggestions and I'll be ready to approve.

jhodapp · 2025-04-05T16:34:24Z

src/app/dashboard/dashboard-content.tsx

+          <AddEntities onCreateSession={() => handleOpenDialog()} />
+        </div>
+      </div>
+      <DashboardContainer>


I'd like to suggest that we should merge DashboardContainer and DashboardContent and I suggest DashboardContainer as the name since we have precedence for this already. Unless I'm misunderstanding things, I don't see any value or requirement to have the extra div layer.

I do think you did take the right approach though in making the Dashboard page as simple as possible and trying to keep it rendered on the server side.

I did some updating here, but not all of what I suggested. But you'll want to check it out so we don't step on each other directly: https://github.com/refactor-group/refactor-platform-fe/pull/102/files#diff-1b7538b3757a933d976e1a796af3bbf3bc3895ae83f60d5a0562db4d2f6bbc02

@jhodapp thanks for the heads up! It looks like that branch is based off of this one right? Do you have a preference of making this change in this branch vs yours?

@calebbourg It is based off of this one indeed. I'm good either way. Maybe it makes the most sense to retarget my PR to merge into yours instead of into main?

ok made this change e3ef1f5

jhodapp · 2025-04-05T16:38:01Z

src/components/ui/dashboard/add-entities.tsx

+        <AddCoachingSessionButton
+          disabled={!isCoach || !currentOrganizationId}
+          onClick={onCreateSession}


Yes!! This fulfills my TODO item, so thank you for refactoring this to match the style of AddMemberButton. What a pleasant surprise to discover. :)

jhodapp · 2025-04-05T16:41:33Z

src/components/ui/coaching-session.tsx

-                  </Link>
-                </DropdownMenuItem>
-                <DropdownMenuItem onClick={() => setUpdateDialogOpen(true)}>
+                <DropdownMenuItem onClick={onUpdate}>


To be consistent with Agreements and Actions, can you update the drop down menu item text to be "Edit" and "Delete" instead of "Update Session" and "Delete Session"?

Updated 536a763

Delete coaching sessions.

…age. Moves CoachingSessionList left. Adds a tab bar splitting sessions out by upcoming/previous sessions.

…or the layout of this component on the Dashboard page, and make logout more robust.

…rer and nicer to read.

…ors_and_enhance_sessions_list

jhodapp

Awesome set of changes, thanks Caleb!

calebbourg self-assigned this Apr 2, 2025

calebbourg requested review from jhodapp and zgavin1 April 2, 2025 11:01

calebbourg added the feature work Specifically implementing a new feature label Apr 2, 2025

calebbourg added this to Refactor Coaching Platform Apr 2, 2025

calebbourg added this to the 1.0-beta1 milestone Apr 2, 2025

zgavin1 reviewed Apr 2, 2025

View reviewed changes

calebbourg changed the base branch from main to 88-feature-add-member-management-page-delete-users April 2, 2025 16:02

jhodapp moved this to Review in Refactor Coaching Platform Apr 2, 2025

jhodapp requested changes Apr 3, 2025

View reviewed changes

calebbourg force-pushed the 88-feature-add-member-management-page-delete-users branch from accebcf to dfa4cb3 Compare April 4, 2025 10:48

calebbourg force-pushed the update_coaching_sessions branch from e5142b5 to e0074c6 Compare April 4, 2025 12:10

Base automatically changed from 88-feature-add-member-management-page-delete-users to main April 4, 2025 17:13

calebbourg added 4 commits April 4, 2025 13:14

add the ability to update a coaching session

6f0be1a

separate form from dialog

3b21b68

PR comments

413981c

move join session button back out of dropdown

f18b415

calebbourg force-pushed the update_coaching_sessions branch from e18bfed to f18b415 Compare April 4, 2025 17:14

calebbourg added 2 commits April 5, 2025 07:18

fix regressions from updating coaching session dialog

704b933

refactor coaching session creation/updates

e9cb3e7

calebbourg added 2 commits April 5, 2025 09:10

refresh coaching session list after delete

d79bbce

only coaches can delete coaching sessions

dbc6966

jhodapp requested changes Apr 5, 2025

View reviewed changes

jhodapp mentioned this pull request Apr 5, 2025

Delete coaching sessions #100

Merged

jhodapp added 4 commits April 5, 2025 15:22

Merge pull request #100 from refactor-group/delete_coaching_sessions

a17885a

Delete coaching sessions.

Removes the SelectCoachingRelationship component from the Dashboard p…

962040b

…age. Moves CoachingSessionList left. Adds a tab bar splitting sessions out by upcoming/previous sessions.

Move the CoachingSessionSelector into the CoachingSessionList, refact…

3982109

…or the layout of this component on the Dashboard page, and make logout more robust.

Clean up the select coaching relationship placeholder text to be clea…

c851b3a

…rer and nicer to read.

jhodapp mentioned this pull request Apr 7, 2025

Refactor the Dashboard page #102

Closed

jhodapp and others added 9 commits April 7, 2025 17:13

We no longer need to debug print relationships.

28cd840

We no longer need to debug print relationships in useEffect either.

e29da27

Handle loading/loading error cases better for the coaching session list.

6e64e99

use edit and delete language

536a763

use edit and delete language

0630dbb

Address review comments

66000f8

Merge pull request #104 from refactor-group/remove_org_and_rel_select…

14ddebb

…ors_and_enhance_sessions_list

merge dashboard-content and dashboard-container

e3ef1f5

Move the DashboardContainer component into the components directory.

7c6ada9

jhodapp approved these changes Apr 9, 2025

View reviewed changes

jhodapp merged commit a536f36 into main Apr 9, 2025
4 checks passed

jhodapp deleted the update_coaching_sessions branch April 9, 2025 15:50

github-project-automation bot moved this from Review to ✅ Done in Refactor Coaching Platform Apr 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update coaching sessions #96

Update coaching sessions #96

calebbourg commented Apr 2, 2025

zgavin1 Apr 2, 2025

jhodapp Apr 3, 2025

calebbourg Apr 4, 2025

zgavin1 Apr 2, 2025

calebbourg Apr 4, 2025

zgavin1 Apr 2, 2025

jhodapp commented Apr 2, 2025

jhodapp left a comment

jhodapp Apr 3, 2025

calebbourg Apr 4, 2025

jhodapp Apr 3, 2025

calebbourg Apr 4, 2025

jhodapp Apr 3, 2025

calebbourg commented Apr 4, 2025

jhodapp commented Apr 4, 2025

jhodapp commented Apr 4, 2025

calebbourg commented Apr 4, 2025

jhodapp commented Apr 4, 2025

calebbourg commented Apr 5, 2025

jhodapp commented Apr 5, 2025

jhodapp left a comment

jhodapp Apr 5, 2025

jhodapp Apr 8, 2025

calebbourg Apr 8, 2025

jhodapp Apr 8, 2025

calebbourg Apr 9, 2025

jhodapp Apr 5, 2025

jhodapp Apr 5, 2025

calebbourg Apr 8, 2025

jhodapp left a comment

Update coaching sessions #96

Update coaching sessions #96

Conversation

calebbourg commented Apr 2, 2025

Description

Changes

Screenshots / Videos Showing UI Changes (if applicable)

Testing Strategy

Concerns

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

jhodapp commented Apr 2, 2025

jhodapp 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

calebbourg commented Apr 4, 2025

jhodapp commented Apr 4, 2025

jhodapp commented Apr 4, 2025

calebbourg commented Apr 4, 2025

jhodapp commented Apr 4, 2025

calebbourg commented Apr 5, 2025

jhodapp commented Apr 5, 2025

jhodapp 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

jhodapp left a comment

Choose a reason for hiding this comment