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.

Sign up for GitHub

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

chore: remove released feature flag for extensions/ future state #5926

Merged
merged 8 commits into from
Dec 10, 2024
+55 −102
Merged

chore: remove released feature flag for extensions/ future state #5926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
60c4cc8
chore: remove released feature flag for future state v2
sarahetter Dec 5, 2024
13a6108
chore: format
sarahetter Dec 5, 2024
4aa5cae
chore: tests
sarahetter Dec 5, 2024
578bfde
test: modify tests
sarahetter Dec 6, 2024
0b011ec
fix: convert normalize case file to typescript to make the build not …
sarahetter Dec 6, 2024
60cfa21
chore: change test name
sarahetter Dec 6, 2024
562038a
Merge branch 'main' into sarahetter/remove-future-state-flag
sarahetter Dec 10, 2024
c40c3f3
Merge branch 'main' into sarahetter/remove-future-state-flag
sarahetter Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 5 additions & 42 deletions packages/config/src/api/site_info.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,53 +35,18 @@ export const getSiteInfo = async function ({
context,
offline = false,
testOpts = {},
featureFlags = {},
siteFeatureFlagPrefix,
}: GetSiteInfoOpts) {
const { env: testEnv = false } = testOpts

const useV2Endpoint = !!accountId && featureFlags.cli_integration_installations_meta

if (useV2Endpoint) {
if (api === undefined || mode === 'buildbot' || testEnv) {
const siteInfo: { id?: string; account_id?: string } = {}

if (siteId !== undefined) siteInfo.id = siteId
if (accountId !== undefined) siteInfo.account_id = accountId

const integrations =
mode === 'buildbot' && !offline
? await getIntegrations({ siteId, testOpts, offline, useV2Endpoint, accountId })
: []

return { siteInfo, accounts: [], addons: [], integrations }
}

const promises = [
getSite(api, siteId, siteFeatureFlagPrefix),
getAccounts(api),
getAddons(api, siteId),
getIntegrations({ siteId, testOpts, offline, useV2Endpoint, accountId }),
]

const [siteInfo, accounts, addons, integrations] = await Promise.all(promises)

if (siteInfo.use_envelope) {
const envelope = await getEnvelope({ api, accountId: siteInfo.account_slug, siteId, context })

siteInfo.build_settings.env = envelope
}

return { siteInfo, accounts, addons, integrations }
}

if (api === undefined || mode === 'buildbot' || testEnv) {
const siteInfo: { id?: string; account_id?: string } = {}

if (siteId !== undefined) siteInfo.id = siteId
if (accountId !== undefined) siteInfo.account_id = accountId

const integrations = mode === 'buildbot' && !offline ? await getIntegrations({ siteId, testOpts, offline }) : []
const integrations =
mode === 'buildbot' && !offline ? await getIntegrations({ siteId, testOpts, offline, accountId }) : []

return { siteInfo, accounts: [], addons: [], integrations }
}
Expand All @@ -90,7 +55,7 @@ export const getSiteInfo = async function ({
getSite(api, siteId, siteFeatureFlagPrefix),
getAccounts(api),
getAddons(api, siteId),
getIntegrations({ siteId, testOpts, offline }),
getIntegrations({ siteId, testOpts, offline, accountId }),
]

const [siteInfo, accounts, addons, integrations] = await Promise.all(promises)
Expand Down Expand Up @@ -144,15 +109,13 @@ type GetIntegrationsOpts = {
accountId?: string
testOpts: TestOptions
offline: boolean
useV2Endpoint?: boolean
}

const getIntegrations = async function ({
siteId,
accountId,
testOpts,
offline,
useV2Endpoint,
}: GetIntegrationsOpts): Promise<IntegrationResponse[]> {
if (!siteId || offline) {
return []
Expand All @@ -162,8 +125,8 @@ const getIntegrations = async function ({

const baseUrl = new URL(host ? `http://${host}` : `https://api.netlifysdk.com`)

// use future state feature flag
const url = useV2Endpoint
// if accountId isn't present, use safe v1 endpoint
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this for backwards compatibility? I was expecting us to fully switch over to the new endpoint without the feature flag, but is it the case that there might still something still calling this code without an account id once this code goes out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly this is just because I don't know if there would ever be a case where accountId isn't present. 😎 The config package has a few mysterious optional types on things like accountId where I'd anticipate them to always be present, but there's no comments around why they were marked as optional.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fair enough. I was maybe being a little greedy thinking that we might even be able to get rid of the whole api endpoint for ${baseUrl}site/${siteId}/integrations/safe in the other system. No change needed here, I was just learning.

const url = accountId
? `${baseUrl}team/${accountId}/integrations/installations/meta/${siteId}`
: `${baseUrl}site/${siteId}/integrations/safe`

Expand Down
14 changes: 11 additions & 3 deletions packages/config/src/case.js → packages/config/src/case.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
// Some properties can be optionally capitalized. We normalize them to lowercase
export const normalizeConfigCase = function ({ Build, build = Build, ...config }) {
export const normalizeConfigCase = function ({
Build,
build = Build,
...config
}: {
Build: Record<string, unknown>
build: Record<string, unknown>
[key: string]: unknown
}): Record<string, unknown> {
const buildA = normalizeBuildCase(build)
return { ...config, build: buildA }
}

const normalizeBuildCase = function ({
const normalizeBuildCase = ({
Base,
base = Base,
Command,
Expand All @@ -22,7 +30,7 @@ const normalizeBuildCase = function ({
Publish,
publish = Publish,
...build
} = {}) {
}: Record<string, unknown> = {}): Record<string, unknown> => {
return {
...build,
base,
Expand Down
96 changes: 39 additions & 57 deletions packages/config/tests/api/tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const SITE_INTEGRATIONS_RESPONSE = {
response: [
{
slug: 'test',
version: 'so-cool',
version: 'so-cool-v1',
has_build: true,
},
],
Expand All @@ -31,7 +31,7 @@ const TEAM_INSTALLATIONS_META_RESPONSE = {
response: [
{
slug: 'test',
version: 'so-cool',
version: 'so-cool-v2',
has_build: true,
},
],
Expand All @@ -42,6 +42,11 @@ const SITE_INTEGRATIONS_EMPTY_RESPONSE = {
response: [],
}

const TEAM_INSTALLATIONS_META_EMPTY_RESPONSE = {
path: '/team/account1/integrations/installations/meta/test',
response: [],
}

const SITE_INFO_BUILD_SETTINGS = {
path: SITE_INFO_PATH,
response: {
Expand Down Expand Up @@ -222,7 +227,7 @@ test('Build settings are not used in CI', async (t) => {
t.snapshot(normalizeOutput(output))
})

test('Integrations are returned if feature flag is true', async (t) => {
test('Integrations are returned from getSiteInfo from v1 safe API when there is not accountID', async (t) => {
const { output } = await new Fixture('./fixtures/base')
.withFlags({
token: 'test',
Expand All @@ -235,7 +240,7 @@ test('Integrations are returned if feature flag is true', async (t) => {
t.assert(config.integrations)
t.assert(config.integrations.length === 1)
t.assert(config.integrations[0].slug === 'test')
t.assert(config.integrations[0].version === 'so-cool')
t.assert(config.integrations[0].version === 'so-cool-v1')
t.assert(config.integrations[0].has_build === true)
})

Expand All @@ -244,8 +249,9 @@ test('Integration specified in config is also returned if integration is availab
.withFlags({
token: 'test',
siteId: 'test',
accountId: 'account1',
})
.runConfigServer([SITE_INFO_DATA, SITE_INTEGRATIONS_RESPONSE, FETCH_INTEGRATIONS_RESPONSE])
.runConfigServer([SITE_INFO_DATA, TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_RESPONSE])

const config = JSON.parse(output)

Expand All @@ -262,8 +268,9 @@ test('Integration specified in config is not returned if integration is not avai
.withFlags({
token: 'test',
siteId: 'test',
accountId: 'account1',
})
.runConfigServer([SITE_INFO_DATA, SITE_INTEGRATIONS_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])
.runConfigServer([SITE_INFO_DATA, TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])

const config = JSON.parse(output)

Expand All @@ -278,8 +285,9 @@ test('In integration dev mode, integration specified in config is returned even
token: 'test',
siteId: 'test',
context: 'dev',
accountId: 'account1',
})
.runConfigServer([SITE_INFO_DATA, SITE_INTEGRATIONS_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])
.runConfigServer([SITE_INFO_DATA, TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])

const config = JSON.parse(output)

Expand All @@ -297,8 +305,9 @@ test('In integration dev mode, integration specified in config is returned even
token: 'test',
siteId: 'test',
context: 'dev',
accountId: 'account1',
})
.runConfigServer([SITE_INFO_DATA, SITE_INTEGRATIONS_EMPTY_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])
.runConfigServer([SITE_INFO_DATA, TEAM_INSTALLATIONS_META_EMPTY_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])

const config = JSON.parse(output)

Expand All @@ -309,8 +318,8 @@ test('In integration dev mode, integration specified in config is returned even
t.assert(config.integrations[0].version === undefined)
})

test('In integration dev mode, integration specified in config is returned and build is forced by config', async (t) => {
const { output } = await new Fixture('./fixtures/dev_integration_with_force_build')
test('In integration dev mode, integration specified in config is returned even if integration is not enabled on site and accountId not present', async (t) => {
const { output } = await new Fixture('./fixtures/dev_integration')
.withFlags({
token: 'test',
siteId: 'test',
Expand All @@ -323,76 +332,52 @@ test('In integration dev mode, integration specified in config is returned and b
t.assert(config.integrations)
t.assert(config.integrations.length === 1)
t.assert(config.integrations[0].slug === 'abc-integration')
t.assert(config.integrations[0].has_build === true)
t.assert(config.integrations[0].has_build === false)
t.assert(config.integrations[0].version === undefined)
})

test('Integrations are not returned if offline', async (t) => {
const { output } = await new Fixture('./fixtures/base')
test('In integration dev mode, integration specified in config is returned and build is forced by config', async (t) => {
const { output } = await new Fixture('./fixtures/dev_integration_with_force_build')
.withFlags({
offline: true,
token: 'test',
siteId: 'test',
mode: 'buildbot',
context: 'dev',
accountId: 'account1',
})
.runConfigServer([SITE_INTEGRATIONS_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])
.runConfigServer([SITE_INFO_DATA, TEAM_INSTALLATIONS_META_EMPTY_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])

const config = JSON.parse(output)

t.assert(config.integrations)
t.assert(config.integrations.length === 0)
t.assert(config.integrations.length === 1)
t.assert(config.integrations[0].slug === 'abc-integration')
t.assert(config.integrations[0].has_build === true)
t.assert(config.integrations[0].version === undefined)
})

test('Integrations and account id are returned if feature flag is false and mode is buildbot', async (t) => {
test('Integrations are not returned if offline', async (t) => {
const { output } = await new Fixture('./fixtures/base')
.withFlags({
offline: true,
siteId: 'test',
mode: 'buildbot',
accountId: 'account1',
token: 'test',
})
.runConfigServer([SITE_INFO_DATA, SITE_INTEGRATIONS_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])
.runConfigServer([TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])

const config = JSON.parse(output)

t.assert(config.integrations)
t.is(config.integrations.length, 1)
t.is(config.integrations[0].slug, 'test')
t.is(config.integrations[0].version, 'so-cool')
t.is(config.integrations[0].has_build, true)

// account id is also available
t.assert(config.siteInfo)
t.is(config.siteInfo.account_id, 'account1')
})

test('Integrations are returned if feature flag is false and mode is dev', async (t) => {
const { output } = await new Fixture('./fixtures/base')
.withFlags({
siteId: 'test',
mode: 'dev',
token: 'test',
})
.runConfigServer([SITE_INFO_DATA, SITE_INTEGRATIONS_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])

const config = JSON.parse(output)

t.assert(config.integrations)
t.assert(config.integrations.length === 1)
t.assert(config.integrations[0].slug === 'test')
t.assert(config.integrations[0].version === 'so-cool')
t.assert(config.integrations[0].has_build === true)
t.assert(config.integrations.length === 0)
})

test('Integrations and account id are returned if flag is true for site and mode is buildbot', async (t) => {
test('Integrations and account id are returned if mode is buildbot', async (t) => {
const { output } = await new Fixture('./fixtures/base')
.withFlags({
siteId: 'test',
mode: 'buildbot',
token: 'test',
accountId: 'account1',
featureFlags: {
cli_integration_installations_meta: true,
},
token: 'test',
})
.runConfigServer([SITE_INFO_DATA, TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])

Expand All @@ -401,24 +386,21 @@ test('Integrations and account id are returned if flag is true for site and mode
t.assert(config.integrations)
t.is(config.integrations.length, 1)
t.is(config.integrations[0].slug, 'test')
t.is(config.integrations[0].version, 'so-cool')
t.is(config.integrations[0].version, 'so-cool-v2')
t.is(config.integrations[0].has_build, true)

// account id is also available
t.assert(config.siteInfo)
t.is(config.siteInfo.account_id, 'account1')
})

test('Integrations are returned if flag is true for site and mode is dev', async (t) => {
test('Integrations are returned if accountId is present and mode is dev', async (t) => {
const { output } = await new Fixture('./fixtures/base')
.withFlags({
siteId: 'test',
mode: 'dev',
token: 'test',
accountId: 'account1',
featureFlags: {
cli_integration_installations_meta: true,
},
})
.runConfigServer([SITE_INFO_DATA, TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])

Expand All @@ -427,7 +409,7 @@ test('Integrations are returned if flag is true for site and mode is dev', async
t.assert(config.integrations)
t.assert(config.integrations.length === 1)
t.assert(config.integrations[0].slug === 'test')
t.assert(config.integrations[0].version === 'so-cool')
t.assert(config.integrations[0].version === 'so-cool-v2')
t.assert(config.integrations[0].has_build === true)
})

Expand Down
Loading