task: enabled in OSS. #8856

chriswk · 2024-11-26T08:13:25Z

Add a column to projects and environments signifying if they're on even if instance is reverted back to OSS after having been in enterprise.

Points of discussion before merging:

I've updated the environments store to only return environments where enabled_in_oss is true when isOss is true.
I've updated the project store to only return id = 'default' when isOss is true
I've updated the client features endpoint to only return project = 'default' configurations.
Going back and forth between Enterprise and OSS does not delete any data, but it just won't be visible when running OSS
Currently our isOss math is !isEnterprise && ui.environment !== 'pro' && !isTest // the isTest was included to avoid having to set all our tests up to flag themselves as enterprise.
I would've liked the isOss check to be replaced with a call to license service and maybe two separate checks hasAdditionalProjects and hasAdditionalEnvironments, but didn't want to have a store depending on a service so for now, the check is only dependent on configuration.

vercel · 2024-11-26T08:13:30Z

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

Name	Status	Preview	Comments	Updated (UTC)
unleash-monorepo-frontend	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 28, 2024 3:01pm

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
unleash-docs	⬜️ Ignored (Inspect)	Visit Preview		Nov 28, 2024 3:01pm

github-actions · 2024-11-26T08:13:44Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details

Scanned Files

gastonfournier

Overall looks fine, I'm wondering (not requiring) if moving the calculation of isOss outside the constructor would be better... maybe a method in config object could be central enough

src/lib/features/client-feature-toggles/client-feature-toggle-store.ts

gastonfournier · 2024-11-27T09:32:56Z

src/migrations/20241125115848-add-enabled-in-oss-to-project-and-environment.js

+        ALTER TABLE environments ADD COLUMN enabled_in_oss BOOLEAN NOT NULL DEFAULT false;
+        ALTER TABLE projects ADD COLUMN enabled_in_oss BOOLEAN NOT NULL DEFAULT false;


Hacking this would not be difficult... just run a query against the db changing all enabled_in_oss to true... it adds some complexity though

indeed, or you can just hack the store and remove the isOss check. The goal is not to stop all use of enterprise features (users willing to change the code won't be deterred for long anyway), but to at least make it require a conscious action to work around.

I think G has a point here though. I think the barrier between touching source code and touching database entities is very different, because patching source code means redoing that every time we create updates and dealing with the merges and database patches are trivially executed once.

I'm not sure I'm convinced either way but I wouldn't object to seeing "default", "development", "production" as a constant in source code

And we hide default by default these days, so it's really only development and production.
Yeah ok, let's be explicit then. Updating

chriswk · 2024-11-27T12:26:49Z

This failing e2e test is real, because it's testing additional projects, and we haven't enabled enterprise here. I'll look into how to solve that.

sighphyre

I think the approach is okay because I can't see another way but I would really like to see some tests around this, if for no other reason than the likelihood of future patches to adjacent code completely borking the queries

sighphyre · 2024-11-27T15:31:09Z

src/lib/features/client-feature-toggles/client-feature-toggle-service.ts

@@ -27,6 +27,7 @@ export class ClientFeatureToggleService {
        this.logger = getLogger('services/client-feature-toggle-service.ts');
        this.segmentReadModel = segmentReadModel;
        this.clientFeatureToggleStore = clientFeatureToggleStore;
+        const isTest = process.env.NODE_ENV === 'test';


Why this change?

Because my initial thought had the oss check on services, and I forgot to remove the last trace. :)

sighphyre · 2024-11-27T15:37:45Z

src/migrations/20241125115848-add-enabled-in-oss-to-project-and-environment.js

+        ALTER TABLE environments ADD COLUMN enabled_in_oss BOOLEAN NOT NULL DEFAULT false;
+        ALTER TABLE projects ADD COLUMN enabled_in_oss BOOLEAN NOT NULL DEFAULT false;


I think G has a point here though. I think the barrier between touching source code and touching database entities is very different, because patching source code means redoing that every time we create updates and dealing with the merges and database patches are trivially executed once.

I'm not sure I'm convinced either way but I wouldn't object to seeing "default", "development", "production" as a constant in source code

sighphyre · 2024-11-27T15:39:26Z

the isTest was included to avoid having to set all our tests up to flag themselves as enterprise.

Nice

Add a column to projects and environments signifying if they're on even if instance is reverted back to OSS after having been in enterprise

gastonfournier

Ok, looks good to me, it's still easy to hack, but it's better than not doing anything

gastonfournier · 2024-11-29T08:31:45Z

src/lib/features/feature-toggle/feature-toggle-store.ts

+        if (isOss) {
+            return queryBuilder.andWhere('project', '=', 'default');
+        }


Looks easy to hack, but fair enough

This reverts commit 663b169.

Reverts #8856

chriswk self-assigned this Nov 26, 2024

vercel bot deployed to Preview – unleash-monorepo-frontend November 26, 2024 09:46 View deployment

vercel bot deployed to Preview – unleash-monorepo-frontend November 26, 2024 13:22 View deployment

chriswk marked this pull request as ready for review November 27, 2024 08:46

chriswk requested review from ivarconr, gastonfournier and sighphyre November 27, 2024 08:49

gastonfournier reviewed Nov 27, 2024

View reviewed changes

vercel bot deployed to Preview – unleash-monorepo-frontend November 27, 2024 13:15 View deployment

sighphyre reviewed Nov 27, 2024

View reviewed changes

vercel bot deployed to Preview – unleash-monorepo-frontend November 28, 2024 10:02 View deployment

chriswk added 5 commits November 28, 2024 13:52

task: enabled in OSS.

793f6b6

Add a column to projects and environments signifying if they're on even if instance is reverted back to OSS after having been in enterprise

chore: lint

8f5e62c

added filtering by enabled_in_oss to feature_environment store as well

14d0d9f

changes from PR

e70c0d5

fix: comments from PR, use whereIn instead of boolean field

be5b234

chriswk force-pushed the task/enabledInOss branch from dd43c67 to be5b234 Compare November 28, 2024 13:04

Roll back change to frontend e2e:oss script

8b91baa

vercel bot deployed to Preview – unleash-monorepo-frontend November 28, 2024 14:50 View deployment

fix: added e2e tests for project and environment filtering

c585f8c

chriswk force-pushed the task/enabledInOss branch from 9c2039c to c585f8c Compare November 28, 2024 14:58

vercel bot deployed to Preview – unleash-monorepo-frontend November 28, 2024 15:01 View deployment

gastonfournier approved these changes Nov 29, 2024

View reviewed changes

chriswk merged commit 663b169 into main Nov 29, 2024
11 checks passed

chriswk deleted the task/enabledInOss branch November 29, 2024 08:43

sjaanus added a commit that referenced this pull request Nov 29, 2024

Revert "task: enabled in OSS. (#8856)"

c1baf2f

This reverts commit 663b169.

sjaanus mentioned this pull request Nov 29, 2024

Revert "task: enabled in OSS." #8892

Merged

sjaanus added a commit that referenced this pull request Nov 29, 2024

Revert "task: enabled in OSS." (#8892)

3c01813

Reverts #8856

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

task: enabled in OSS. #8856

task: enabled in OSS. #8856

chriswk commented Nov 26, 2024 •

edited

Loading

vercel bot commented Nov 26, 2024 •

edited

Loading

github-actions bot commented Nov 26, 2024 •

edited

Loading

gastonfournier left a comment

gastonfournier Nov 27, 2024

chriswk Nov 27, 2024 •

edited

Loading

sighphyre Nov 27, 2024

chriswk Nov 28, 2024

chriswk commented Nov 27, 2024

sighphyre left a comment

sighphyre Nov 27, 2024

chriswk Nov 28, 2024

sighphyre Nov 27, 2024

sighphyre commented Nov 27, 2024

gastonfournier left a comment

gastonfournier Nov 29, 2024

		ALTER TABLE environments ADD COLUMN enabled_in_oss BOOLEAN NOT NULL DEFAULT false;
		ALTER TABLE projects ADD COLUMN enabled_in_oss BOOLEAN NOT NULL DEFAULT false;

task: enabled in OSS. #8856

task: enabled in OSS. #8856

Conversation

chriswk commented Nov 26, 2024 • edited Loading

vercel bot commented Nov 26, 2024 • edited Loading

github-actions bot commented Nov 26, 2024 • edited Loading

Dependency Review

OpenSSF Scorecard

Scanned Files

gastonfournier left a comment

Choose a reason for hiding this comment

gastonfournier Nov 27, 2024

Choose a reason for hiding this comment

chriswk Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

sighphyre Nov 27, 2024

Choose a reason for hiding this comment

chriswk Nov 28, 2024

Choose a reason for hiding this comment

chriswk commented Nov 27, 2024

sighphyre left a comment

Choose a reason for hiding this comment

sighphyre Nov 27, 2024

Choose a reason for hiding this comment

chriswk Nov 28, 2024

Choose a reason for hiding this comment

sighphyre Nov 27, 2024

Choose a reason for hiding this comment

sighphyre commented Nov 27, 2024

gastonfournier left a comment

Choose a reason for hiding this comment

gastonfournier Nov 29, 2024

Choose a reason for hiding this comment

chriswk commented Nov 26, 2024 •

edited

Loading

vercel bot commented Nov 26, 2024 •

edited

Loading

github-actions bot commented Nov 26, 2024 •

edited

Loading

chriswk Nov 27, 2024 •

edited

Loading