User Groups Paywall #10033

MrFlashAccount · 2024-05-22T11:31:26Z

Pull Request Description

Tl;dr

Closes: enso-org/cloud-v2#1200

This PR adds Initial paywall functionality inside enso and adds paywalls for the Users Group page in settings.

Demo Presentation

CleanShot.2024-05-22.at.15.14.53.mp4

Context:

This Change:

What this change does in the larger context. Specific details to highlight for review:

Adds a new set of components for Paywall functionality
Closes the UsersGroup functionaly behind 2 paywalls - first is for personal/free acc, and the second is for team acc

Test Plan:

The only catch with testing is how to switch UI between different states, and I don't have an answer except switching between accounts or modifying the code to invert the logic.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.

somebody1234 · 2024-05-22T13:13:30Z

mostly unrelated, but: i don't think we should let users on the Team plan manage user groups.
iirc the idea is, everyone is in the same user group, unconditionally. so the user group can't be deleted, and users can't be excluded from the group

MrFlashAccount · 2024-05-22T13:16:12Z

@somebody1234 We might want to move this discussion outside of the PR. Maybe move it to the related Issue? I guess we need opinions from @PabloBuchu and/or from @jdunkerley

MrFlashAccount · 2024-05-22T13:17:20Z

app/ide-desktop/eslint.config.js

@@ -137,14 +133,6 @@ const RESTRICTED_SYNTAXES = [
        selector: `TSAsExpression:not(:has(TSTypeReference > Identifier[name=const]))`,
        message: 'Avoid `as T`. Consider using a type annotation instead.',
    },
-    {


This rule makes no sense with 'exactOptionalPropertyTypes`.

exactOptionalPropertyTypes is not related to this rule. if anything, exactOptionalPropertyTypes removes undefined from the types of optional properties (unless explicitly declared as | undefined), meaning that this rule shouldn't cause any problem at all.

In case I want to allow passing smth: undefined, I need to specify it explicitly, but by specifying it explicitly, I'm making eslint angry.

@MrFlashAccount but the point of undefined is that you don't need to pass it explicitly. if you need to omit something, simply don't specify the property at all

Often we use the object destruction pattern and checking if a property is not undefined before passing it is tedious.

See: https://github.com/enso-org/enso/blob/develop/app/ide-desktop/lib/dashboard/src/components/AriaComponents/Form/Form.tsx#L55

MrFlashAccount · 2024-05-22T13:18:09Z

app/ide-desktop/eslint.config.js

@@ -461,11 +449,6 @@ export default [
                    selector: ':not(TSModuleDeclaration)[declare=true]',
                    message: 'No ambient declarations',
                },
-                {


This rule makes no sense in case if we re-import default statements from barrel files

MrFlashAccount · 2024-05-22T13:18:44Z

app/ide-desktop/eslint.config.js

@@ -87,10 +87,6 @@ const RESTRICTED_SYNTAXES = [
        selector: `:matches(ImportDefaultSpecifier[local.name=/^${NAME}/i], ImportNamespaceSpecifier > Identifier[name=/^${NAME}/i])`,
        message: `Don't prefix modules with \`${NAME}\``,
    },
-    {


Removed it because creating an interface sometimes it too annoying

see review comments

app/ide-desktop/lib/dashboard/src/components/Paywall/PaywallDialog.tsx

somebody1234 · 2024-05-22T13:15:41Z

app/ide-desktop/lib/dashboard/src/appUtils.tsx

+/**
+ * Build a Subscription URL for contacting sales.
+ */
+export function getContactSalesURL(): string {


is there a reason this should be a function?

also i don't think this is the right place to put these functions, as they are not being used in App.tsx, nor are they related to App.tsx.

They're here because they're related to Paths(and all paths are specified here). Eventually, we'll move the URLs closer to their pages and the function will follow.

all paths are specified here

correction: all paths used in App.tsx are specified here. the paths are (or at least, should be) used nowhere outside of App.tsx

correction: all paths used in App.tsx are specified here. the paths are (or at least, should be) used nowhere outside of App.tsx

Since we don't specify paths outside of the App.tsx this correction makes no change

getContactSalesURL is used only in getComponentForPlan.tsx and UpgradeButton.tsx, neither of which are App.tsx - unless i'm misunderstanding something.

We keep this function with the URL just because we don't want to forget it when we refactor the Router-related stuff and put to it's own place under /pages

But if you have a better option - I'll comply

app/ide-desktop/lib/dashboard/src/components/Paywall/PaywallDialogButton.tsx

app/ide-desktop/lib/dashboard/src/components/Paywall/UpgradeButton.tsx

app/ide-desktop/lib/dashboard/src/hooks/billing/FeaturesConfiguration.ts

somebody1234 · 2024-05-22T13:21:31Z

app/ide-desktop/lib/dashboard/src/hooks/billing/FeaturesConfiguration.ts

+  | (3 & { readonly name: PaywallLevelName; readonly label: text.TextId })
+
+export const PAYWALL_LEVELS: Record<PaywallLevelName, PaywallLevelValue> = {
+  free: Object.assign(0, { name: 'free', label: 'freePlanName' } as const),


is there a good reason to use Number objects here? why are the numbers not simply a property of the object?

Because plans are Inclusive - all features from a level below are included in the levels above.
So if we want to check that the feature is available, we can simply compare 2 numbers - the user's level and the feature level.

we can do that with a property as well. user.plan.level < plan.level

I see no arguments why user.plan.level < plan.level is better than level < userLevel

it's not! but i'm really, really unsure about passing around a Number object, just so the code looks a tiny bit cleaner in one specific situation.

I my experience checking things like 'level === PaywallLevels.enterprise' or 'level > PaywallLevels.enterpriseEntry' is more than just a one specific case.
Also, since level is a primitive type, we don't care if references don't match.

I agree this looks strange and unusual, but it has advantages and doesn't violate the agreement we have in javascript, like do not mutate prototypes

app/ide-desktop/lib/dashboard/src/text/index.ts

app/ide-desktop/lib/dashboard/src/components/styled/HorizontalMenuBar.tsx

app/ide-desktop/lib/dashboard/src/components/Paywall/components/PaywallLock.tsx

app/ide-desktop/lib/dashboard/src/components/Paywall/components/PaywallButton.tsx

somebody1234

buttons in settings sidebar no longer have lowered opacity when inactive (they are supposed to).

somebody1234 · 2024-05-23T09:27:28Z

app/ide-desktop/lib/dashboard/src/layouts/SettingsSidebar.tsx

-                />
-              ))}
+
+              <ariaComponents.ButtonGroup gap="xxsmall" direction="column" align="start">


please try to avoid changing layout from the figma design. (the figma design does not have a gap)

Gaps are intended because, even though it's not described in the design if we remove the gaps, menu items will look like they stuck together(see screenshot). Alternatively, we can use a different hover for elements.

fair. in that case, we should probably reduce the size of the menu entries and add vertical padding so that the overall height in pixels is still the same? (32 * entries.length)

alternatively reduce by just 1px or 0.5px to make the gaps consistent with those ofnthe assets table rows

MrFlashAccount · 2024-05-23T10:56:26Z

buttons in the settings sidebar no longer have lowered opacity when inactive (they are supposed to).

It's intended, we decided to make the buttons look like that with @PabloBuchu because currently, the inactive items look disabled.

somebody1234

requested changes have been resolved

MrFlashAccount · 2024-05-23T11:21:53Z

requested changes have been resolved

Can I ask to approve the PR then?

somebody1234

github moment

somebody1234 · 2024-05-23T11:47:09Z

(remember that QA is not yet done though)

MrFlashAccount · 2024-06-06T16:56:05Z

Closing this PR in favor of #10087

MrFlashAccount self-assigned this May 22, 2024

MrFlashAccount force-pushed the wip/sergeigarin/paywalls branch from e89d08c to 218694e Compare May 22, 2024 11:32

MrFlashAccount added g-dashboard CI: No changelog needed Do not require a changelog entry for this PR. x-new-feature Type: new feature request labels May 22, 2024

MrFlashAccount marked this pull request as ready for review May 22, 2024 13:06

MrFlashAccount requested review from PabloBuchu, indiv0 and somebody1234 as code owners May 22, 2024 13:06

MrFlashAccount commented May 22, 2024

View reviewed changes

somebody1234 reviewed May 22, 2024

View reviewed changes

MrFlashAccount force-pushed the wip/sergeigarin/paywalls branch 2 times, most recently from 81f2638 to 86008ae Compare May 23, 2024 08:05

somebody1234 requested changes May 23, 2024

View reviewed changes

somebody1234 reviewed May 23, 2024

View reviewed changes

somebody1234 approved these changes May 23, 2024

View reviewed changes

MrFlashAccount added 6 commits May 23, 2024 17:29

paywalls

54677f0

fix text

a3c345d

Paywall button

5ddece8

Imrovements

0bf2f05

Finish up user groups Paywall

e8d18e7

Fix small issues

635acfc

MrFlashAccount added 4 commits May 23, 2024 17:29

fix lint

2c0b472

fix review issues

53f04b9

Improve sidebar view

18d88ba

fix lint

9d08190

MrFlashAccount force-pushed the wip/sergeigarin/paywalls branch from b787c08 to 9d08190 Compare May 23, 2024 14:32

small fixes in button

9cb7c89

MrFlashAccount mentioned this pull request May 27, 2024

Share + Invite Paywalls #10087

Closed

4 tasks

MrFlashAccount closed this Jun 6, 2024

User Groups Paywall #10033

User Groups Paywall #10033

Conversation

MrFlashAccount commented May 22, 2024 • edited Loading

Pull Request Description

Tl;dr

Context:

This Change:

Test Plan:

Important Notes

Checklist

somebody1234 commented May 22, 2024

MrFlashAccount commented May 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrFlashAccount May 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrFlashAccount May 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrFlashAccount May 22, 2024 • edited Loading

Choose a reason for hiding this comment

somebody1234 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrFlashAccount May 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrFlashAccount commented May 23, 2024

somebody1234 left a comment

Choose a reason for hiding this comment

MrFlashAccount commented May 23, 2024

somebody1234 left a comment

Choose a reason for hiding this comment

somebody1234 commented May 23, 2024

MrFlashAccount commented Jun 6, 2024

MrFlashAccount commented May 22, 2024 •

edited

Loading

MrFlashAccount May 22, 2024 •

edited

Loading

MrFlashAccount May 22, 2024 •

edited

Loading

MrFlashAccount May 22, 2024 •

edited

Loading

MrFlashAccount May 23, 2024 •

edited

Loading