Skip to content

308 teams page teams page front end implementtion#331

Open
kbventures wants to merge 96 commits into
devfrom
308-teams-page-teams-page-front-end-implementtion
Open

308 teams page teams page front end implementtion#331
kbventures wants to merge 96 commits into
devfrom
308-teams-page-teams-page-front-end-implementtion

Conversation

@kbventures

Copy link
Copy Markdown
Contributor

Summary

Teams Page, Team Page, Profile Page(not current user), Sign Out Button and Gravatar Input/Edit

#308

bcha92 and others added 18 commits June 23, 2026 02:03
- Create src/types/index.ts with shared AccessLevel, ProfileStatus, MemberProfile, TeamMemberProfile
- Extract gravatar helpers into src/utils/gravatar.ts shared util
- Simplify safeAvatarUrl to only allow https/http (no local path exception)
- Add Gravatar URL validation to GeneralInfoFormModal on avatar save
- Restore package.json and package-lock.json to match dev branch

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Follows the profiles/profile pattern — teams list moves to src/pages/teams/
and src/pages/team/ becomes detail-only (shows "Team not found." without ?id=).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… 298-dependency-update-only-update-minor-changes-in-dependencies-and-dev-dependencies-group-no-breaking-changes
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…name array

- add authenticated profile/member list routes (with email) for protected pages
- restore secureHeaders at app level (was removed, test still asserted them)
- return member teamNames as JSON array instead of ||-delimited string

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The /authenticated profile and team-member routes expose emails, so they
must require organizer access (not just any logged-in user) to match who
actually uses them.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…y dropping

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add vitest unit tests for safeAvatarUrl (URL sanitization) and getInitials,
and Hurl tests for the new organizer-only GET /profiles/authenticated and
GET /teams/{id}/members/authenticated endpoints (auth gating + private email
field + pagination).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
safeAvatarUrl returned encodeURI(parsed.href), but URL.href is already
percent-encoded, so e.g. a space became %2520 instead of %20. Return
parsed.href directly.

Add vitest coverage for buildPaginationMeta and PaginationQuerySchema/
IdParamSchema (both exercised by the new authenticated list endpoints), and
for the team, team-members, and auth validation schemas.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The committed versions failed dprint check (extra blank lines in
AddTeamFormModal/TeamDetail, missing trailing newline in teams/style.css),
which would fail the CI format:check step.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread api/src/index.ts Outdated
export type IdParam = z.infer<typeof IdParamSchema>;

export const PaginationQuerySchema = z.object({
limit: z.coerce.number().int().positive().optional(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see the 'page' getting a default but 'limit' does not have one. Is it intentional or would we want a default for limit too?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

+1: This should limit the maximum number of fetched rows.

Comment thread api/src/routes/team-members/data.ts Outdated
Comment thread api/src/routes/team-members/data.ts
.trim()
.min(1, 'Avatar must be at least one character long.')
.max(SHORT_TEXT_SIZE_IN_CHAR, `Avatar must be at most ${SHORT_TEXT_SIZE_IN_CHAR} characters long.`)
.refine((url) => isGravatarAvatarUrl(url) || isGravatarProfileUrl(url), { message: 'Avatar must be a Gravatar URL.' });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When we tested together, we noticed that the profile page is not saving updates and throwing some errors. This must be where that error is originating from based on matching error message. Could be worthwhile to dig here when fixing that issue.

FROM ${DBTables.ACCESS} AS access
WHERE
profile.id = ?
AND access.id = profile.id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this. Good catch.

Comment thread api/src/utils/gravatar.ts
Comment on lines +3 to +59
export function isGravatarAvatarUrl(url: string): boolean {
try {
const parsedUrl = new URL(url);
const [, avatarPath, hash] = parsedUrl.pathname.split('/');

const hostname = parsedUrl.hostname.toLowerCase();

return parsedUrl.protocol === 'https:' &&
(hostname === 'gravatar.com' || hostname.endsWith('.gravatar.com')) &&
avatarPath === 'avatar' &&
GRAVATAR_HASH_PATTERN.test(hash ?? '');
} catch {
return false;
}
}

export function isGravatarProfileUrl(url: string): boolean {
try {
const parsedUrl = new URL(url);
const [firstPath = '', secondPath = '', thirdPath = '', extraPath] = parsedUrl.pathname.split('/').filter(Boolean);
const hostname = parsedUrl.hostname.toLowerCase();

if (parsedUrl.protocol !== 'https:') { return false; }

return (
(hostname === 'gravatar.com' || hostname === 'www.gravatar.com') &&
firstPath !== 'avatar' &&
!secondPath
) || (
hostname === 'api.gravatar.com' &&
firstPath === 'v3' &&
secondPath === 'profiles' &&
thirdPath !== '' &&
!extraPath
);
} catch {
return false;
}
}

export async function resolveGravatarAvatarUrl(url: string): Promise<string | null> {
if (isGravatarAvatarUrl(url)) { return url; }
if (!isGravatarProfileUrl(url)) { return url; }

const parsedUrl = new URL(url);
const [firstPath = '', , thirdPath = ''] = parsedUrl.pathname.split('/').filter(Boolean);
const slug = parsedUrl.hostname === 'api.gravatar.com' ? thirdPath : firstPath;
try {
const response = await fetch(`https://api.gravatar.com/v3/profiles/${encodeURIComponent(slug.replace(/\.card$/iu, ''))}`);
if (!response.ok) { return null; }

const profile: { avatar_url?: string } = await response.json();
return profile.avatar_url && isGravatarAvatarUrl(profile.avatar_url) ? profile.avatar_url : null;
} catch {
return null;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: These suite of functions could benefit from a few ts docs. They are related and similar, and requires a little work to understand how they are different and what do they do.

Comment thread src/context/AuthContext.tsx Outdated
Comment thread api/src/utils/gravatar.ts Outdated
bcha92 and others added 6 commits June 25, 2026 21:14

@manvinderjit manvinderjit left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@kbventures Great work and thank you for all your effort.

I have done a quick overall review of the changes and run the project locally. I have only pointed major blockers.

As the code uses AI, there is potential for refactoring to improve usability but considering we need to got to MVP ASAP, I am ignoring those for now.

return context.json({ message: 'Invalid or missing token' } satisfies StatusResponse, StatusCodes.BAD_REQUEST);
}

const success = await context.env.PasswordResetToken.get(token);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

bug: The reset password feature is broken because of the following key change from resetToken - email

to
email - resetToken

It was brought about by the following code on line 297

await context.env.PasswordResetToken.put(email, resetToken, { expirationTtl: TEN_MINUTES_IN_SECONDS });
Image

The password reset token fetched from database will always be empty because get(token) still uses the token as the key, but the key is now email, so it will never find anything.

The fetch should use email as the key and not token.

Comment thread src/components/AuthenticatedLayout/AuthenticatedLayout.tsx Outdated
accessLevel: null,
avatar: DEFAULT_AVATAR,
isLoading: true,
profileStatus: null

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

suggestion: instead of using profileStatus: null, it might be better to have a dedicated isAuthenticated boolean value with status as true or false.

related to: https://github.com/torontojs/communityhub/pull/331/changes#r3495654530

Comment thread src/hooks/useHeartBeat.ts
credentials: 'include'
});
if (isLoading) { return; }
if (profileStatus !== null) { window.location.href = '/pages/home'; }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

question: the check if (profileStatus !== null) { window.location.href = '/pages/home'; } relies on the assumption that the api will always return a status.

What if the api doesn't return a status or returns a falsy status and the user is still authenticated?

Might be better to add an isAuthenticated boolean as indicated in the following comment

related to:
https://github.com/torontojs/communityhub/pull/331/changes#r3495651641

Comment thread api/src/routes/team-members/data.ts
export type IdParam = z.infer<typeof IdParamSchema>;

export const PaginationQuerySchema = z.object({
limit: z.coerce.number().int().positive().optional(),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

+1: This should limit the maximum number of fetched rows.

.authenticated-layout .sidebar-left {
border-right: var(--border);
box-sizing: border-box;
height: 41.25rem;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

css: The vertical side bar border doesn't extend to the full page length when the page is bigger than viewport.

Image


const VALID_UUID = '3c5123c0-8548-4a02-a83c-32e9ce67eae8';

describe('PaginationQuerySchema', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comprehensive suite of test (Y).

Comment thread src/components/AuthenticatedLayout/AuthenticatedLayout.tsx Outdated
return {
offset,
start: offset,
end: !limit || offset + limit > total ? total - 1 : offset + limit - 1,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question: It seems like against an empty table (total=0), end will be -1. If ever this happens, will this break things?

kbventures and others added 3 commits July 4, 2026 07:37
…ate-minor-changes-in-dependencies-and-dev-dependencies-group-no-breaking-changes

298 fix: minor dependency updates to address security vulnerabilities
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend/API 🍑 Anything related to the backend or API side of things Docs 📚 Improvements or additions to documentation Frontend 💅 Anything related to the frontend side of things

Projects

Status: Assigned

Development

Successfully merging this pull request may close these issues.

[Teams Page] Teams Page Front End Implementtion

7 participants