-
Notifications
You must be signed in to change notification settings - Fork 0
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
Reminder #48
Conversation
@lilith i've gotten the could you set these as github secrets and add them to lmk if you'd like me to msg you what vals i used~ |
For the intergration tests, how do we ensure that the api is disabled? |
I'm a bit confused. are you asking how we know whether the api is disabled for non-authenticated queries to our endpoints? |
Sorry. I mean when we run yarn test, will any SMSes be sent? The keys I put
into github should likely be sandbox mode or something
…On Sun, Aug 6, 2023, 11:33 AM Jessica Ho ***@***.***> wrote:
For the intergration tests, how do we ensure that the api is disabled?
I'm a bit confused. are you asking how we know whether the api is disabled
for non-authenticated queries to our endpoints?
—
Reply to this email directly, view it on GitHub
<#48 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA2LH2PFUVF2JWXQQLZNP3XT7IPZANCNFSM6AAAAAA3FYNZEE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Ah I see. Yeah as long as you use the test keys, we should be fine. I never got a login link SMS to my personal number while i was testing |
Okay, message them to me.
…On Sun, Aug 6, 2023, 2:49 PM Jessica Ho ***@***.***> wrote:
Sorry. I mean when we run yarn test, will any SMSes be sent? The keys I
put into github should likely be sandbox mode or something
Ah I see. Yeah as long as you use the test keys, we should be fine. I
never got a login link SMS to my personal number while i was testing
—
Reply to this email directly, view it on GitHub
<#48 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA2LH5UXEJWBHKPGHRCDITXT77MLANCNFSM6AAAAAA3FYNZEE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Github Actions is now working |
If it expires it still populates the phone number field, right?
…On Mon, Aug 7, 2023, 12:49 AM Jessica Ho ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/lib/server/twilio.ts
<#48 (comment)>:
> @@ -94,12 +94,24 @@ const msgToSend = async (
msg = `Thanks for subscribing to reminders and friend availability notifications from ${url}! You can disable this at any time on your Profile page or by responding STOP.`;
break;
}
+ case 'reminder': {
+ const { phone } = msgComps;
+ msg = `Hi! It's your periodic reminder to update your schedule: https://playdate.help/login/${phone}` <https://playdate.help/login/$%7Bphone%7D>;
mmm yeah we can do that. the only downside is the session could expire
between when the reminder is sent and when the user finally clicks on the
link but i think that's fine
—
Reply to this email directly, view it on GitHub
<#48 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA2LH5H2KVOLEJ5BOZXHQDXUCFWHANCNFSM6AAAAAA3FYNZEE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
We should add a test for how an expired link behaves. I am getting a 500
error rn
…On Mon, Aug 7, 2023, 12:51 AM Lilith River ***@***.***> wrote:
If it expires it still populates the phone number field, right?
On Mon, Aug 7, 2023, 12:49 AM Jessica Ho ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/lib/server/twilio.ts
> <#48 (comment)>:
>
> > @@ -94,12 +94,24 @@ const msgToSend = async (
> msg = `Thanks for subscribing to reminders and friend availability notifications from ${url}! You can disable this at any time on your Profile page or by responding STOP.`;
> break;
> }
> + case 'reminder': {
> + const { phone } = msgComps;
> + msg = `Hi! It's your periodic reminder to update your schedule: https://playdate.help/login/${phone}` <https://playdate.help/login/$%7Bphone%7D>;
>
> mmm yeah we can do that. the only downside is the session could expire
> between when the reminder is sent and when the user finally clicks on the
> link but i think that's fine
>
> —
> Reply to this email directly, view it on GitHub
> <#48 (comment)>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAA2LH5H2KVOLEJ5BOZXHQDXUCFWHANCNFSM6AAAAAA3FYNZEE>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
At the moment, I don't think so. it just redirects you back to the homepage |
I'll try it now 👀 |
Maybe if the link doesn't exist at all then you get a 500?
Either way, if there’s a phone number in the url we should always prefill
the text box
…On Mon, Aug 7, 2023, 12:55 AM Jessica Ho ***@***.***> wrote:
We should add a test for how an expired link behaves. I am getting a 500
I'll try it now 👀
—
Reply to this email directly, view it on GitHub
<#48 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA2LH5X4XSE56AVLELPNBLXUCGMTANCNFSM6AAAAAA3FYNZEE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
GitGuardian id | Secret | Commit | Filename | |
---|---|---|---|---|
7593429 | Generic High Entropy Secret | b4eef1d | prisma/seed.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Our GitHub checks need improvements? Share your feedbacks!
this is just the token for testing an expired magic link |
@@ -263,13 +265,19 @@ export const getMsg = async (url: URL) => { | |||
return response; | |||
}; | |||
|
|||
function shuffleArr(arr: any[]) { | |||
for (let i = arr.length - 1; i > 0; i--) { | |||
const j = Math.floor(Math.random() * (i + 1)); |
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.
If Math.random() ever returns exactly 1 (rare but possible), this will fail with out of bounds if it happens on the first iteration
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.
Nevermind, if this is the browser-compliant Math.random() it excludes one. Carry on
This causes schedule slippage if the message isn't sent on the second every time. Rather than using 'now', use the reminderDateTime from the db and change the date. Make sure all time zone logic is consistent and stored times are in UTC (otherwise doing math across DST will cause a mess).
|
I'm merging so I can do more UX testing, feel free to push the reminder date fix tho |
moved
sendNotif
tolib/server
and start cron job inhooks.server.ts
added
PUBLIC_URL
to GHA