Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add detour notifications to notifications server #2815

Merged
merged 2 commits into from
Sep 27, 2024
Merged

feat: add detour notifications to notifications server #2815

merged 2 commits into from
Sep 27, 2024

Conversation

firestack
Copy link
Member

@firestack firestack commented Sep 25, 2024 •
edited
Loading

Step 2 of adding detour notifications to Skate. This creates the notification API for having the notification server create a new notification for a detour.

Asana Ticket: https://app.asana.com/0/0/1208393164852685/f

Depends on:

@firestack firestack force-pushed the kf/asn/dn/notification-server branch 2 times, most recently from 302ad0d to 102eb5f Compare September 25, 2024 15:32
@firestack firestack force-pushed the kf/asn/dn/notification branch from d79ea2c to c231cfb Compare September 25, 2024 15:34
@firestack firestack force-pushed the kf/asn/dn/notification-server branch from 102eb5f to d632b49 Compare September 25, 2024 15:34
@github-actions GitHub Actions
Copy link

Coverage of commit d632b49

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@github-actions GitHub Actions
Copy link

Coverage of commit d632b49

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@github-actions GitHub Actions
Copy link

Coverage of commit d632b49

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification-server branch 2 times, most recently from b323e24 to 956c608 Compare September 25, 2024 15:46
@github-actions GitHub Actions
Copy link

Coverage of commit b323e24

Summary coverage rate:
  lines......: 92.4% (3343 of 3619 lines)
  functions..: 71.7% (1390 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@firestack firestack marked this pull request as ready for review September 25, 2024 15:51
@firestack firestack requested a review from a team as a code owner September 25, 2024 15:51
@github-actions GitHub Actions
Copy link

Coverage of commit 956c608

Summary coverage rate:
  lines......: 92.4% (3343 of 3619 lines)
  functions..: 71.7% (1390 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification branch 2 times, most recently from 5653aab to bc3b076 Compare September 25, 2024 19:07
@firestack firestack force-pushed the kf/asn/dn/notification-server branch from 956c608 to 39d989c Compare September 26, 2024 12:29
@firestack firestack force-pushed the kf/asn/dn/notification branch from bc3b076 to 1d7f8d7 Compare September 26, 2024 12:29
@github-actions GitHub Actions
Copy link

Coverage of commit 39d989c

Summary coverage rate:
  lines......: 92.4% (3343 of 3619 lines)
  functions..: 71.7% (1390 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@github-actions GitHub Actions
Copy link

Coverage of commit 39d989c

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification-server branch from 39d989c to 313828a Compare September 26, 2024 12:45
@github-actions GitHub Actions
Copy link

Coverage of commit 313828a

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@firestack firestack mentioned this pull request Sep 26, 2024
Merged
@firestack firestack force-pushed the kf/asn/dn/notification branch from a9659a2 to 91773c9 Compare September 26, 2024 13:39
@firestack firestack force-pushed the kf/asn/dn/notification-server branch from 313828a to 64d4d59 Compare September 26, 2024 13:40
@github-actions GitHub Actions
Copy link

Coverage of commit 64d4d59

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@github-actions GitHub Actions
Copy link

Coverage of commit 64d4d59

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@firestack firestack changed the title feat: add detour notifications to notifications server feat: add detour notifications to notifications server Sep 26, 2024
@firestack firestack force-pushed the kf/asn/dn/notification branch from 91773c9 to d91843c Compare September 26, 2024 14:39
@firestack firestack force-pushed the kf/asn/dn/notification-server branch from 64d4d59 to 1454143 Compare September 26, 2024 14:39
@github-actions GitHub Actions
Copy link

Coverage of commit 1454143

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification-server branch from 1454143 to a8fb678 Compare September 26, 2024 15:02
@firestack firestack force-pushed the kf/asn/dn/notification branch from d91843c to e31c806 Compare September 26, 2024 15:02
@github-actions GitHub Actions
Copy link

Coverage of commit a8fb678

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@github-actions GitHub Actions
Copy link

Coverage of commit a8fb678

Summary coverage rate:
  lines......: 92.4% (3343 of 3619 lines)
  functions..: 71.7% (1390 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification-server branch from a8fb678 to dfde647 Compare September 26, 2024 15:23
@firestack firestack force-pushed the kf/asn/dn/notification branch from e31c806 to b3bd45d Compare September 26, 2024 15:23
@github-actions GitHub Actions
Copy link

Coverage of commit dfde647

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@github-actions GitHub Actions
Copy link

Coverage of commit dfde647

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification branch from 1884c6d to c26872e Compare September 26, 2024 15:58
@firestack firestack force-pushed the kf/asn/dn/notification-server branch from dfde647 to 12fa629 Compare September 26, 2024 15:58
@firestack firestack force-pushed the kf/asn/dn/notification branch from 1e956b0 to b043e49 Compare September 26, 2024 16:01
@firestack firestack force-pushed the kf/asn/dn/notification-server branch from 12fa629 to 2c20a1f Compare September 26, 2024 16:01
@github-actions GitHub Actions
Copy link

Coverage of commit 2c20a1f

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@github-actions GitHub Actions
Copy link

Coverage of commit 2c20a1f

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@github-actions GitHub Actions
Copy link

Coverage of commit 2c20a1f

Summary coverage rate:
  lines......: 92.4% (3343 of 3619 lines)
  functions..: 71.7% (1390 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

Base automatically changed from kf/asn/dn/notification to main September 26, 2024 16:08
@firestack firestack force-pushed the kf/asn/dn/notification-server branch 2 times, most recently from ead40ab to 1c1200b Compare September 26, 2024 16:11
@github-actions GitHub Actions
Copy link

Coverage of commit 1c1200b

Summary coverage rate:
  lines......: 92.4% (3344 of 3619 lines)
  functions..: 71.8% (1391 of 1938 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/notification_server.ex                        |92.3%     78|82.6%    23|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification-server branch from 1c1200b to 25bf191 Compare September 26, 2024 16:29
hannahpurcell
hannahpurcell approved these changes Sep 27, 2024
View reviewed changes
Copy link
Collaborator

@hannahpurcell hannahpurcell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm less proficient in reviewing GenServer work, but it helped that the notification_server already existed for other notification types, so I could follow the set pattern for detour notifs. So I only have a couple questions, but none of them block approval

lib/notifications/notification_server.ex Outdated Show resolved Hide resolved
lib/notifications/notification_server.ex Show resolved Hide resolved
test/notifications/notification_server_test.exs Outdated Show resolved Hide resolved
@firestack firestack force-pushed the kf/asn/dn/notification-server branch 2 times, most recently from 9939117 to 76a38d8 Compare September 27, 2024 12:11
@firestack firestack force-pushed the kf/asn/dn/notification-server branch from 76a38d8 to f692394 Compare September 27, 2024 12:13
@firestack firestack enabled auto-merge (squash) September 27, 2024 12:34
@firestack firestack merged commit 7badb90 into main Sep 27, 2024
9 checks passed
@firestack firestack deleted the kf/asn/dn/notification-server branch September 27, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hannahpurcell hannahpurcell hannahpurcell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@firestack @hannahpurcell