Skip to content

Commit

Permalink
refactor(ff): make flag a bool flag
Browse files Browse the repository at this point in the history
  • Loading branch information
seaerchin committed Oct 20, 2023
1 parent 50ae6aa commit 7880953
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 39 deletions.
1 change: 1 addition & 0 deletions src/constants/featureFlags.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export const FEATURE_FLAGS = {
GGS_WHITELISTED_REPOS: "ggs_whitelisted_repos",
IS_BUILD_TIMES_REDUCTION_ENABLED: "is_build_times_reduction_enabled",
IS_GGS_WHITELISTED: "is_ggs_whitelisted",
} as const
43 changes: 11 additions & 32 deletions src/middleware/routeHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,13 @@ const gitFileSystemService = new GitFileSystemService(
new SimpleGit({ maxConcurrentProcesses: MAX_CONCURRENT_GIT_PROCESSES })
)

const isRepoWhitelisted = (siteName, ggsWhitelistedRepos) => {
const isRepoWhitelisted = (siteName, growthbook) => {
// TODO: adding log to simplify debugging, to be removed after stabilising
logger.info(
`Checking if ${siteName} is GGS whitelisted: ${ggsWhitelistedRepos.includes(
siteName
)}`
)
ggsWhitelistedRepos.includes(siteName)
logger.info(`Evaluating if ${siteName} is GGS whitelisted.`)
// NOTE: Growthbook has to be optional
// as we cannot guarantee its presence on `sessionData`.
// It is present iff there is a `siteHandler` attached.
return !!growthbook?.getFeatureValue(FEATURE_FLAGS.IS_GGS_WHITELISTED, false)
}

const handleGitFileLock = async (repoName, next) => {
Expand Down Expand Up @@ -74,23 +73,15 @@ const attachWriteRouteHandlerWrapper = (routeHandler) => async (
const { siteName } = req.params
const { growthbook } = req

let ggsWhitelistedRepos = { repos: [] }
if (growthbook) {
ggsWhitelistedRepos = growthbook.getFeatureValue(
FEATURE_FLAGS.GGS_WHITELISTED_REPOS,
{
repos: [],
}
)
}

let isGitAvailable = true

// only check git file lock if the repo is whitelisted
if (isRepoWhitelisted(siteName, ggsWhitelistedRepos.repos)) {
if (isRepoWhitelisted(siteName, growthbook)) {
isGitAvailable = await handleGitFileLock(siteName, next)
}

if (!isGitAvailable) return

try {
await lock(siteName)
} catch (err) {
Expand All @@ -102,6 +93,7 @@ const attachWriteRouteHandlerWrapper = (routeHandler) => async (
await unlock(siteName)
next(err)
})

try {
await unlock(siteName)
} catch (err) {
Expand All @@ -120,20 +112,7 @@ const attachRollbackRouteHandlerWrapper = (routeHandler) => async (
const { accessToken } = userSessionData
const { growthbook } = req

let ggsWhitelistedRepos = { repos: [] }
if (growthbook) {
ggsWhitelistedRepos = growthbook.getFeatureValue(
FEATURE_FLAGS.GGS_WHITELISTED_REPOS,
{
repos: [],
}
)
}

const shouldUseGitFileSystem = isRepoWhitelisted(
siteName,
ggsWhitelistedRepos.repos
)
const shouldUseGitFileSystem = isRepoWhitelisted(siteName, growthbook)

const isGitAvailable = await handleGitFileLock(siteName, next)
if (!isGitAvailable) return
Expand Down
13 changes: 7 additions & 6 deletions src/services/db/RepoService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,14 @@ export default class RepoService extends GitHubService {
ggsWhitelistedRepos: string[]
): boolean {
// TODO: Adding for initial debugging if required. Remove once stabilised
logger.info(
`Evaluating if ${repoName} is GGS whitelisted: ${ggsWhitelistedRepos.includes(
repoName
)}`
logger.info(`Evaluating if ${repoName} is GGS whitelisted.`)
// NOTE: Growthbook has to be optional
// as we cannot guarantee its presence on `sessionData`.
// It is present iff there is a `siteHandler` attached.
return !!growthbook?.getFeatureValue(

Check failure on line 108 in src/services/db/RepoService.ts

View workflow job for this annotation

GitHub Actions / build

Cannot find name 'growthbook'. Did you mean 'GrowthBook'?
FEATURE_FLAGS.IS_GGS_WHITELISTED,
false
)

return ggsWhitelistedRepos.includes(repoName)
}

getCommitDiff(siteName: string, base?: string, head?: string) {
Expand Down
10 changes: 9 additions & 1 deletion src/services/db/__tests__/RepoService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ describe("RepoService", () => {
repos: ["fake-repo", mockSiteName],
},
},
is_ggs_whitelisted: {
defaultValue: true,
},
})
})

Expand All @@ -110,13 +113,18 @@ describe("RepoService", () => {
expect(actual2).toBe(true)
})

it("should indicate non-whitelisted repos as non-whitelisted correctly", () => {
it("should indicate blacklisted repos as not being allowed", () => {
// NOTE: Mock a blacklisted repo
const gbSpy = jest.spyOn(mockGrowthBook, "getFeatureValue")
gbSpy.mockReturnValueOnce(false)

const actual = RepoService.isRepoGgsWhitelisted(
"not-whitelisted",
RepoService.getGgsWhitelistedRepos(
mockUserWithSiteSessionDataAndGrowthBook.growthbook
)
)

expect(actual).toBe(false)
})
})
Expand Down
1 change: 1 addition & 0 deletions src/types/featureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
export interface FeatureFlags {
ggs_whitelisted_repos: { repos: string[] }
is_build_times_reduction_enabled: boolean
is_ggs_whitelisted: boolean
}

// List of attributes we set in GrowthBook Instance in auth middleware
Expand Down

0 comments on commit 7880953

Please sign in to comment.