Skip to content

fix: initialized flags on event-types page#13469

Merged
keithwillcode merged 10 commits intocalcom:mainfrom
ibex088:fix/initialize-flags-on-event-types
Feb 2, 2024
Merged

fix: initialized flags on event-types page#13469
keithwillcode merged 10 commits intocalcom:mainfrom
ibex088:fix/initialize-flags-on-event-types

Conversation

@ibex088
Copy link
Copy Markdown
Contributor

@ibex088 ibex088 commented Jan 30, 2024

What does this PR do?

Fixes #13468

added it for event-types page only, fixes the isses in hand

Future consideration: Potentially enabling flags to be accessible on all pages consistently.

@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 30, 2024

@SomayChauhan is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 30, 2024

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 30, 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! 🙌

@PeerRich
Copy link
Copy Markdown
Member

can we also remove the feature flag for managed event types? its not needed anymore

@github-actions github-actions Bot added the ❗️ migrations contains migration files label Jan 31, 2024
@ibex088
Copy link
Copy Markdown
Contributor Author

ibex088 commented Jan 31, 2024

removed

@@ -0,0 +1,3 @@
-- revert 20230404202721_add_feature_flag_managed_event_types

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.

Why are we deleting this feature flag?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

can we also remove the feature flag for managed event types? its not needed anymore

i thought @PeerRich meant its not need anymore, so removed it
otherwise the toggle is showing up in settings/admin/flags

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes, i am ok removing this. this was used to roll it out slowly but now managed event types is a core part of our offering and doenst need a feature flag imo

@keithwillcode keithwillcode marked this pull request as draft January 31, 2024 09:29
@ibex088 ibex088 marked this pull request as ready for review January 31, 2024 09:38
@ibex088 ibex088 marked this pull request as draft January 31, 2024 09:38
@PeerRich PeerRich marked this pull request as ready for review January 31, 2024 10:51
@keithwillcode keithwillcode enabled auto-merge (squash) January 31, 2024 11:06
Comment on lines +1 to +23
import type { GetServerSidePropsContext } from "next";

import { getServerSession } from "@calcom/features/auth/lib/getServerSession";

import { ssrInit } from "@server/lib/ssr";

export const getServerSideProps = async (context: GetServerSidePropsContext) => {
const ssr = await ssrInit(context);
const session = await getServerSession({ req: context.req, res: context.res });

if (!session) {
return {
redirect: {
redirect: {
permanent: false,
destination: "/auth/login",
},
},
};
}

return { props: { trpcState: ssr.dehydrate() } };
};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we even need this if we remove the feature flag for managed event types?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i think we should still include this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok.

btw. end to end tests failing, not sure if its related to this PR @keithwillcode ?

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.

It’s not unfortunately. We have E2E test suite issues at the moment. Hoping to resolve by tomorrow and will get this merged.

@PeerRich PeerRich added the Low priority Created by Linear-GitHub Sync label Jan 31, 2024
@PeerRich PeerRich added this to the v3.9 milestone Jan 31, 2024
@keithwillcode keithwillcode added the community Created by Linear-GitHub Sync label Jan 31, 2024
@keithwillcode keithwillcode modified the milestones: v3.9, v3.8 Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community Created by Linear-GitHub Sync Low priority Created by Linear-GitHub Sync ❗️ migrations contains migration files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unable to create a managed event type

3 participants