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

Set up firebase for coffee chat bingo #593

Merged
merged 9 commits into from
Apr 15, 2024

Conversation

alyssayzhang
Copy link
Contributor

@alyssayzhang alyssayzhang commented Mar 24, 2024

Summary

This pull request is the first step towards implementing coffee chat bingo.

  • created a new collection called coffee-chats in firebase.ts
  • defined a DBCoffeeChat type

Notion/Figma Link

https://www.notion.so/Set-up-Firebase-for-coffee-chat-bingo-3d6c635217ec48a8aaf08ed7a5e4f197

Test Plan

Notes

@dti-github-bot
Copy link
Member

dti-github-bot commented Mar 24, 2024

[diff-counting] Significant lines: 32.

Copy link
Contributor

@patriciaahuang patriciaahuang left a comment

Choose a reason for hiding this comment

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

The set up looks good in the prod database! You probably did some testing in the dev database, so there's an extra coffee-chat-requirement collection. In order to keep the dev database updated with the prod, you may want to remove it.

@alyssayzhang alyssayzhang marked this pull request as ready for review March 25, 2024 22:21
@alyssayzhang alyssayzhang requested a review from a team as a code owner March 25, 2024 22:21
Comment on lines 101 to 107
export type DBCoffeeChat = {
uuid: string;
members: firestore.DocumentReference[];
image: string;
category: string;
description: string;
status: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure to add this new type to common-types/index.d.ts as well!

Copy link
Collaborator

@andrew032011 andrew032011 left a comment

Choose a reason for hiding this comment

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

A few nits, but overall looks good

image: string;
category: string;
description: string;
status: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change status type to be Status, not string.

@@ -191,3 +191,13 @@ interface Shoutout {
readonly hidden: boolean;
readonly uuid: string;
}

interface CoffeeChat {
readonly uuid: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

As you can see (from above), though we never followed this rule, we theoretically should be making all these fields readonly. Don't worry about the above ones but let's fix this one.

Copy link
Contributor

@oscarwang20 oscarwang20 left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks again for finishing this :D

@alyssayzhang alyssayzhang merged commit 589d23b into main Apr 15, 2024
17 checks passed
@alyssayzhang alyssayzhang deleted the set-up-firebase-for-coffee-chat-bingo branch April 15, 2024 23:22
oscarwang20 pushed a commit that referenced this pull request May 6, 2024
* initial commit

* add dbcoffeechat type

* testing file

* testing

* rm testing file

* add type into common types

* make description non optional

* add date

* fix nits
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.

5 participants