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

feat: Implement orgs/teams logoUrls #13699

Merged
merged 8 commits into from
Feb 16, 2024

Conversation

emrysal
Copy link
Contributor

@emrysal emrysal commented Feb 15, 2024

What does this PR do?

  • Starts uploading the organisation logo + using it.
  • Various optimisation tweaks here and there

Copy link

vercel bot commented Feb 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Ignored Deployments
Name Status Preview Comments Updated (UTC)
ai ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:14pm
cal ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:14pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:14pm
qa ⬜️ Ignored (Inspect) Visit Preview Feb 15, 2024 10:14pm

Copy link
Contributor

github-actions bot commented Feb 15, 2024

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.

@keithwillcode keithwillcode added core area: core, team members only foundation labels Feb 15, 2024
@@ -325,7 +325,7 @@ const TeamProfileForm = ({ team }: TeamProfileFormProps) => {
<Controller
control={form.control}
name="logo"
render={({ field: { value } }) => {
render={({ field: { value, onChange } }) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Utilise RHF

@@ -99,12 +99,6 @@ export class EventTypeRepository {
const profileId = lookupTarget.type === LookupTarget.User ? null : lookupTarget.id;
const select = {
...eventTypeSelect,
// TODO: As required by getByViewHandler - Make it configurable
team: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cleanup: Redundant subQuery, teamId already available in eventTypeSelect (does extra query)

@@ -1,4 +1,5 @@
import jimp from "jimp";
import "server-only";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bit of safe-guarding.

@@ -20,6 +21,30 @@ type UpdateOptions = {
input: TUpdateInputSchema;
};

export const uploadLogo = async ({ teamId, logo: data }: { teamId: number; logo: string }) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO, extract to lib fn

@@ -13,9 +14,9 @@ export const ZUpdateInputSchema = z.object({
bio: z.string().optional(),
logo: z
.string()
.transform(async (val) => await resizeBase64Image(val))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add resize (before, the actual full size image was used, was potentially huge)

Copy link
Contributor

github-actions bot commented Feb 15, 2024

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

Copy link

deploysentinel bot commented Feb 15, 2024

Current Playwright Test Results Summary

✅ 447 Passing - ⚠️ 13 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 02/15/2024 10:34:26pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: a0d455c

Started: 02/15/2024 10:24:03pm UTC

⚠️ Flakes

📄   apps/web/playwright/booking/selectQuestion.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking With Phone Question and Each Other Question Booking With Select Question and multiselect Question Select and multiselect text not required
Retry 1Initial Attempt
0% (0) 0 / 254 runs
failed over last 7 days
6.30% (16) 16 / 254 runs
flaked over last 7 days

📄   apps/web/playwright/booking/radioGroupQuestion.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking With Radio Question and Each Other Question Booking With Radio Question and Address Question Booking With Radio Question and multiselect Question Radio and multiselect text not required
Retry 1Initial Attempt
0% (0) 0 / 255 runs
failed over last 7 days
4.31% (11) 11 / 255 runs
flaked over last 7 days
Booking With Radio Question and Each Other Question Booking With Radio Question and Address Question Booking With Radio Question and Phone Question Radio and Phone not required
Retry 1Initial Attempt
0% (0) 0 / 254 runs
failed over last 7 days
7.09% (18) 18 / 254 runs
flaked over last 7 days

📄   apps/web/playwright/login.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
user can login & logout succesfully -- future login flow user & logout using dashboard
Retry 1Initial Attempt
0.78% (2) 2 / 255 runs
failed over last 7 days
45.49% (116) 116 / 255 runs
flaked over last 7 days

📄   apps/web/playwright/booking/addressQuestione2e/addressQuestion.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking With Address Question and Each Other Question Booking With Address Question and Number Question Address required and Number required
Retry 1Initial Attempt
0% (0) 0 / 258 runs
failed over last 7 days
5.81% (15) 15 / 258 runs
flaked over last 7 days
Booking With Address Question and Each Other Question Booking With Address Question and Short text question Address and Short text not required
Retry 2Retry 1Initial Attempt
0% (0) 0 / 258 runs
failed over last 7 days
5.43% (14) 14 / 258 runs
flaked over last 7 days

📄   apps/web/playwright/booking/phoneQuestion.e2e.ts • 3 Flakes

Top 1 Common Error Messages

null

3 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Booking With Phone Question and Each Other Question Booking With Phone Question and Address Question Phone and Address required
Retry 1Initial Attempt
2.26% (6) 6 / 266 runs
failed over last 7 days
5.64% (15) 15 / 266 runs
flaked over last 7 days
Booking With Phone Question and Each Other Question Booking With Phone Question and Address Question Booking With Phone Question and multiselect Question Phone required and multiselect text not required
Retry 1Initial Attempt
0% (0) 0 / 257 runs
failed over last 7 days
5.06% (13) 13 / 257 runs
flaked over last 7 days
Booking With Phone Question and Each Other Question Booking With Phone Question and Address Question Booking With Phone Question and Short text question Phone and Short text required
Retry 1Initial Attempt
0% (0) 0 / 256 runs
failed over last 7 days
5.08% (13) 13 / 256 runs
flaked over last 7 days

📄   apps/web/playwright/insights.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Insights should have all option in team-and-self filter as admin
Retry 1Initial Attempt
0.38% (1) 1 / 262 run
failed over last 7 days
0.76% (2) 2 / 262 runs
flaked over last 7 days
Insights should be able to switch between memberUsers
Retry 1Initial Attempt
0.38% (1) 1 / 262 run
failed over last 7 days
0.38% (1) 1 / 262 run
flaked over last 7 days

📄   apps/web/playwright/event-types.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Event Types tests -- future user Different Locations Tests Can add Link Meeting as location and book with it
Retry 1Initial Attempt
0% (0) 0 / 263 runs
failed over last 7 days
6.84% (18) 18 / 263 runs
flaked over last 7 days

📄   apps/web/playwright/organization/team-management.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Teams Can create teams via Wizard
Retry 1Initial Attempt
0% (0) 0 / 236 runs
failed over last 7 days
0.85% (2) 2 / 236 runs
flaked over last 7 days

View Detailed Build Results


Copy link

graphite-app bot commented Feb 15, 2024

Graphite Automations

A Graphite automation took an action on this PR • (02/15/24)

1 reviewer was added based on Keith Williams's automation, 'Add foundation team as reviewer'

Comment on lines 77 to 84
if (input.logo) {
data.logo = input.logo;
data.logoUrl = await uploadLogo({
logo: input.logo,
teamId: currentOrgId,
});
}

Copy link
Contributor

Choose a reason for hiding this comment

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

There should be an option to remove the logo also like we do in profile page of user

Copy link
Contributor

Choose a reason for hiding this comment

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

the UI already has remove button so you just have to add logic here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot, missed the else in case of a "null" value.

@keithwillcode keithwillcode added this to the v3.9 milestone Feb 15, 2024
@@ -87,6 +87,7 @@ export async function getTeamWithMembers(args: {
name: true,
slug: true,
...(!!includeTeamLogo ? { logo: true } : {}),
Copy link
Contributor

Choose a reason for hiding this comment

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

@emrysal do we need to use includeTeamLogo now?

Copy link
Contributor Author

@emrysal emrysal Feb 15, 2024

Choose a reason for hiding this comment

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

We'll remove that - for this PR we still need logo

Udit-takkar
Udit-takkar previously approved these changes Feb 15, 2024
Copy link
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

LGTM

@emrysal
Copy link
Contributor Author

emrysal commented Feb 15, 2024

Converted to draft as we don't want to merge this before 3.8 - will be released as part of 3.8.1 to ensure we don't release big changes right now.

Copy link
Contributor

@keithwillcode keithwillcode left a comment

Choose a reason for hiding this comment

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

Any tests we can add here?

@keithwillcode
Copy link
Contributor

@emrysal In a follow-up PR, let's add tests for

@keithwillcode keithwillcode merged commit c08365f into main Feb 16, 2024
39 checks passed
@keithwillcode keithwillcode deleted the feat/logo-url-team-members-orgs-and-teams branch February 16, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core area: core, team members only foundation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants