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

User data Firestore collections + functions #9

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

tara-adus
Copy link
Collaborator

No description provided.

Copy link
Contributor

@jdlane jdlane left a comment

Choose a reason for hiding this comment

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

Hey guys, great work with these functions. Take a look at my comment about the firestore rules, they seem to have some issues compiling. The other changes I requested are mostly best practices and code quality, so those should be pretty straightforward. Don't hesitate to ping me with any questions!

Copy link
Contributor

Choose a reason for hiding this comment

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

These types should go in their own types file so that we can keep our TypeScript types together and separate from the firestore logic.

@@ -15,5 +15,20 @@ service cloud.firestore {
match /{document=**} {
allow read, write: if request.time < timestamp.date(2024, 10, 25);
}

match /applicantUsers/{userId} {
allow write: request.resource.data.name != null &&
Copy link
Contributor

Choose a reason for hiding this comment

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

These rules aren't compiling when I run them using the emulator. The applicantUsers rule is definitely missing an "if" but I'm also getting an error on "size". Let me know if you guys need any help testing with the emulator

import { getAnalytics } from "firebase/analytics";
//If this line says import not found then you need to set up firebase api on your machine
import firebaseConfig from "./firebase_config/FireConfig";


const cong = initializeApp(firebaseConfig);
const firestore = getFirestore(cong);
Copy link
Contributor

Choose a reason for hiding this comment

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

This firestore variable is called "db" on the main branch, so it might be easier to switch that name now before dealing with the merge conflicts.

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.

3 participants