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.

Sign up for GitHub

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

Jump to bottom

Group Availability Tab #97

Closed
wants to merge 25 commits into from
Closed

Group Availability Tab #97

wants to merge 25 commits into from

Conversation

MinhxNguyen7
Copy link
Member

@MinhxNguyen7 MinhxNguyen7 commented May 13, 2024 •
edited by seancfong
Loading

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 •
edited
Loading

@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

KevinWu098
KevinWu098 requested changes Jun 7, 2024
View reviewed changes
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

src/routes/availability/[slug]/+page.server.ts Outdated
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

src/routes/availability/[slug]/+page.server.ts Outdated
* @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
@seancfong seancfong requested a review from KevinWu098 June 7, 2024 21:49
@seancfong seancfong closed this Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KevinWu098 KevinWu098 Awaiting requested review from KevinWu098

Assignees

@seancfong seancfong

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Group availability page functionality
3 participants
@MinhxNguyen7 @seancfong @KevinWu098