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.

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

feat: add detour notifications to notifications module #2813

Merged
merged 4 commits into from
Sep 26, 2024

Conversation

firestack
Copy link
Member

@firestack firestack commented Sep 25, 2024

Step 1 of adding detour notifications to Skate. To be able to send detour notifications, we first need to create the functions and setup for storing and retrieving this info from the database.

This will be used by notification_server in future PR's.

The entire feature can be seen in context on the branch main...kf/asn/detour-notifications

@firestack firestack force-pushed the kf/asn/dn/notification branch 2 times, most recently from 1464bac to 7e1db17 Compare September 25, 2024 13:20
Copy link

Coverage of commit 7e1db17

Summary coverage rate:
  lines......: 92.5% (3334 of 3606 lines)
  functions..: 71.8% (1388 of 1934 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification branch from 7e1db17 to 2986304 Compare September 25, 2024 13:45
Copy link

Coverage of commit 2986304

Summary coverage rate:
  lines......: 92.5% (3334 of 3606 lines)
  functions..: 71.8% (1388 of 1934 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0

Download coverage report

@firestack firestack marked this pull request as ready for review September 25, 2024 14:11
@firestack firestack requested a review from a team as a code owner September 25, 2024 14:11
@firestack firestack force-pushed the kf/asn/dn/notification branch from 2986304 to d79ea2c Compare September 25, 2024 14:46
|> select_bridge_movements()
|> select_block_waivers()
|> select_detour_info()
|> where([notification: n], n.created_at > ^cutoff_time)
Copy link
Member Author

Choose a reason for hiding this comment

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

hmm, gonna need to figure out a better way to do this now... since detours can be active for longer than the cutoff time..

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we even need/want to limit detour lifespan? I figured that a detour would just be active until it's deactivated.

Copy link
Member Author

Choose a reason for hiding this comment

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

You're correct, but block waivers and bridge movements don't agree and do require this, so I need a way to only apply this for non detour notifications

@firestack firestack marked this pull request as draft September 25, 2024 14:56
@firestack
Copy link
Member Author

firestack commented Sep 25, 2024

I tried to add a test for logging notification creation, but was unable to, I created a followup ticket to make sure this doesn't get missed, so that we're in parity with other notifications.

@firestack firestack marked this pull request as ready for review September 25, 2024 15:12
@firestack firestack force-pushed the kf/asn/dn/notification branch from d79ea2c to c231cfb Compare September 25, 2024 15:34
Copy link

Coverage of commit c231cfb

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

Copy link

Coverage of commit 0a5c2ae

Summary coverage rate:
  lines......: 92.4% (3334 of 3608 lines)
  functions..: 71.7% (1388 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification branch from 0a5c2ae to 2a47520 Compare September 25, 2024 18:28
Copy link

Coverage of commit 2a47520

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification branch from 5653aab to bc3b076 Compare September 25, 2024 19:07
Copy link

Coverage of commit bc3b076

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification branch from bc3b076 to 1d7f8d7 Compare September 26, 2024 12:29
Copy link

Coverage of commit 1d7f8d7

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

Copy link

Coverage of commit a9659a2

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification branch from a9659a2 to 91773c9 Compare September 26, 2024 13:39
Copy link

Coverage of commit 91773c9

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification branch from 91773c9 to d91843c Compare September 26, 2024 14:39
Copy link

Coverage of commit d91843c

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification branch from d91843c to e31c806 Compare September 26, 2024 15:02
Copy link

Coverage of commit e31c806

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification branch from e31c806 to b3bd45d Compare September 26, 2024 15:23
Copy link

Coverage of commit b3bd45d

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

@firestack firestack force-pushed the kf/asn/dn/notification branch from 1884c6d to c26872e Compare September 26, 2024 15:58
Copy link
Contributor

@joshlarson joshlarson left a comment

Choose a reason for hiding this comment

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

Made a couple of nits, and I think this looks good!

Copy link

Coverage of commit c26872e

Summary coverage rate:
  lines......: 92.4% (3334 of 3608 lines)
  functions..: 71.7% (1388 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

feat(ex/notifications/detour): add virtual fields to detour notification schema
feat(ex/notifications): query detour info from notifications

Co-authored-by: Josh Larson <[email protected]>
@firestack firestack force-pushed the kf/asn/dn/notification branch from 1e956b0 to b043e49 Compare September 26, 2024 16:01
Copy link

Coverage of commit b043e49

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

Copy link

Coverage of commit b043e49

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

Copy link

Coverage of commit b043e49

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

Copy link

Coverage of commit b043e49

Summary coverage rate:
  lines......: 92.4% (3335 of 3608 lines)
  functions..: 71.8% (1389 of 1935 functions)
  branches...: no data found

Files changed coverage rate:
                                                                  |Lines       |Functions  |Branches    
  Filename                                                        |Rate     Num|Rate    Num|Rate     Num
  ======================================================================================================
  lib/notifications/db/detour.ex                                  |66.7%      6|70.0%    10|    -      0
  lib/notifications/db/notification.ex                            |70.6%     17|68.4%    19|    -      0
  lib/notifications/detour.ex                                     | 100%      2| 100%     2|    -      0
  lib/notifications/notification.ex                               | 100%     64|94.1%    17|    -      0
  test/support/factory.ex                                         |96.8%     63|66.7%    63|    -      0

Download coverage report

@firestack firestack merged commit daf6795 into main Sep 26, 2024
9 checks passed
@firestack firestack deleted the kf/asn/dn/notification branch September 26, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants