-
Notifications
You must be signed in to change notification settings - Fork 9
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
Conversation
1464bac
to
7e1db17
Compare
Coverage of commit
|
7e1db17
to
2986304
Compare
Coverage of commit
|
2986304
to
d79ea2c
Compare
|> select_bridge_movements() | ||
|> select_block_waivers() | ||
|> select_detour_info() | ||
|> where([notification: n], n.created_at > ^cutoff_time) |
There was a problem hiding this comment.
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..
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
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. |
d79ea2c
to
c231cfb
Compare
Coverage of commit
|
Coverage of commit
|
0a5c2ae
to
2a47520
Compare
Coverage of commit
|
5653aab
to
bc3b076
Compare
Coverage of commit
|
bc3b076
to
1d7f8d7
Compare
Coverage of commit
|
1d7f8d7
to
a9659a2
Compare
Coverage of commit
|
a9659a2
to
91773c9
Compare
Coverage of commit
|
91773c9
to
d91843c
Compare
Coverage of commit
|
d91843c
to
e31c806
Compare
Coverage of commit
|
e31c806
to
b3bd45d
Compare
Coverage of commit
|
1884c6d
to
c26872e
Compare
There was a problem hiding this 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!
Coverage of commit
|
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]>
1e956b0
to
b043e49
Compare
Coverage of commit
|
Coverage of commit
|
Coverage of commit
|
Coverage of commit
|
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