Achen 237 google calendar permission request#262
Conversation
…gcal perms are given
aizilerts
had a problem deploying
to
staging-262
GitHub Actions
Failure
alexanderl19
force-pushed
the
achen-237-google-calendar-permission-request
branch
from
a678386 to
f045efc
Compare
alexanderl19
had a problem deploying
to
staging-262
GitHub Actions
Failure
alexanderl19
force-pushed
the
achen-237-google-calendar-permission-request
branch
from
ad78590 to
f045efc
Compare
…ndar-permission-request
alexanderl19
temporarily deployed
to
staging-262
GitHub Actions
Inactive
|
Where are we with this? |
…7-google-calendar-permission-request
aizilerts
temporarily deployed
to
staging-262
GitHub Actions
Inactive
KevinWu098
temporarily deployed
to
staging-262
GitHub Actions
Inactive
KevinWu098
left a comment
KevinWu098
left a comment
There was a problem hiding this comment.
Definitely works on staging! I have a few comments though about comments (no pun intended) in this PR
| cookieStore.delete("session"); | ||
| cookieStore.delete({ name: "session", path: "/" }); |
There was a problem hiding this comment.
issue: why are we deleting the same cookies twice?
There was a problem hiding this comment.
Originally I had both of these just to make sure the cookies were being deleted, as I was having issues with that. I looked at the session token cookie function again and I believe we only need the second one, so I deleted the first
| cookieStore.delete("oauth_state"); | ||
| cookieStore.delete("oauth_code_verifier"); | ||
| cookieStore.delete("auth_redirect_url"); |
There was a problem hiding this comment.
question: why are we only deleting these inside of this code path, instead of always?
There was a problem hiding this comment.
You're right, the cookies should also be deleted for the new user branch. I added the same deletions into that branch
| const [googleCalendarEvents, setGoogleCalendarEvents] = useState< | ||
| GoogleCalendarEvent[] | ||
| >([]); | ||
| // const [hasFetchedCalendar, setHasFetchedCalendar] = useState(false); |
| overlayGoogleCalendar, | ||
| availabilityDates, | ||
| anchorNormalizedDate, | ||
| // hasFetchedCalendar, |
| return; | ||
| } | ||
| setGoogleCalendarEvents(result.events); | ||
| // setHasFetchedCalendar(true); |
There was a problem hiding this comment.
question: what did/does this do?
There was a problem hiding this comment.
This loads the google calendar events after the user has been authenticated/given permissions
| fromTime={fromTimeMinutes} | ||
| members={members} | ||
| timezone={userTimezone} | ||
| // timezone={userTimezone} |
There was a problem hiding this comment.
question: why do we not care about timezone anymore?
There was a problem hiding this comment.
Pretty sure I commented this out back then to keep congruent with the GroupResponses type, but this is outdated, and I've uncommented it now
| doesntNeedDay, | ||
| }: GroupResponsesProps) { | ||
| const [_newBlocks, newAvailDates] = newZonedPageAvailAndDates( | ||
| const [_newBlocks, _newAvailDates] = newZonedPageAvailAndDates( |
There was a problem hiding this comment.
issue: the underscore implies we're not going to use this value, but we clearly do. can we elaborate on what's going on here
There was a problem hiding this comment.
I'm not sure exactly why I used an underscore here either (i think i had been going insane) but I've removed it now
| @@ -4,48 +4,48 @@ import { NextResponse } from "next/server"; | |||
| export async function proxy(request: NextRequest): Promise<NextResponse> { | |||
There was a problem hiding this comment.
issue: why have we uncommented all of proxy.ts (fka middleware.ts)?
There was a problem hiding this comment.
Originally I commented it because it was interfering with authentication and this was a quick fix. I've put in an if statement that does the same thing and uncommented the rest of proxy.ts
|
How is progress here? |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/app/auth/login/google/callback/route.tsx">
<violation number="1" location="src/app/auth/login/google/callback/route.tsx:25">
P1: Remove sensitive OAuth/cookie debug logs; they expose authentication material in server logs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…7-google-calendar-permission-request
3 participants
** This is a small fix for my previous pr!
On deployment, using Nextjs router was causing an error, so a person in the case 2 userflow (see other pr) would be stuck in a loop trying to access gcal events.