kart7990 / virtualpitwall

https://staging.virtualpitwall.com
Other
17 stars 7 forks source link

v2 Google Login not working #9

Closed elft3r closed 9 months ago

elft3r commented 9 months ago

Hey @kart7990, while testing the new v2 version with the Google Login, I ran into a 500 - Internal Server Error, when calling https://virtualpitwall-test.azurewebsites.net/api/v2.0/authentication/loginexternal. Are there still some pieces missing, for the new login mechanism to work?

Also, I needed to remove the <RequireAuth> block to test locally. In v1, I'm able to see the dashboard without being logged in. Do you want to change that with v2? https://github.com/kart7990/virtualpitwall/blob/ebf0a3e8c65f6eadccfd6423a3efe1c3c6211114/src/app/pitwall/layout.tsx#L11-L17

Best, Jochen

kart7990 commented 9 months ago

Hey Jochen, Yeah removing RequireAuth is the way to go for now. Yes, I had it public in v1, but I plan to make it required to have an account for v2. A few reasons for that such as dashboard layout and preference saving, but also with v1 I noticed a few times during the beta period we had 20+ active viewers on a single dashboard. While that's great, I think we might need to enforce some limits (or tiered approach) due to the amount of data pumping through this. No idea what that will look like, but forcing auth lays the foundation for it.

The add-session-conditions branch has grown a bit in scope 😅 - a bit hard to keep things isolated while I'm working so hard to get all the foundation in place to build on.

Thanks for the addition of the formatting on the conditions, that was somewhere on my todos, so a big help. Need to wire it up to preference redux state, which exists but doesn't have a functioning UI to select it yet.

-David

elft3r commented 9 months ago

Thanks, David, for clarifying the points and giving me more insights.

I did some more tests and the login worked yesterday. However, today, something is not working with the staging deployment. I receive 500 errors when calling the API. Could you have a look and let me know what the problem is?

kart7990 commented 9 months ago

Working on it, lots of stuff moving around right now. Hope to have things stable soon. I'm making the v2 branch the main branch and moving the old project to legacy/main. Additionally I'm setting up ci/cd so anything which gets merged to main is built and deployed to staging automatically. I'm having issues with iis virtual applications and next/node intercepting the /api requests. I'm going to deploy the api under a separate webapp/domain to avoid this headache.

i.e. { "APP_ENV": "staging", "BASE_URL": "https://staging.virtualpitwall.com", "BASE_API_URL": "https://staging.api.virtualpitwall.com" }

kart7990 commented 9 months ago

This means you'll probably want to delete everything and clone fresh (or reset hard to latest commit in main). I of course won't be doing stuff like this once everything gets settled, still just trying to get the project setup and organized in a way that makes sense. 😄

kart7990 commented 9 months ago

I think I've got it back in a good state. I have updated the readme with a bit more details. Let me know if you're still seeing issues. Right now all I know is broken is the client app download, which will be an easy fix.

elft3r commented 9 months ago

Thanks, I did some more tests and now everything looks good again. I can log in and test the small changes I worked on. PR is coming soon 😊

Yeah, getting everything stitched together and in a way for others to consume, always requires some work and multiple iterations, with breaking changes.

Thank you also for the updated README, this helps me to get a better understanding of the data model. I have now a solid understanding of the flow and model and will take on some of the components. Let me know, if there is one you want me to focus on.

Best, Jochen

kart7990 commented 9 months ago

Great! Yeah the goal now is basically to reach parity with the legacy version, plus team support.

My thoughts for team support: Allow connections from multiple team members using the windows app to the same Pitwall session. This will allow driver swaps without having to switch to a different dashboard url. Additionally, if a team member connects who isn't a driver (spectator mode), they will be added as a "timing provider" which can be selected from a drop down of all connected users. This will allow spectators to enable max car visibility (since they won't be worried about frames) to allow for all cars to be used in Pitwall analysis.

That being said, I'd like to focus on the team support changes as it has a large amount of backend and windows app work.

I was working hard to get the foundation in place to be able to add the other features from legacy, and I think it's mostly in a decent spot to achieve that now. If you want to focus on any of those pieces that would be great, I don't current have any of that in progress. We're still missing a bit on the styling side, I don't think anything looks super great yet, but I'm more worried about functionality than looking pretty at the moment.

Let's use discord for syncing from now on if that works for you. I'm going to copy this over there.

elft3r commented 9 months ago

Cool, team support sounds great.

Discord works for me 👍