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: display warning and prevent creation on limit #2526

Merged
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
1398479
Return limits and maxs in the API response, validate disable button, …
novakzaballa Jul 25, 2023
d92fa99
Add validation for limit alerts
novakzaballa Jul 26, 2023
273a07b
Update serializers
novakzaballa Jul 26, 2023
03c8642
Add Test
novakzaballa Jul 26, 2023
cbc4d49
Update serializers
novakzaballa Jul 26, 2023
d00a307
Merge branch 'main' into feature/display-warning-and-prevent-creation…
novakzaballa Jul 27, 2023
244f2c4
Add alert_warning design system, add read_only_fields
novakzaballa Jul 27, 2023
420a7f4
Use annotate for calculate totals, move and rename test
novakzaballa Aug 1, 2023
33004fa
Update env view
novakzaballa Aug 2, 2023
71eecfa
Update limit alert
novakzaballa Aug 3, 2023
3bd32ac
Add alert toast
novakzaballa Aug 3, 2023
c4b4397
Merge branch 'main' into feature/display-warning-and-prevent-creation…
novakzaballa Aug 4, 2023
c5522c2
Update toast text
novakzaballa Aug 4, 2023
b4b69cf
Add types to displayToastAlert function
novakzaballa Aug 4, 2023
480733e
Merge branch 'main' into feature/display-warning-and-prevent-creation…
novakzaballa Aug 4, 2023
5cd3658
Delete alertLimit component
novakzaballa Aug 4, 2023
6188849
exclude deleted rows in totals
novakzaballa Aug 4, 2023
a9eaa10
Delete alert toast
novakzaballa Aug 4, 2023
68c2881
add WarningMessage, Warning icon and styles
novakzaballa Aug 6, 2023
417d301
Use WarningMessage
novakzaballa Aug 6, 2023
96511af
Merge branch 'main' into feature/display-warning-and-prevent-creation…
novakzaballa Aug 7, 2023
9a2ec99
Add types, add Number.MAX_SAFE_INTEGER, change warningMessage color
novakzaballa Aug 7, 2023
72ab33e
Change Error message
novakzaballa Aug 7, 2023
fef7879
Update segmentOverrides
novakzaballa Aug 7, 2023
5f6cfb3
Number.MAX_SAFE_INTEGER deleted
novakzaballa Aug 7, 2023
01695e4
Update segmentOverrides
novakzaballa Aug 7, 2023
772d6da
Bug with total segment overrides fixed
novakzaballa Aug 8, 2023
46a5fa3
bug with select fixed and "create feature-specific segment" button is…
novakzaballa Aug 8, 2023
264290d
Disable select when the limit is reached
novakzaballa Aug 8, 2023
95f36f5
Adding default fallback values to limits and counts for backwards com…
novakzaballa Aug 8, 2023
e8d9b3f
Merge branch 'main' into feature/display-warning-and-prevent-creation…
matthewelwell Aug 9, 2023
d656954
Revert API changes
matthewelwell Aug 9, 2023
210b0d7
merge main
novakzaballa Aug 24, 2023
c6836cd
solve conflicts
novakzaballa Aug 25, 2023
5b6f3bb
Reset changelog
matthewelwell Aug 31, 2023
b8c5a18
Reset changelog
matthewelwell Aug 31, 2023
6270848
Merge branch 'main' into feature/display-warning-and-prevent-creation…
matthewelwell Aug 31, 2023
323e153
fix: Added a defualt value for threshold in calculateRemainingLimitsP…
novakzaballa Aug 31, 2023
e8f463d
fix: change percentage from 30 to 70
novakzaballa Aug 31, 2023
5171523
Get the value of maxApiCalls from "get-subscription-metadata" endpoint
novakzaballa Sep 7, 2023
3b7c45f
Wording / spacing changes
matthewelwell Sep 11, 2023
98fbeea
Small tweak to wording / spacing
matthewelwell Sep 11, 2023
3a6f086
Spacing
matthewelwell Sep 11, 2023
75558d4
Remove upgrade icon
matthewelwell Sep 11, 2023
62d2a4b
Merge branch 'main' into feature/display-warning-and-prevent-creation…
novakzaballa Sep 12, 2023
6d30b9d
feat: Change max api calls alert banner
novakzaballa Sep 12, 2023
16b7f8b
Migrate changes to RTK
kyle-ssg Sep 13, 2023
6586b7d
fix: Add SegmentOverridesLimit
novakzaballa Sep 13, 2023
c9c2ace
Fix: Delete unnecessary code
novakzaballa Sep 14, 2023
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
3 changes: 2 additions & 1 deletion .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
*.json
*.handlebars
*.css
.bablerc
.bablerc
**/CHANGELOG.md
6 changes: 6 additions & 0 deletions frontend/common/dispatcher/app-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ const AppActions = Object.assign({}, require('./base/_app-actions'), {
projectId,
})
},
getEnv(environmentId) {
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why we need to get the environment this way? It adds to tech debt, ideally it would be an action in useEnvironment.ts

Dispatcher.handleViewAction({
actionType: Actions.GET_ENVIRONMENT,
environmentId,
})
},
createFlag(projectId, environmentId, flag, segmentOverrides) {
Dispatcher.handleViewAction({
actionType: Actions.CREATE_FLAG,
Expand Down
6 changes: 5 additions & 1 deletion frontend/common/providers/FeatureListProvider.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react'
import FeatureListStore from 'common/stores/feature-list-store'
import ProjectStore from 'common/stores/project-store'

const FeatureListProvider = class extends React.Component {
static displayName = 'FeatureListProvider'
Expand All @@ -11,8 +12,9 @@ const FeatureListProvider = class extends React.Component {
isLoading: FeatureListStore.isLoading,
isSaving: FeatureListStore.isSaving,
lastSaved: FeatureListStore.getLastSaved(),
maxFeaturesAllowed: ProjectStore.getMaxFeaturesAllowed(),
projectFlags: FeatureListStore.getProjectFlags(),
usageData: FeatureListStore.getFeatureUsage(),
totalFeatures: ProjectStore.getTotalFeatures(),
}
ES6Component(this)
this.listenTo(FeatureListStore, 'change', () => {
Expand All @@ -22,7 +24,9 @@ const FeatureListProvider = class extends React.Component {
isLoading: FeatureListStore.isLoading,
isSaving: FeatureListStore.isSaving,
lastSaved: FeatureListStore.getLastSaved(),
maxFeaturesAllowed: ProjectStore.getMaxFeaturesAllowed(),
projectFlags: FeatureListStore.getProjectFlags(),
totalFeatures: ProjectStore.getTotalFeatures(),
usageData: FeatureListStore.getFeatureUsage(),
})
})
Expand Down
49 changes: 49 additions & 0 deletions frontend/common/services/useSubscriptionMetadata.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { Res } from 'common/types/responses'
import { Req } from 'common/types/requests'
import { service } from 'common/service'

export const getSubscriptionMetadataService = service
.enhanceEndpoints({ addTagTypes: ['GetSubscriptionMetadata'] })
.injectEndpoints({
endpoints: (builder) => ({
getSubscriptionMetadata: builder.query<
Res['getSubscriptionMetadata'],
Req['getSubscriptionMetadata']
>({
providesTags: (res) => [
{ id: res?.id, type: 'GetSubscriptionMetadata' },
],
query: (query: Req['getSubscriptionMetadata']) => ({
url: `organisations/${query.id}/get-subscription-metadata/`,
}),
}),
// END OF ENDPOINTS
}),
})

export async function getSubscriptionMetadata(
store: any,
data: Req['getSubscriptionMetadata'],
options?: Parameters<
typeof getSubscriptionMetadataService.endpoints.getSubscriptionMetadata.initiate
>[1],
) {
return store.dispatch(
getSubscriptionMetadataService.endpoints.getSubscriptionMetadata.initiate(
data,
options,
),
)
}
// END OF FUNCTION_EXPORTS

export const {
useGetSubscriptionMetadataQuery,
// END OF EXPORTS
} = getSubscriptionMetadataService

/* Usage examples:
const { data, isLoading } = useGetSubscriptionMetadataQuery({ id: 2 }, {}) //get hook
const [getSubscriptionMetadata, { isLoading, data, isSuccess }] = useGetSubscriptionMetadataMutation() //create hook
getSubscriptionMetadataService.endpoints.getSubscriptionMetadata.select({id: 2})(store.getState()) //access data from any function
*/
42 changes: 42 additions & 0 deletions frontend/common/stores/project-store.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { getIsWidget } from 'components/pages/WidgetPage'

import Constants from 'common/constants'
import Utils from 'common/utils/utils'

const Dispatcher = require('../dispatcher/dispatcher')
const BaseStore = require('./base/_store')
Expand Down Expand Up @@ -83,6 +84,14 @@ const controller = {
store.saved()
})
},
getEnv: (envId) => {
data.get(`${Project.api}environments/${envId}/`).then((environment) => {
Copy link
Member

Choose a reason for hiding this comment

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

Is this used anymore?

environment.total_segment_overrides =
environment.total_segment_overrides || 0
store.model = Object.assign(store.model, { environment })
store.saved()
})
},
getProject: (id, cb, force) => {
if (force) {
store.loading()
Expand All @@ -92,6 +101,12 @@ const controller = {
data.get(`${Project.api}environments/?project=${id}`).catch(() => []),
])
.then(([project, environments]) => {
project.max_segments_allowed = project.max_segments_allowed
project.max_features_allowed = project.max_features_allowed
project.max_segment_overrides_allowed =
project.max_segment_overrides_allowed
project.total_features = project.total_features || 0
project.total_segments = project.total_segments || 0
store.model = Object.assign(project, {
environments: _.sortBy(environments.results, 'name'),
})
Expand All @@ -118,6 +133,12 @@ const controller = {
data.get(`${Project.api}environments/?project=${id}`).catch(() => []),
])
.then(([project, environments]) => {
project.max_segments_allowed = project.max_segments_allowed
project.max_features_allowed = project.max_features_allowed
project.max_segment_overrides_allowed =
project.max_segment_overrides_allowed
project.total_features = project.total_features || 0
project.total_segments = project.total_segments || 0
store.model = Object.assign(project, {
environments: _.sortBy(environments.results, 'name'),
})
Expand Down Expand Up @@ -162,6 +183,24 @@ const store = Object.assign({}, BaseStore, {
})
},
getEnvs: () => store.model && store.model.environments,
getMaxFeaturesAllowed: () => {
return store.model && store.model.max_features_allowed
},
getMaxSegmentOverridesAllowed: () => {
return store.model && store.model.max_segment_overrides_allowed
},
getMaxSegmentsAllowed: () => {
return store.model && store.model.max_segments_allowed
},
getTotalFeatures: () => {
return store.model && store.model.total_features
},
getTotalSegmentOverrides: () => {
return store.model && store.model.environment.total_segment_overrides
},
getTotalSegments: () => {
return store.model && store.model.total_segments
},
id: 'project',
model: null,
})
Expand All @@ -187,6 +226,9 @@ store.dispatcherIndex = Dispatcher.register(store, (payload) => {
case Actions.EDIT_ENVIRONMENT:
controller.editEnv(action.env)
break
case Actions.GET_ENVIRONMENT:
controller.getEnv(action.environmentId)
break
case Actions.DELETE_ENVIRONMENT:
controller.deleteEnv(action.env)
break
Expand Down
1 change: 1 addition & 0 deletions frontend/common/types/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,5 +105,6 @@ export type Req = {
user: string
}
getProjectFlags: { project: string }
getGetSubscriptionMetadata: { id: string }
// END OF TYPES
}
7 changes: 7 additions & 0 deletions frontend/common/types/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export type Environment = {
minimum_change_request_approvals?: number
allow_client_traits: boolean
hide_sensitive_data: boolean
total_segment_overrides?: number
}
export type Project = {
id: number
Expand All @@ -62,6 +63,11 @@ export type Project = {
use_edge_identities: boolean
prevent_flag_defaults: boolean
enable_realtime_updates: boolean
max_segments_allowed?: number | null
max_features_allowed?: number | null
max_segment_overrides_allowed?: number | null
total_features?: number
total_segments?: number
environments: Environment[]
}

Expand Down Expand Up @@ -336,5 +342,6 @@ export type Res = {

projectFlags: PagedResponse<ProjectFlag>
identityFeatureStates: IdentityFeatureState[]
getSubscriptionMetadata: { id: string }
// END OF TYPES
}
37 changes: 27 additions & 10 deletions frontend/common/utils/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
import flagsmith from 'flagsmith'
import { ReactNode } from 'react'
import _ from 'lodash'
import ErrorMessage from 'components/ErrorMessage'
import WarningMessage from 'components/WarningMessage'
import Constants from 'common/constants'

const semver = require('semver')
Expand Down Expand Up @@ -54,25 +56,40 @@ const Utils = Object.assign({}, require('./base/_utils'), {
return 100 - total
},

calculaterRemainingCallsPercentage(value, total) {
const minRemainingPercentage = 30
calculateRemainingLimitsPercentage(
total: number | undefined,
max: number | undefined,
threshold = 90,
) {
if (total === 0) {
return 0
}

const percentage = (value / total) * 100
const remainingPercentage = 100 - percentage

if (remainingPercentage <= minRemainingPercentage) {
return true
const percentage = (total / max) * 100
if (percentage >= threshold) {
return {
percentage: Math.floor(percentage),
}
}
return false
return 0
},

displayLimitAlert(type: string, percentage: number | undefined) {
const envOrProject =
type === 'segment overrides' ? 'environment' : 'project'
return percentage >= 100 ? (
<ErrorMessage
error={`Your ${envOrProject} reached the limit of ${type}, please contact support to discuss increasing this limit.`}
/>
) : percentage ? (
<WarningMessage
warningMessage={`Your ${envOrProject} is using ${percentage}% of the total allowance of ${type}.`}
/>
) : null
},

changeRequestsEnabled(value: number | null | undefined) {
return typeof value === 'number'
},

escapeHtml(html: string) {
const text = document.createTextNode(html)
const p = document.createElement('p')
Expand Down
50 changes: 49 additions & 1 deletion frontend/web/components/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@ import { resolveAuthFlow } from '@datadog/ui-extensions-sdk'
import ConfigProvider from 'common/providers/ConfigProvider'
import Permission from 'common/providers/Permission'
import { getOrganisationUsage } from 'common/services/useOrganisationUsage'
import { getSubscriptionMetadata } from 'common/services/useSubscriptionMetadata'
import Button from './base/forms/Button'
import Icon from 'components/Icon'
import Icon from './Icon'
import AccountStore from 'common/stores/account-store'
import InfoMessage from './InfoMessage'
import Format from 'common/utils/format'
import ErrorMessage from 'components/ErrorMessage'
import WarningMessage from 'components/WarningMessage'

const App = class extends Component {
static propTypes = {
Expand All @@ -38,6 +42,7 @@ const App = class extends Component {
asideIsVisible: !isMobile,
lastEnvironmentId: '',
lastProjectId: '',
maxApiCalls: 50000,
pin: '',
showAnnouncement: true,
totalApiCalls: 0,
Expand Down Expand Up @@ -70,6 +75,13 @@ const App = class extends Component {
AccountStore.getOrganisation()?.id &&
this.state.activeOrganisation !== AccountStore.getOrganisation().id
) {
getSubscriptionMetadata(getStore(), {
Copy link
Member

Choose a reason for hiding this comment

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

Is this used anymore

id: AccountStore.getOrganisation()?.id,
}).then((res) => {
this.setState({
maxApiCalls: res?.data?.max_api_calls,
})
})
getOrganisationUsage(getStore(), {
organisationId: AccountStore.getOrganisation()?.id,
}).then((res) => {
Expand Down Expand Up @@ -288,6 +300,17 @@ const App = class extends Component {
)
const dismissed = flagsmith.getTrait('dismissed_announcement')
const showBanner = !dismissed || dismissed !== announcementValue.id
const maxApiCallsPercentage = Utils.calculateRemainingLimitsPercentage(
Copy link
Member

Choose a reason for hiding this comment

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

Is this used anymore

this.state.totalApiCalls,
this.state.maxApiCalls,
70,
).percentage

const alertMaxApiCallsText = `You have used ${Format.shortenNumber(
this.state.totalApiCalls,
)}/${Format.shortenNumber(
this.state.maxApiCalls,
)} of your allowed requests.`

return (
<Provider store={getStore()}>
Expand Down Expand Up @@ -484,6 +507,31 @@ const App = class extends Component {
</div>
) : (
<Fragment>
<Row>
{user &&
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned on slack, I think this should be componentised so that you can use hooks to get the data.

Utils.getFlagsmithHasFeature(
'payments_enabled',
) &&
Utils.getFlagsmithHasFeature(
'max_api_calls_alert',
) &&
(maxApiCallsPercentage &&
maxApiCallsPercentage < 100 ? (
<WarningMessage
warningMessage={alertMaxApiCallsText}
warningMessageClass={'announcement'}
enabledButton
/>
) : (
maxApiCallsPercentage >= 100 && (
<ErrorMessage
error={alertMaxApiCallsText}
errorMessageClass={'announcement'}
enabledButton
/>
)
))}
</Row>
{user &&
showBanner &&
Utils.getFlagsmithHasFeature('announcement') &&
Expand Down
2 changes: 2 additions & 0 deletions frontend/web/components/Aside.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const Aside = class extends Component {
? {}
: { live_from_after: new Date().toISOString() },
)
AppActions.getEnv(this.props.environmentId)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to do this

}
})
}
Expand Down Expand Up @@ -77,6 +78,7 @@ const Aside = class extends Component {
? {}
: { live_from_after: new Date().toISOString() },
)
AppActions.getEnv(this.props.environmentId)
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused by this, we're already getting the project's environments in AppActions.getProject

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is triggered every time we change environments. It is done this way because we need to save the response to the store so we can use its data to determine the content of the alert and whether it will be displayed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use the environment endpoint because the project one doesn't have the limits information we need, and Matt asked to keep it this way since it will be more efficient in terms of querying the DB

Copy link
Member

Choose a reason for hiding this comment

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

After our conversation, I think this can now be removed.

}
}
}
Expand Down
Loading