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/add enterprise badge to change req settings #2585

Merged

Conversation

andreas-unleash
Copy link
Contributor

Disable change requests for Pro and oss

About the changes

Closes #

Important files

Discussion points

Signed-off-by: andreas-unleash <andreas@getunleash.ai>
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
@vercel
Copy link

vercel bot commented Dec 1, 2022

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

Name Status Preview Comments Updated
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Dec 5, 2022 at 3:16PM (UTC)
1 Ignored Deployment
Name Status Preview Comments Updated
unleash-docs ⬜️ Ignored (Inspect) Dec 5, 2022 at 3:16PM (UTC)

…ings

# Conflicts:
#	frontend/src/component/project/Project/ProjectSettings/ChangeRequestConfiguration/ChangeRequestConfiguration.tsx
#	frontend/src/hooks/usePlausibleTracker.ts
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
const { hasAccess } = useContext(AccessContext);
const { isOss, uiConfig } = useUiConfig();
const isPro = !(
Boolean(uiConfig.versionInfo?.current.oss) ||
Copy link
Member

Choose a reason for hiding this comment

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

uiConfig has isOSS

Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to have this in a reusable hook or util: const { isOss, isPro, isEnteprise } = usePlan() or something. There's also useInstanceStatus.

Copy link
Member

Choose a reason for hiding this comment

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

Here's something that may help: #2600

{
Header: 'Required approvals',
align: 'center',
Cell: ({ row: { original } }: any) => {
Copy link
Member

Choose a reason for hiding this comment

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

maybe this can be extracted to a component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, just moved the implementation for the moment, but it could use some cleanup

andreas-unleash and others added 3 commits December 2, 2022 11:37
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
Co-authored-by: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com>
…equestConfiguration/ChangeRequestConfiguration.tsx

Co-authored-by: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com>
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
…nge_req_settings' into feat/add_enterprise_badge_to_change_req_settings
…Settings.tsx

Co-authored-by: Nuno Góis <github@nunogois.com>
Co-authored-by: Nuno Góis <github@nunogois.com>
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
…nge_req_settings' into feat/add_enterprise_badge_to_change_req_settings

# Conflicts:
#	frontend/src/component/project/ProjectAccess/ProjectAccess.tsx
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
Copy link
Member

@nunogois nunogois left a comment

Choose a reason for hiding this comment

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

Looking good!
I still think we should have a look at some of the remaining comments before merging, though.

andreas-unleash and others added 6 commits December 5, 2022 17:05
Co-authored-by: Nuno Góis <github@nunogois.com>
Co-authored-by: Nuno Góis <github@nunogois.com>
Co-authored-by: Nuno Góis <github@nunogois.com>
Co-authored-by: Nuno Góis <github@nunogois.com>
Co-authored-by: Nuno Góis <github@nunogois.com>
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
Copy link
Member

@nunogois nunogois left a comment

Choose a reason for hiding this comment

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

LGTM

@nicolaesocaciu
Copy link
Contributor

@andreas-unleash I need to double check the messages with Elise and come back with a final version, so you know that we will need another small PR for this

But other than this it looks super nice 👍

@andreas-unleash andreas-unleash merged commit 7af155f into main Dec 6, 2022
@andreas-unleash andreas-unleash deleted the feat/add_enterprise_badge_to_change_req_settings branch December 6, 2022 08:05
nunogois added a commit that referenced this pull request Dec 6, 2022
Hopefully a cleaner and DRY way of checking for the current Unleash plan
level, which may help in cases like
#2585 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants