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

Group Availability Tab #97

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

Group Availability Tab #97

wants to merge 25 commits into from

Conversation

MinhxNguyen7
Copy link
Member

@MinhxNguyen7 MinhxNguyen7 commented May 13, 2024

Description

  • Add functionality to view other members' availabilities in the group availability tab.
    • Queries the DB, loads the data via SvelteKit's load, and updates the groupAvailabilities store.
  • Also update drizzle-kit because drizzle-studio only works with the latest version.

TODOs

  • Minimal code to create a meeting. (done in Meeting Creation Functionality #102)
  • Query relevant data from the DB, format it, then update the store(s).
  • Update the groupAvailabilities store when the user changes their availability.
  • Test that everything works.
    • Write code to create a meeting with multiple members. This is not strictly necessary since we can do this manually while testing.
  • Remove sample data generation code.

Test Plan

  • Create a meeting. The personal and group availability stores should be empty.
  • Add personal availability and save it. The group availability should reflect without refreshing the page.
  • Refreshing the page should load in the most up-to-date group availability.
  • Log out of the current user and sign in as another user. Updating personal availability in the same meeting should show the overlapping availability in the groups tab.

Issues

@MinhxNguyen7 MinhxNguyen7 added the enhancement New feature or request label May 13, 2024
@MinhxNguyen7 MinhxNguyen7 self-assigned this May 13, 2024
@MinhxNguyen7
Copy link
Member Author

There's a bug where the groupAvailability doesn't render the availabilities loaded into the store because it's server-side rendered, and the store is local. We can solve this by moving the state of the store into the component itself, then prop-drill the data through the load into the components.

@seancfong
Copy link
Member

@MinhxNguyen7 Group availability loads correctly now, even with multiple users. Also did some clean up by moving the personal availability parsing up to +page.svelte

However, editing personal availability doesn't update the group availability store unless the page is reloaded. This would have to be accounted for in the future.

image

@seancfong seancfong requested a review from KevinWu098 June 3, 2024 19:06
@seancfong seancfong marked this pull request as ready for review June 3, 2024 19:07
@seancfong
Copy link
Member

seancfong commented Jun 7, 2024

@KevinWu098 Should be ready to review again. Currently, group availability works only for registered users. We can create a new issue for guests and you can look into that.

Here's a sample meeting on staging that shows it working.

https://staging-97.zotmeet.com/availability/7f8b2794-5605-44cb-84bd-f04fc098c705

@KevinWu098
Copy link
Member

Looks good on deployment -- will look over code asap

Copy link
Member

@KevinWu098 KevinWu098 left a comment

Choose a reason for hiding this comment

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

LGTM -- just some minor nits

Comment on lines 26 to 27
// @ts-expect-error slug is defined in the route
const meeting_id: string = params?.slug;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// @ts-expect-error slug is defined in the route
const meeting_id: string = params?.slug;
const meeting_id: string = params?.slug ?? "";

chore: unused ts directive -- slug is read as any. It might be better to use the nullish coalescing operator ?? to force it into a string

* @param meetingId
* @returns a record of the member name to their availabilities, each sorted by date
*/
async function getMeetingMemeberAvailabilities(meetingId: string) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
async function getMeetingMemeberAvailabilities(meetingId: string) {
async function getMeetingMemberAvailabilities(meetingId: string) {

typo: Memeber instead of Member; also change line 34 where this function is called

src/routes/api/create-meeting/+server.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Group availability page functionality
3 participants