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

Issue 212 frontend query sql #221

Merged
merged 20 commits into from
Sep 21, 2024
Merged

Issue 212 frontend query sql #221

merged 20 commits into from
Sep 21, 2024

Conversation

rphovley
Copy link
Member

@rphovley rphovley commented Sep 19, 2024

The Pull Request is ready

Changes

  • Added backend utilities for reading and writing api key to file. Included rules for only allowing reading of the key when initialized or by "admin" (used for CLI command)
  • Added backend endpoints for authorization and a the query endpoint that is protected by the api key access. This endpoint will take in raw sql and execute it directly against the DB. Allowing the browser application to retain most of the logic which will make for a better separation of concerns once the desktop app is in use
  • Was going to use cookies, but decided to use localStorage instead as the domains will not match
  • Added auth checks on the frontend for initializing the api key, validating it's correctness, and importing a new one if you are opening the application in a new browser
  • Added a page for when there are required updates. Full screen since the update would break the application if it's not up to date with latest
  • CORS updates and minor documentation updates for keeping breaking changes to a minimum

The code follows best practices

  • duplicate code has been extracted where possible
  • issues for follow-up tasks have been created
  • tests have been written for any new functionality
  • there is no any type used
  • texts have been checked for grammar and spelling issues

Notes

Copy link
Contributor

github-actions bot commented Sep 19, 2024

Visit the preview URL for this PR (updated for commit 00ad14a):

https://codeclimbersio--pr221-issue-212-frontend-q-3pfssudm.web.app

(expires Sat, 28 Sep 2024 00:19:21 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 3c28ab1d661a7526dc2530b2448a1aa4f88d23d4

@rphovley rphovley force-pushed the issue-212-frontend-query-sql branch from 0bdbe9f to 24444e5 Compare September 20, 2024 22:27
@rphovley rphovley merged commit 8b79376 into main Sep 21, 2024
8 of 9 checks passed
@rphovley rphovley deleted the issue-212-frontend-query-sql branch September 21, 2024 00:19
Copy link
Member

@Tanner-Scadden Tanner-Scadden left a comment

Choose a reason for hiding this comment

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

Your code is always so clean. Looks really good man 🔥 .

Honestly I use to hate classes after everywhere I've worked using them poorly, but you're changing my mind!

target?: string
}

const CodeClimbersLoadingButton = ({
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it just be better to have a generic button component that takes in the eventName? LoadingButton to me mean that you'd be displaying a spinner and expect a isLoading | isPending prop


function DashboardLayout({ children }: DashboardLayoutProps) {
const { isMajorUpdate, isMinorUpdate } = useUpdateVersionHook()
if (isMajorUpdate || isMinorUpdate) {
Copy link
Member

Choose a reason for hiding this comment

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

Smart!

@@ -28,7 +29,20 @@ export async function bootstrap() {
})
traceEnvironment()

app.enableCors()
app.enableCors({
Copy link
Member

Choose a reason for hiding this comment

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

Very nice

credentials: 'include',
})
return useBetterQuery<T[], Error>({
queryKey: ['localdb/query'],
Copy link
Member

Choose a reason for hiding this comment

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

You'll want to make this key unique per query string passed in. As is it'll reuse the same key for any query string passed in

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.

Prototype for pushing business logic for local data into browser
2 participants