From b67bdf79f426737098297e9d5733e9e12012d493 Mon Sep 17 00:00:00 2001 From: Maxim Mordasov Date: Wed, 20 Sep 2023 14:34:57 +0300 Subject: [PATCH 01/14] Build 'When I am on-call' for web UI (#3034) # What this PR does Build 'When I am on-call' for web UI ## Which issue(s) this PR fixes https://github.com/grafana/oncall/issues/2915 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- .../ScheduleUserDetails.tsx | 2 +- .../containers/Rotation/Rotation.module.css | 1 + .../src/containers/Rotation/Rotation.tsx | 45 +++++- .../containers/RotationForm/ShiftSwapForm.tsx | 18 +-- .../src/containers/Rotations/Rotations.tsx | 2 - .../containers/Rotations/ScheduleFinal.tsx | 21 +-- .../Rotations/ScheduleOverrides.tsx | 3 - .../containers/Rotations/SchedulePersonal.tsx | 143 ++++++++++++++++++ .../containers/ScheduleSlot/ScheduleSlot.tsx | 125 ++++++++------- .../src/models/schedule/schedule.helpers.ts | 28 ++++ .../src/models/schedule/schedule.ts | 58 ++++++- .../src/models/schedule/schedule.types.ts | 4 +- grafana-plugin/src/models/user/user.types.ts | 2 + grafana-plugin/src/pages/index.tsx | 1 + .../src/pages/schedules/Schedules.module.css | 10 +- .../src/pages/schedules/Schedules.tsx | 86 ++++++----- 16 files changed, 403 insertions(+), 146 deletions(-) create mode 100644 grafana-plugin/src/containers/Rotations/SchedulePersonal.tsx diff --git a/grafana-plugin/src/components/ScheduleUserDetails/ScheduleUserDetails.tsx b/grafana-plugin/src/components/ScheduleUserDetails/ScheduleUserDetails.tsx index 759760c4..46786eb2 100644 --- a/grafana-plugin/src/components/ScheduleUserDetails/ScheduleUserDetails.tsx +++ b/grafana-plugin/src/components/ScheduleUserDetails/ScheduleUserDetails.tsx @@ -56,7 +56,7 @@ const ScheduleUserDetails: FC = (props) => { {user.username} - {isOncall && } + {isOncall && } {isInWH ? ( ) : ( diff --git a/grafana-plugin/src/containers/Rotation/Rotation.module.css b/grafana-plugin/src/containers/Rotation/Rotation.module.css index 805ef3c8..87cb524c 100644 --- a/grafana-plugin/src/containers/Rotation/Rotation.module.css +++ b/grafana-plugin/src/containers/Rotation/Rotation.module.css @@ -56,6 +56,7 @@ .empty { height: 28px; cursor: pointer; + text-align: center; /* background: #5f505633; border: 1px dashed #5c474d; diff --git a/grafana-plugin/src/containers/Rotation/Rotation.tsx b/grafana-plugin/src/containers/Rotation/Rotation.tsx index 486c988f..97156cc1 100644 --- a/grafana-plugin/src/containers/Rotation/Rotation.tsx +++ b/grafana-plugin/src/containers/Rotation/Rotation.tsx @@ -6,8 +6,9 @@ import dayjs from 'dayjs'; import hash from 'object-hash'; import { ScheduleFiltersType } from 'components/ScheduleFilters/ScheduleFilters.types'; +import Text from 'components/Text/Text'; import ScheduleSlot from 'containers/ScheduleSlot/ScheduleSlot'; -import { Schedule, Event, RotationFormLiveParams, Shift, ShiftSwap } from 'models/schedule/schedule.types'; +import { Event, RotationFormLiveParams, Shift, ShiftSwap } from 'models/schedule/schedule.types'; import { Timezone } from 'models/timezone/timezone.types'; import RotationTutorial from './RotationTutorial'; @@ -17,7 +18,6 @@ import styles from './Rotation.module.css'; const cx = cn.bind(styles); interface RotationProps { - scheduleId: Schedule['id']; startMoment: dayjs.Dayjs; currentTimezone: Timezone; layerIndex?: number; @@ -27,6 +27,7 @@ interface RotationProps { onClick?: (start: dayjs.Dayjs, end: dayjs.Dayjs) => void; handleAddOverride?: (start: dayjs.Dayjs, end: dayjs.Dayjs) => void; handleAddShiftSwap?: (id: 'new', params: Partial) => void; + handleOpenSchedule?: (event: Event) => void; onShiftSwapClick?: (swapId: ShiftSwap['id']) => void; days?: number; transparent?: boolean; @@ -35,12 +36,13 @@ interface RotationProps { filters?: ScheduleFiltersType; getColor?: (shiftId: Shift['id']) => string; onSlotClick?: (event: Event) => void; + emptyText?: string; + showScheduleNameAsSlotTitle?: boolean; } const Rotation: FC = (props) => { const { events, - scheduleId, startMoment, currentTimezone, color: propsColor, @@ -50,11 +52,14 @@ const Rotation: FC = (props) => { onClick, handleAddOverride, handleAddShiftSwap, + handleOpenSchedule, onShiftSwapClick, simplified, filters, getColor, onSlotClick, + emptyText, + showScheduleNameAsSlotTitle, } = props; const [animate, _setAnimate] = useState(true); @@ -73,6 +78,10 @@ const Rotation: FC = (props) => { }; const getAddOverrideClickHandler = (scheduleEvent: Event) => { + if (simplified) { + return undefined; + } + return (event: React.MouseEvent) => { event.stopPropagation(); @@ -81,6 +90,10 @@ const Rotation: FC = (props) => { }; const getAddShiftSwapClickHandler = (scheduleEvent: Event) => { + if (simplified) { + return undefined; + } + return (event: React.MouseEvent) => { event.stopPropagation(); @@ -91,6 +104,18 @@ const Rotation: FC = (props) => { }; }; + const getOpenScheduleClickHandler = (scheduleEvent: Event) => { + if (!handleOpenSchedule) { + return undefined; + } + + return (event: React.MouseEvent) => { + event.stopPropagation(); + + handleOpenSchedule(scheduleEvent); + }; + }; + const getSlotClickHandler = (event: Event) => { if (!onSlotClick) { return undefined; @@ -127,7 +152,6 @@ const Rotation: FC = (props) => { {events.map((event) => { return ( = (props) => { color={propsColor || getColor(event.shift?.pk)} handleAddOverride={getAddOverrideClickHandler(event)} handleAddShiftSwap={getAddShiftSwapClickHandler(event)} + handleOpenSchedule={getOpenScheduleClickHandler(event)} onShiftSwapClick={onShiftSwapClick} - simplified={simplified} filters={filters} onClick={getSlotClickHandler(event)} + showScheduleNameAsSlotTitle={showScheduleNameAsSlotTitle} /> ); })} ) : ( - + ) ) : ( @@ -156,8 +181,12 @@ const Rotation: FC = (props) => { ); }; -const Empty = () => { - return
; +const Empty = ({ text }: { text: string }) => { + return ( +
+ {text} +
+ ); }; export default Rotation; diff --git a/grafana-plugin/src/containers/RotationForm/ShiftSwapForm.tsx b/grafana-plugin/src/containers/RotationForm/ShiftSwapForm.tsx index f750ccae..cbe6488b 100644 --- a/grafana-plugin/src/containers/RotationForm/ShiftSwapForm.tsx +++ b/grafana-plugin/src/containers/RotationForm/ShiftSwapForm.tsx @@ -86,7 +86,7 @@ const ShiftSwapForm = (props: ShiftSwapFormProps) => { if (id === 'new') { store.scheduleStore.updateShiftsSwapPreview(scheduleId, startMoment, { id: 'new', - beneficiary: currentUserPk, + beneficiary: { pk: currentUserPk }, ...shiftSwap, }); } @@ -120,13 +120,7 @@ const ShiftSwapForm = (props: ShiftSwapFormProps) => { onUpdate(); }, [id]); - useEffect(() => { - if (shiftSwap?.beneficiary && !store.userStore.items[shiftSwap.beneficiary]) { - store.userStore.updateItem(shiftSwap.beneficiary); - } - }, [shiftSwap?.beneficiary]); - - const beneficiaryName = shiftSwap?.beneficiary && store.userStore.items[shiftSwap.beneficiary]?.name; + const beneficiaryName = shiftSwap?.beneficiary?.display_name; const isNew = id === 'new'; const isPastDue = useMemo(() => shiftSwap && dayjs(shiftSwap.swap_start).isBefore(dayjs()), [shiftSwap]); @@ -159,7 +153,7 @@ const ShiftSwapForm = (props: ShiftSwapFormProps) => { tooltip="Delete" name="trash-alt" onClick={handleDelete} - disabled={shiftSwap.beneficiary !== currentUserPk} + disabled={shiftSwap.beneficiary?.pk !== currentUserPk} /> @@ -204,7 +198,7 @@ const ShiftSwapForm = (props: ShiftSwapFormProps) => { {shiftSwap?.benefactor ? ( { diff --git a/grafana-plugin/src/containers/Rotations/Rotations.tsx b/grafana-plugin/src/containers/Rotations/Rotations.tsx index 85063bed..b39fd4c2 100644 --- a/grafana-plugin/src/containers/Rotations/Rotations.tsx +++ b/grafana-plugin/src/containers/Rotations/Rotations.tsx @@ -167,7 +167,6 @@ class Rotations extends Component { classNames={{ ...styles }} > { this.onRotationClick(shiftId, shiftStart, shiftEnd); }} @@ -206,7 +205,6 @@ class Rotations extends Component {
{ this.handleAddLayer(nextPriority, shiftStart, shiftEnd); }} diff --git a/grafana-plugin/src/containers/Rotations/ScheduleFinal.tsx b/grafana-plugin/src/containers/Rotations/ScheduleFinal.tsx index 9ad69290..d75e6cab 100644 --- a/grafana-plugin/src/containers/Rotations/ScheduleFinal.tsx +++ b/grafana-plugin/src/containers/Rotations/ScheduleFinal.tsx @@ -33,7 +33,6 @@ interface ScheduleFinalProps extends WithStoreProps { currentTimezone: Timezone; scheduleId: Schedule['id']; simplified?: boolean; - onClick: (shiftId: Shift['id']) => void; onShowOverrideForm: (shiftId: 'new', shiftStart: dayjs.Dayjs, shiftEnd: dayjs.Dayjs) => void; onShowShiftSwapForm: (id: ShiftSwap['id'] | 'new', params?: Partial) => void; disabled?: boolean; @@ -41,16 +40,8 @@ interface ScheduleFinalProps extends WithStoreProps { onSlotClick?: (event: Event) => void; } -interface ScheduleOverridesState { - searchTerm: string; -} - @observer -class ScheduleFinal extends Component { - state: ScheduleOverridesState = { - searchTerm: '', - }; - +class ScheduleFinal extends Component { render() { const { startMoment, currentTimezone, store, simplified, scheduleId, filters, onShowShiftSwapForm, onSlotClick } = this.props; @@ -94,7 +85,6 @@ class ScheduleFinal extends Component - + )} @@ -126,8 +111,6 @@ class ScheduleFinal extends Component {}; - handleShowOverrideForm = (shiftStart: dayjs.Dayjs, shiftEnd: dayjs.Dayjs) => { const { onShowOverrideForm } = this.props; diff --git a/grafana-plugin/src/containers/Rotations/ScheduleOverrides.tsx b/grafana-plugin/src/containers/Rotations/ScheduleOverrides.tsx index ab0d11d9..0c22747d 100644 --- a/grafana-plugin/src/containers/Rotations/ScheduleOverrides.tsx +++ b/grafana-plugin/src/containers/Rotations/ScheduleOverrides.tsx @@ -155,7 +155,6 @@ class ScheduleOverrides extends Component ( ( { diff --git a/grafana-plugin/src/containers/Rotations/SchedulePersonal.tsx b/grafana-plugin/src/containers/Rotations/SchedulePersonal.tsx new file mode 100644 index 00000000..ce290d0c --- /dev/null +++ b/grafana-plugin/src/containers/Rotations/SchedulePersonal.tsx @@ -0,0 +1,143 @@ +import React, { Component } from 'react'; + +import { Badge, HorizontalGroup } from '@grafana/ui'; +import cn from 'classnames/bind'; +import dayjs from 'dayjs'; +import { observer } from 'mobx-react'; +import { RouteComponentProps, withRouter } from 'react-router-dom'; +import { CSSTransition, TransitionGroup } from 'react-transition-group'; + +import Avatar from 'components/Avatar/Avatar'; +import Text from 'components/Text/Text'; +import TimelineMarks from 'components/TimelineMarks/TimelineMarks'; +import Rotation from 'containers/Rotation/Rotation'; +import { getColorForSchedule, getPersonalShiftsFromStore } from 'models/schedule/schedule.helpers'; +import { Shift, Event } from 'models/schedule/schedule.types'; +import { Timezone } from 'models/timezone/timezone.types'; +import { User } from 'models/user/user.types'; +import { WithStoreProps } from 'state/types'; +import { withMobXProviderContext } from 'state/withStore'; +import { PLUGIN_ROOT } from 'utils/consts'; + +import { DEFAULT_TRANSITION_TIMEOUT } from './Rotations.config'; + +import styles from './Rotations.module.css'; + +const cx = cn.bind(styles); + +interface SchedulePersonalProps extends WithStoreProps, RouteComponentProps { + startMoment: dayjs.Dayjs; + currentTimezone: Timezone; + userPk: User['pk']; + onSlotClick?: (event: Event) => void; +} + +@observer +class SchedulePersonal extends Component { + componentDidMount() { + const { store, startMoment } = this.props; + + store.scheduleStore.updatePersonalEvents(store.userStore.currentUserPk, startMoment); + } + + componentDidUpdate(prevProps: Readonly): void { + const { store, startMoment } = this.props; + + if (prevProps.startMoment !== this.props.startMoment) { + store.scheduleStore.updatePersonalEvents(store.userStore.currentUserPk, startMoment); + } + } + + render() { + const { userPk, startMoment, currentTimezone, store, onSlotClick } = this.props; + + const base = 7 * 24 * 60; // in minutes + const diff = dayjs().tz(currentTimezone).diff(startMoment, 'minutes'); + + const currentTimeX = diff / base; + + const shifts = getPersonalShiftsFromStore(store, userPk, startMoment); + + const currentTimeHidden = currentTimeX < 0 || currentTimeX > 1; + + const getColor = (shiftId: Shift['id']) => { + const shift = store.scheduleStore.shifts[shiftId]; + + if (!shift) { + if (shiftId) { + store.scheduleStore.updateOncallShift(shiftId); + } + return; + } + + return getColorForSchedule(shift.schedule); + }; + + const isOncall = store.scheduleStore.onCallNow[userPk]; + + const storeUser = store.userStore.items[userPk]; + + return ( + <> +
+
+
+ + + On-call schedule {store.userStore.currentUser.name} + + {/* @ts-ignore */} + {isOncall ? : } + +
+
+
+ {!currentTimeHidden &&
} + + + {shifts && shifts.length ? ( + shifts.map(({ events }, index) => { + return ( + + + + ); + }) + ) : ( + + + + )} + +
+
+ + ); + } + + openSchedule = (event: Event) => { + const { store, history } = this.props; + + const shiftId = event.shift?.pk; + const shift = store.scheduleStore.shifts[shiftId]; + + history.push(`${PLUGIN_ROOT}/schedules/${shift.schedule}`); + }; +} + +export default withRouter(withMobXProviderContext(SchedulePersonal)); diff --git a/grafana-plugin/src/containers/ScheduleSlot/ScheduleSlot.tsx b/grafana-plugin/src/containers/ScheduleSlot/ScheduleSlot.tsx index aeb24a2a..821ed787 100644 --- a/grafana-plugin/src/containers/ScheduleSlot/ScheduleSlot.tsx +++ b/grafana-plugin/src/containers/ScheduleSlot/ScheduleSlot.tsx @@ -10,7 +10,7 @@ import { ScheduleFiltersType } from 'components/ScheduleFilters/ScheduleFilters. import Text from 'components/Text/Text'; import WorkingHours from 'components/WorkingHours/WorkingHours'; import { getShiftName, SHIFT_SWAP_COLOR } from 'models/schedule/schedule.helpers'; -import { Event, Schedule, ShiftSwap } from 'models/schedule/schedule.types'; +import { Event, ShiftSwap } from 'models/schedule/schedule.types'; import { getTzOffsetString } from 'models/timezone/timezone.helpers'; import { Timezone } from 'models/timezone/timezone.types'; import { User } from 'models/user/user.types'; @@ -22,16 +22,16 @@ import styles from './ScheduleSlot.module.css'; interface ScheduleSlotProps { event: Event; - scheduleId: Schedule['id']; startMoment: dayjs.Dayjs; currentTimezone: Timezone; handleAddOverride: (event: React.MouseEvent) => void; handleAddShiftSwap: (event: React.MouseEvent) => void; + handleOpenSchedule: (event: React.MouseEvent) => void; onShiftSwapClick: (id: ShiftSwap['id']) => void; color?: string; - simplified?: boolean; filters?: ScheduleFiltersType; onClick: (event: React.MouseEvent) => void; + showScheduleNameAsSlotTitle?: boolean; } const cx = cn.bind(styles); @@ -39,15 +39,15 @@ const cx = cn.bind(styles); const ScheduleSlot: FC = observer((props) => { const { event, - scheduleId, currentTimezone, color, handleAddOverride, handleAddShiftSwap, + handleOpenSchedule, onShiftSwapClick, - simplified, filters, onClick, + showScheduleNameAsSlotTitle, } = props; const start = dayjs(event.start); @@ -63,14 +63,7 @@ const ScheduleSlot: FC = observer((props) => { const renderEvent = (event): React.ReactElement | React.ReactElement[] => { if (event.shiftSwapId) { - return ( - - ); + return ; } if (event.is_gap) { @@ -95,17 +88,17 @@ const ScheduleSlot: FC = observer((props) => { return ( ); }; @@ -122,39 +115,41 @@ export default ScheduleSlot; interface ShiftSwapEventProps { event: Event; currentTimezone: Timezone; - simplified: boolean; currentMoment: dayjs.Dayjs; } const ShiftSwapEvent = (props: ShiftSwapEventProps) => { - const { event, currentTimezone, simplified, currentMoment } = props; + const { event, currentTimezone, currentMoment } = props; const store = useStore(); const shiftSwap = store.scheduleStore.shiftSwaps[event.shiftSwapId]; + const beneficiary = shiftSwap?.beneficiary; + const benefactor = shiftSwap?.benefactor; + useEffect(() => { - if (shiftSwap?.beneficiary && !store.userStore.items[shiftSwap.beneficiary]) { - store.userStore.updateItem(shiftSwap.beneficiary); + if (shiftSwap?.beneficiary && !store.userStore.items[shiftSwap.beneficiary.pk]) { + store.userStore.updateItem(shiftSwap.beneficiary.pk); } }, [shiftSwap?.beneficiary]); useEffect(() => { - if (shiftSwap?.benefactor && !store.userStore.items[shiftSwap.benefactor]) { - store.userStore.updateItem(shiftSwap.benefactor); + if (shiftSwap?.benefactor && !store.userStore.items[shiftSwap.benefactor.pk]) { + store.userStore.updateItem(shiftSwap.benefactor.pk); } }, [shiftSwap?.benefactor]); - const beneficiary = store.userStore.items[shiftSwap?.beneficiary]; - const benefactor = store.userStore.items[shiftSwap?.benefactor]; + const beneficiaryStoreUser = store.userStore.items[shiftSwap?.beneficiary?.pk]; + const benefactorStoreUser = store.userStore.items[shiftSwap?.benefactor?.pk]; const scheduleSlotContent = (
{shiftSwap && ( - {beneficiary && } + {beneficiary && } {benefactor ? ( - + ) : (
@@ -177,12 +172,11 @@ const ShiftSwapEvent = (props: ShiftSwapEventProps) => { content={ @@ -195,33 +189,33 @@ const ShiftSwapEvent = (props: ShiftSwapEventProps) => { interface RegularEventProps { event: Event; - scheduleId: Schedule['id']; currentTimezone: Timezone; handleAddOverride: (event: React.MouseEvent) => void; handleAddShiftSwap: (event: React.MouseEvent) => void; + handleOpenSchedule: (event: React.MouseEvent) => void; onShiftSwapClick: (id: ShiftSwap['id']) => void; - simplified: boolean; color?: string; filters?: ScheduleFiltersType; start: dayjs.Dayjs; duration: number; currentMoment: dayjs.Dayjs; + showScheduleNameAsSlotTitle: boolean; } const RegularEvent = (props: RegularEventProps) => { const { event, - scheduleId, onShiftSwapClick, filters, color, currentTimezone, - simplified, start, duration, handleAddOverride, handleAddShiftSwap, + handleOpenSchedule, currentMoment, + showScheduleNameAsSlotTitle, } = props; const store = useStore(); @@ -238,10 +232,6 @@ const RegularEvent = (props: RegularEventProps) => { [onShiftSwapClick] ); - const onCallNow = store.scheduleStore.items[scheduleId]?.on_call_now; - - const enableWebOverrides = store.scheduleStore.items[scheduleId]?.enable_web_overrides; - return ( <> {users.map(({ display_name, pk: userPk, swap_request }) => { @@ -250,11 +240,7 @@ const RegularEvent = (props: RegularEventProps) => { const isCurrentUserSlot = userPk === store.userStore.currentUserPk; const inactive = filters && filters.users.length && !filters.users.includes(userPk); - const title = storeUser ? getTitle(storeUser) : display_name; - - const isOncall = Boolean( - storeUser && onCallNow && onCallNow.some((onCallUser) => storeUser.pk === onCallUser.pk) - ); + const userTitle = storeUser ? getTitle(storeUser) : display_name; const isShiftSwap = Boolean(swap_request); @@ -281,7 +267,7 @@ const RegularEvent = (props: RegularEventProps) => { /> )}
- {swap_request && !swap_request.user ? : title} + {swap_request && !swap_request.user ? : userTitle}
); @@ -296,30 +282,26 @@ const RegularEvent = (props: RegularEventProps) => { key={userPk} content={ @@ -340,12 +322,13 @@ interface ScheduleSlotDetailsProps { event: Event; handleAddOverride?: (event: React.SyntheticEvent) => void; handleAddShiftSwap?: (event: React.SyntheticEvent) => void; - simplified?: boolean; + handleOpenSchedule?: (event: React.SyntheticEvent) => void; color: string; isShiftSwap?: boolean; beneficiaryName?: string; benefactorName?: string; currentMoment: dayjs.Dayjs; + showScheduleNameAsSlotTitle?: boolean; } const ScheduleSlotDetails = (props: ScheduleSlotDetailsProps) => { @@ -355,17 +338,40 @@ const ScheduleSlotDetails = (props: ScheduleSlotDetailsProps) => { event, handleAddOverride, handleAddShiftSwap, + handleOpenSchedule, color, isShiftSwap, beneficiaryName, benefactorName, currentMoment, + showScheduleNameAsSlotTitle, } = props; - const store = useStore(); - const { scheduleStore } = store; + const { scheduleStore } = useStore(); - const shift = scheduleStore.shifts[event.shift?.pk]; + const shiftId = event.shift?.pk; + const shift = scheduleStore.shifts[shiftId]; + + const schedule = scheduleStore.items[shift?.schedule]; + + const enableWebOverrides = schedule?.enable_web_overrides; + + useEffect(() => { + if (shiftId && !scheduleStore.shifts[shiftId]) { + scheduleStore.updateOncallShift(shiftId); + } + }, [shiftId]); + + useEffect(() => { + if (shift && !scheduleStore.items[shift.schedule]) { + scheduleStore.loadItem(shift.schedule); + } + }, [shift]); + + const title = isShiftSwap ? 'Shift swap' : showScheduleNameAsSlotTitle ? schedule?.name : getShiftName(shift); + + // const onCallNow = schedule?.on_call_now; + // const isOncall = Boolean(storeUser && onCallNow && onCallNow.some((onCallUser) => storeUser.pk === onCallUser.pk)); return (
@@ -375,7 +381,7 @@ const ScheduleSlotDetails = (props: ScheduleSlotDetailsProps) => {
- {isShiftSwap ? 'Shift swap' : getShiftName(shift)} + {title} @@ -445,11 +451,16 @@ const ScheduleSlotDetails = (props: ScheduleSlotDetailsProps) => { Request shift swap )} - {handleAddOverride && ( + {handleAddOverride && enableWebOverrides && ( )} + {handleOpenSchedule && ( + + )}
diff --git a/grafana-plugin/src/models/schedule/schedule.helpers.ts b/grafana-plugin/src/models/schedule/schedule.helpers.ts index 0b92b8ef..6464db0b 100644 --- a/grafana-plugin/src/models/schedule/schedule.helpers.ts +++ b/grafana-plugin/src/models/schedule/schedule.helpers.ts @@ -1,5 +1,6 @@ import dayjs from 'dayjs'; +import { User } from 'models/user/user.types'; import { RootStore } from 'state'; import { Event, Layer, Schedule, ScheduleType, Shift, ShiftEvents, ShiftSwap } from './schedule.types'; @@ -83,6 +84,14 @@ export const splitToShiftsAndFillGaps = (events: Event[]) => { return shifts; }; +export const getPersonalShiftsFromStore = ( + store: RootStore, + userPk: User['pk'], + startMoment: dayjs.Dayjs +): ShiftEvents[] => { + return store.scheduleStore.personalEvents[userPk]?.[getFromString(startMoment)] as any; +}; + export const getShiftsFromStore = ( store: RootStore, scheduleId: Schedule['id'], @@ -355,6 +364,25 @@ export const SHIFT_SWAP_COLOR = '#C69B06'; const COLORS = [L1_COLORS, L2_COLORS, L3_COLORS]; +const scheduleToColor = {}; + +export const getColorForSchedule = (scheduleId: Schedule['id']) => { + if (scheduleToColor[scheduleId]) { + return scheduleToColor[scheduleId]; + } + + const colors = [...L1_COLORS, ...L2_COLORS, ...L3_COLORS]; + + const index = Object.keys(scheduleToColor).length; + const normalizedIndex = index % colors.length; + + const color = colors[normalizedIndex]; + + scheduleToColor[scheduleId] = color; + + return color; +}; + export const getColor = (layerIndex: number, rotationIndex: number) => { const normalizedLayerIndex = layerIndex % COLORS.length; const normalizedRotationIndex = rotationIndex % COLORS[normalizedLayerIndex]?.length; diff --git a/grafana-plugin/src/models/schedule/schedule.ts b/grafana-plugin/src/models/schedule/schedule.ts index 8219d57e..08749df3 100644 --- a/grafana-plugin/src/models/schedule/schedule.ts +++ b/grafana-plugin/src/models/schedule/schedule.ts @@ -4,6 +4,7 @@ import { action, observable } from 'mobx'; import { RemoteFiltersType } from 'containers/RemoteFilters/RemoteFilters.types'; import BaseStore from 'models/base_store'; import { EscalationChain } from 'models/escalation_chain/escalation_chain.types'; +import { User } from 'models/user/user.types'; import { makeRequest } from 'network'; import { RootStore } from 'state'; import { SelectOption } from 'state/types'; @@ -41,6 +42,8 @@ export class ScheduleStore extends BaseStore { @observable.shallow shifts: { [id: string]: Shift } = {}; + shiftsCurrentlyUpdating = {}; + @observable.shallow relatedEscalationChains: { [id: string]: EscalationChain[] } = {}; @@ -69,6 +72,18 @@ export class ScheduleStore extends BaseStore { }; } = {}; + @observable.shallow + personalEvents: { + [userPk: string]: { + [startMoment: string]: ShiftEvents[]; + }; + } = {}; + + @observable.shallow + onCallNow: { + [userPk: string]: boolean; + } = {}; + @observable finalPreview?: { [fromString: string]: Array<{ shiftId: Shift['id']; events: Event[] }> }; @@ -385,6 +400,12 @@ export class ScheduleStore extends BaseStore { @action async updateOncallShift(shiftId: Shift['id']) { + if (this.shiftsCurrentlyUpdating[shiftId]) { + return; + } + + this.shiftsCurrentlyUpdating[shiftId] = true; + const response = await makeRequest(`/oncall_shifts/${shiftId}`, {}); this.shifts = { @@ -392,6 +413,8 @@ export class ScheduleStore extends BaseStore { [shiftId]: response, }; + delete this.shiftsCurrentlyUpdating[shiftId]; + return response; } @@ -467,7 +490,7 @@ export class ScheduleStore extends BaseStore { } async loadShiftSwap(id: ShiftSwap['id']) { - const result = await makeRequest(`/shift_swaps/${id}`, {}); + const result = await makeRequest(`/shift_swaps/${id}`, { params: { expand_users: true } }); this.shiftSwaps = { ...this.shiftSwaps, [id]: result }; @@ -511,4 +534,37 @@ export class ScheduleStore extends BaseStore { }, }; } + + async updatePersonalEvents(userPk: User['pk'], startMoment: dayjs.Dayjs, days = 9) { + const fromString = getFromString(startMoment); + + const dayBefore = startMoment.subtract(1, 'day'); + + const { is_oncall, schedules } = await makeRequest(`/schedules/current_user_events/`, { + method: 'GET', + params: { + date: getFromString(dayBefore), + days, + }, + }); + + const shiftEventsList = schedules.reduce((acc, schedule) => { + return [...acc, ...splitToShiftsAndFillGaps(schedule.events)]; + }, []); + + const shiftEventsListFlattened = flattenShiftEvents(shiftEventsList); + + this.personalEvents = { + ...this.personalEvents, + [userPk]: { + ...this.personalEvents[userPk], + [fromString]: shiftEventsListFlattened, + }, + }; + + this.onCallNow = { + ...this.onCallNow, + [userPk]: is_oncall, + }; + } } diff --git a/grafana-plugin/src/models/schedule/schedule.types.ts b/grafana-plugin/src/models/schedule/schedule.types.ts index 670109cc..2ce3272e 100644 --- a/grafana-plugin/src/models/schedule/schedule.types.ts +++ b/grafana-plugin/src/models/schedule/schedule.types.ts @@ -139,9 +139,9 @@ export interface ShiftSwap { schedule: Schedule['id']; swap_start: string; swap_end: string; - beneficiary: User['pk']; + beneficiary: Partial; status: 'open' | 'taken' | 'past_due'; - benefactor: User['pk']; + benefactor: Partial; description: string; } diff --git a/grafana-plugin/src/models/user/user.types.ts b/grafana-plugin/src/models/user/user.types.ts index 87ab5be3..a89e0f22 100644 --- a/grafana-plugin/src/models/user/user.types.ts +++ b/grafana-plugin/src/models/user/user.types.ts @@ -10,7 +10,9 @@ export interface User { email: string; phone: string; avatar: string; + avatar_full: string; name: string; + display_name: string; company: string; hide_phone_number: boolean; role_in_company: string; diff --git a/grafana-plugin/src/pages/index.tsx b/grafana-plugin/src/pages/index.tsx index ca363964..bea68483 100644 --- a/grafana-plugin/src/pages/index.tsx +++ b/grafana-plugin/src/pages/index.tsx @@ -63,6 +63,7 @@ export const pages: { [id: string]: PageDefinition } = [ icon: 'calendar-alt', id: 'schedules', text: 'Schedules', + hideTitle: true, hideFromBreadcrumbs: true, path: getPath('schedules'), action: UserActions.SchedulesRead, diff --git a/grafana-plugin/src/pages/schedules/Schedules.module.css b/grafana-plugin/src/pages/schedules/Schedules.module.css index 986a7a22..2511f367 100644 --- a/grafana-plugin/src/pages/schedules/Schedules.module.css +++ b/grafana-plugin/src/pages/schedules/Schedules.module.css @@ -3,6 +3,14 @@ margin: 20px 0; } +.schedule-personal { + position: relative; + margin: 20px 0; + + --rotations-border: var(--border-weak); + --rotations-background: var(--background-secondary); +} + .title { margin-bottom: var(--title-marginBottom); } @@ -18,6 +26,7 @@ row-gap: 4px; column-gap: 8px; width: 100%; + margin-bottom: 20px; } .schedules__actions { @@ -25,7 +34,6 @@ justify-content: flex-end; flex-grow: 1; gap: 8px; - padding-top: 19px; } .schedules__user-on-call { diff --git a/grafana-plugin/src/pages/schedules/Schedules.tsx b/grafana-plugin/src/pages/schedules/Schedules.tsx index 7fc5950f..d2fa86be 100644 --- a/grafana-plugin/src/pages/schedules/Schedules.tsx +++ b/grafana-plugin/src/pages/schedules/Schedules.tsx @@ -21,6 +21,7 @@ import WithConfirm from 'components/WithConfirm/WithConfirm'; import RemoteFilters from 'containers/RemoteFilters/RemoteFilters'; import { RemoteFiltersType } from 'containers/RemoteFilters/RemoteFilters.types'; import ScheduleFinal from 'containers/Rotations/ScheduleFinal'; +import SchedulePersonal from 'containers/Rotations/SchedulePersonal'; import ScheduleForm from 'containers/ScheduleForm/ScheduleForm'; import TeamName from 'containers/TeamName/TeamName'; import { WithPermissionControlTooltip } from 'containers/WithPermissionControl/WithPermissionControlTooltip'; @@ -68,31 +69,19 @@ class SchedulesPage extends React.Component filters === this.state.filters); - - this.setState({ page: p ? Number(p) : 1 }, this.updateSchedules); - } */ - - /* updateSchedules = async () => { - const { store } = this.props; - const { filters, page } = this.state; - - await store.scheduleStore.updateItems(filters, page); - }; - */ render() { const { store, query } = this.props; const { grafanaTeamStore } = store; - const { showNewScheduleSelector, expandedRowKeys, scheduleIdToEdit, page } = this.state; + const { showNewScheduleSelector, expandedRowKeys, scheduleIdToEdit, page, startMoment } = this.state; const { results, count } = store.scheduleStore.getSearchResult(); @@ -149,14 +138,9 @@ class SchedulesPage extends React.Component
- -
- +
+ + Schedules
{users && (
-
- + +
+ { + console.log(rest); }} - emptyText={this.renderNotFound()} /> - +
+
+ +
+
{showNewScheduleSelector && ( @@ -451,7 +453,9 @@ class SchedulesPage extends React.Component { const { store } = this.props; - const { page } = this.state; + const { page, startMoment } = this.state; + + store.scheduleStore.updatePersonalEvents(store.userStore.currentUserPk, startMoment); // For removal we need to check if count is 1 // which means we should change the page to the previous one From 6896d267fb7447d390ff3daa12de882c423a3166 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Wed, 20 Sep 2023 08:49:58 -0300 Subject: [PATCH 02/14] Refactoring/optimizing some bits in schedule views (#3039) Related to https://github.com/grafana/oncall-private/issues/2163 --- engine/apps/api/tests/test_schedules.py | 2 +- engine/apps/api/views/schedule.py | 43 ++++++++++--------- engine/apps/schedules/ical_utils.py | 6 +-- .../apps/schedules/models/on_call_schedule.py | 2 +- engine/common/api_helpers/mixins.py | 6 ++- 5 files changed, 32 insertions(+), 27 deletions(-) diff --git a/engine/apps/api/tests/test_schedules.py b/engine/apps/api/tests/test_schedules.py index 50e6d508..6f703bda 100644 --- a/engine/apps/api/tests/test_schedules.py +++ b/engine/apps/api/tests/test_schedules.py @@ -2081,7 +2081,7 @@ def test_get_schedule_on_call_now( client = APIClient() url = reverse("api-internal:schedule-list") with patch( - "apps.schedules.models.on_call_schedule.OnCallScheduleQuerySet.get_oncall_users", + "apps.api.views.schedule.get_oncall_users_for_multiple_schedules", return_value={schedule.pk: [user]}, ): response = client.get(url, format="json", **make_user_auth_headers(user, token)) diff --git a/engine/apps/api/views/schedule.py b/engine/apps/api/views/schedule.py index 7da0ac6a..b46ac785 100644 --- a/engine/apps/api/views/schedule.py +++ b/engine/apps/api/views/schedule.py @@ -34,6 +34,7 @@ from apps.auth_token.auth import PluginAuthentication from apps.auth_token.constants import SCHEDULE_EXPORT_TOKEN_NAME from apps.auth_token.models import ScheduleExportAuthToken from apps.mobile_app.auth import MobileAppAuthTokenAuthentication +from apps.schedules.ical_utils import get_oncall_users_for_multiple_schedules from apps.schedules.models import OnCallSchedule from apps.slack.models import SlackChannel from apps.slack.tasks import update_slack_user_group_for_schedules @@ -136,10 +137,8 @@ class ScheduleView( The result of this method is cached and is reused for the whole lifetime of a request, since self.get_serializer_context() is called multiple times for every instance in the queryset. """ - current_page_schedules = self.paginate_queryset(self.filter_queryset(self.get_queryset())) - pks = [schedule.pk for schedule in current_page_schedules] - queryset = OnCallSchedule.objects.filter(pk__in=pks) - return queryset.get_oncall_users() + current_page_schedules = self.paginate_queryset(self.filter_queryset(self.get_queryset(annotate=False))) + return get_oncall_users_for_multiple_schedules(current_page_schedules) def get_serializer_context(self): context = super().get_serializer_context() @@ -167,7 +166,7 @@ class ScheduleView( ) return queryset - def get_queryset(self, ignore_filtering_by_available_teams=False): + def get_queryset(self, ignore_filtering_by_available_teams=False, annotate=True): is_short_request = self.request.query_params.get("short", "false") == "true" filter_by_type = self.request.query_params.getlist("type") mine = BooleanField(allow_null=True).to_internal_value(data=self.request.query_params.get("mine")) @@ -181,7 +180,7 @@ class ScheduleView( ) if not ignore_filtering_by_available_teams: queryset = queryset.filter(*self.available_teams_lookup_args).distinct() - if not is_short_request: + if not is_short_request or annotate: queryset = self._annotate_queryset(queryset) queryset = self.serializer_class.setup_eager_loading(queryset) if filter_by_type: @@ -231,15 +230,16 @@ class ScheduleView( if instance.user_group is not None: update_slack_user_group_for_schedules.apply_async((instance.user_group.pk,)) - def get_object(self) -> OnCallSchedule: + def get_object(self, annotate=True) -> OnCallSchedule: # get the object from the whole organization if there is a flag `get_from_organization=true` # otherwise get the object from the current team get_from_organization: bool = self.request.query_params.get("from_organization", "false") == "true" if get_from_organization: - return self.get_object_from_organization() - return super().get_object() + return self.get_object_from_organization(annotate=annotate) + queryset_kwargs = {"annotate": annotate} + return super().get_object(queryset_kwargs) - def get_object_from_organization(self, ignore_filtering_by_available_teams=False): + def get_object_from_organization(self, ignore_filtering_by_available_teams=False, annotate=True): # use this method to get the object from the whole organization instead of the current team pk = self.kwargs["pk"] organization = self.request.auth.organization @@ -248,7 +248,10 @@ class ScheduleView( ) if not ignore_filtering_by_available_teams: queryset = queryset.filter(*self.available_teams_lookup_args).distinct() - queryset = self._annotate_queryset(queryset) + + if annotate: + queryset = self._annotate_queryset(queryset) + queryset = self.serializer_class.setup_eager_loading(queryset) try: obj = queryset.get() @@ -283,7 +286,7 @@ class ScheduleView( with_empty = self.request.query_params.get("with_empty", False) == "true" with_gap = self.request.query_params.get("with_gap", False) == "true" - schedule = self.get_object() + schedule = self.get_object(annotate=False) pytz_tz = pytz.timezone(user_tz) datetime_start = datetime.datetime.combine(starting_date, datetime.time.min, tzinfo=pytz_tz) @@ -319,7 +322,7 @@ class ScheduleView( raise BadRequest(detail="Invalid type value") resolve_schedule = filter_by is None or filter_by == EVENTS_FILTER_BY_FINAL - schedule = self.get_object() + schedule = self.get_object(annotate=False) pytz_tz = pytz.timezone(user_tz) datetime_start = datetime.datetime.combine(starting_date, datetime.time.min, tzinfo=pytz_tz) @@ -349,7 +352,7 @@ class ScheduleView( @action(detail=True, methods=["get"]) def filter_shift_swaps(self, request: Request, pk: str) -> Response: user_tz, starting_date, days = get_date_range_from_request(self.request) - schedule = self.get_object() + schedule = self.get_object(annotate=False) pytz_tz = pytz.timezone(user_tz) datetime_start = datetime.datetime.combine(starting_date, datetime.time.min, tzinfo=pytz_tz) @@ -367,7 +370,7 @@ class ScheduleView( """Return next shift for users in schedule.""" now = timezone.now() datetime_end = now + datetime.timedelta(days=30) - schedule = self.get_object() + schedule = self.get_object(annotate=False) events = schedule.final_events(now, datetime_end) @@ -382,7 +385,7 @@ class ScheduleView( @action(detail=True, methods=["get"]) def related_users(self, request, pk): - schedule = self.get_object() + schedule = self.get_object(annotate=False) serializer = ScheduleUserSerializer(schedule.related_users(), many=True) result = {"users": serializer.data} return Response(result, status=status.HTTP_200_OK) @@ -390,7 +393,7 @@ class ScheduleView( @action(detail=True, methods=["get"]) def related_escalation_chains(self, request, pk): """Return escalation chains associated to schedule.""" - schedule = self.get_object() + schedule = self.get_object(annotate=True) escalation_chains = EscalationChain.objects.filter(escalation_policies__notify_schedule=schedule).distinct() result = [{"name": e.name, "pk": e.public_primary_key} for e in escalation_chains] @@ -398,7 +401,7 @@ class ScheduleView( @action(detail=True, methods=["get"]) def quality(self, request, pk): - schedule = self.get_object() + schedule = self.get_object(annotate=False) _, date = self.get_request_timezone() datetime_start = datetime.datetime.combine(date, datetime.time.min, tzinfo=pytz.UTC) @@ -440,7 +443,7 @@ class ScheduleView( @action(detail=True, methods=["post"]) def reload_ical(self, request, pk): - schedule = self.get_object() + schedule = self.get_object(annotate=False) schedule.drop_cached_ical() schedule.check_empty_shifts_for_next_week() schedule.check_gaps_for_next_week() @@ -452,7 +455,7 @@ class ScheduleView( @action(detail=True, methods=["get", "post", "delete"]) def export_token(self, request, pk): - schedule = self.get_object() + schedule = self.get_object(annotate=False) if self.request.method == "GET": try: diff --git a/engine/apps/schedules/ical_utils.py b/engine/apps/schedules/ical_utils.py index 5a487677..cdd02ffc 100644 --- a/engine/apps/schedules/ical_utils.py +++ b/engine/apps/schedules/ical_utils.py @@ -366,18 +366,18 @@ def list_users_to_notify_from_ical_for_period( def get_oncall_users_for_multiple_schedules( - schedules: "OnCallScheduleQuerySet", events_datetime=None + schedules: typing.List["OnCallSchedule"], events_datetime=None ) -> typing.Dict["OnCallSchedule", UserQuerySet]: if events_datetime is None: events_datetime = datetime.datetime.now(timezone.utc) # Exit early if there are no schedules - if not schedules.exists(): + if not schedules: return {} # Get on-call users oncall_users = {} - for schedule in schedules.all(): + for schedule in schedules: # pass user list to list_users_to_notify_from_ical schedule_oncall_users = list_users_to_notify_from_ical(schedule, events_datetime=events_datetime) oncall_users.update({schedule.pk: schedule_oncall_users}) diff --git a/engine/apps/schedules/models/on_call_schedule.py b/engine/apps/schedules/models/on_call_schedule.py index 3275789c..e1f1103c 100644 --- a/engine/apps/schedules/models/on_call_schedule.py +++ b/engine/apps/schedules/models/on_call_schedule.py @@ -151,7 +151,7 @@ def generate_public_primary_key_for_oncall_schedule_channel(): class OnCallScheduleQuerySet(PolymorphicQuerySet): def get_oncall_users(self, events_datetime=None): - return get_oncall_users_for_multiple_schedules(self, events_datetime) + return get_oncall_users_for_multiple_schedules(self.all(), events_datetime) def related_to_user(self, user): username_regex = RE_ICAL_SEARCH_USERNAME.format(user.username) diff --git a/engine/common/api_helpers/mixins.py b/engine/common/api_helpers/mixins.py index c873c0d7..4204095f 100644 --- a/engine/common/api_helpers/mixins.py +++ b/engine/common/api_helpers/mixins.py @@ -150,9 +150,11 @@ _MT = typing.TypeVar("_MT", bound=models.Model) class PublicPrimaryKeyMixin(typing.Generic[_MT]): - def get_object(self) -> _MT: + def get_object(self, queryset_kwargs=None) -> _MT: pk = self.kwargs["pk"] - queryset = self.filter_queryset(self.get_queryset()) + if queryset_kwargs is None: + queryset_kwargs = {} + queryset = self.filter_queryset(self.get_queryset(**queryset_kwargs)) try: obj = queryset.get(public_primary_key=pk) From 2848836f6fd799508e3449acc723ec9005a3f313 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 20 Sep 2023 17:02:27 +0100 Subject: [PATCH 03/14] Update `make docs` procedure (#3048) Co-authored-by: grafanabot Co-authored-by: Jack Baldry --- docs/docs.mk | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/docs.mk b/docs/docs.mk index 2611bbe8..c4ba766f 100644 --- a/docs/docs.mk +++ b/docs/docs.mk @@ -80,7 +80,7 @@ docs-pull: ## Pull documentation base image. make-docs: ## Fetch the latest make-docs script. make-docs: - if [[ ! -f "$(PWD)/make-docs" ]]; then + if [[ ! -f "$(CURDIR)/make-docs" ]]; then echo 'WARN: No make-docs script found in the working directory. Run `make update` to download it.' >&2 exit 1 fi @@ -88,27 +88,27 @@ make-docs: .PHONY: docs docs: ## Serve documentation locally, which includes pulling the latest `DOCS_IMAGE` (default: `grafana/docs-base:latest`) container image. See also `docs-no-pull`. docs: docs-pull make-docs - $(PWD)/make-docs $(PROJECTS) + $(CURDIR)/make-docs $(PROJECTS) .PHONY: docs-no-pull docs-no-pull: ## Serve documentation locally without pulling the `DOCS_IMAGE` (default: `grafana/docs-base:latest`) container image. docs-no-pull: make-docs - $(PWD)/make-docs $(PROJECTS) + $(CURDIR)/make-docs $(PROJECTS) .PHONY: docs-debug docs-debug: ## Run Hugo web server with debugging enabled. TODO: support all SERVER_FLAGS defined in website Makefile. docs-debug: make-docs - WEBSITE_EXEC='hugo server --bind 0.0.0.0 --port 3002 --debug' $(PWD)/make-docs $(PROJECTS) + WEBSITE_EXEC='hugo server --bind 0.0.0.0 --port 3002 --debug' $(CURDIR)/make-docs $(PROJECTS) .PHONY: doc-validator doc-validator: ## Run doc-validator on the entire docs folder. doc-validator: make-docs - DOCS_IMAGE=$(DOC_VALIDATOR_IMAGE) $(PWD)/make-docs $(PROJECTS) + DOCS_IMAGE=$(DOC_VALIDATOR_IMAGE) $(CURDIR)/make-docs $(PROJECTS) .PHONY: vale vale: ## Run vale on the entire docs folder. vale: make-docs - DOCS_IMAGE=$(VALE_IMAGE) $(PWD)/make-docs $(PROJECTS) + DOCS_IMAGE=$(VALE_IMAGE) $(CURDIR)/make-docs $(PROJECTS) .PHONY: update update: ## Fetch the latest version of this Makefile and the `make-docs` script from Writers' Toolkit. From 85c8605eadc2958cdf360c8844100e52a099bceb Mon Sep 17 00:00:00 2001 From: Maxim Mordasov Date: Thu, 21 Sep 2023 17:31:56 +0300 Subject: [PATCH 04/14] Unify breadcrumbs behaviour with other Grafana Apps and main core (#3003) # What this PR does Unify breadcrumbs behaviour with other Grafana Apps and main core ## Which issue(s) this PR fixes https://github.com/grafana/oncall/issues/1906 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 5 ++++ grafana-plugin/.eslintrc.js | 2 +- .../DefaultPageLayout/DefaultPageLayout.tsx | 10 +++---- .../src/models/alertgroup/alertgroup.ts | 6 +++-- .../src/pages/incident/Incident.tsx | 18 +++++++++++-- grafana-plugin/src/pages/index.tsx | 26 +++++++++++++----- .../src/pages/schedule/Schedule.tsx | 27 +++++++++++++++---- .../src/plugin/GrafanaPluginRootPage.tsx | 13 ++++++--- 8 files changed, 82 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23aac416..27ffc083 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- Unify breadcrumbs behaviour with other Grafana Apps and main core ([#1906](https://github.com/grafana/oncall/issues/1906)) + ## v1.3.38 (2023-09-19) ### Fixed @@ -20,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Notify user via Slack/mobile push-notification when their shift swap request is taken by @joeyorlando ([#2992](https://github.com/grafana/oncall/pull/2992)) +- Unify breadcrumbs behaviour with other Grafana Apps and main core# ([1906](https://github.com/grafana/oncall/issues/1906)) ### Changed diff --git a/grafana-plugin/.eslintrc.js b/grafana-plugin/.eslintrc.js index 46f9f165..92831e8c 100644 --- a/grafana-plugin/.eslintrc.js +++ b/grafana-plugin/.eslintrc.js @@ -6,7 +6,7 @@ module.exports = { plugins: ['rulesdir', 'import'], settings: { 'import/internal-regex': - '^assets|^components|^containers|^icons|^models|^network|^pages|^services|^state|^utils|^plugin', + '^assets|^components|^containers|^contexts|^icons|^models|^network|^pages|^services|^state|^utils|^plugin', }, rules: { eqeqeq: 'warn', diff --git a/grafana-plugin/src/containers/DefaultPageLayout/DefaultPageLayout.tsx b/grafana-plugin/src/containers/DefaultPageLayout/DefaultPageLayout.tsx index 3b2531a1..8f5155bc 100644 --- a/grafana-plugin/src/containers/DefaultPageLayout/DefaultPageLayout.tsx +++ b/grafana-plugin/src/containers/DefaultPageLayout/DefaultPageLayout.tsx @@ -1,14 +1,13 @@ import React, { FC } from 'react'; +import { NavModelItem } from '@grafana/data'; import { PluginPage } from 'PluginPage'; import cn from 'classnames/bind'; import { observer } from 'mobx-react'; import { AppRootProps } from 'types'; import Alerts from 'containers/Alerts/Alerts'; -import { pages } from 'pages'; import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; -import { DEFAULT_PAGE } from 'utils/consts'; import styles from './DefaultPageLayout.module.scss'; @@ -17,10 +16,11 @@ const cx = cn.bind(styles); interface DefaultPageLayoutProps extends AppRootProps { children?: any; page: string; + pageNav: NavModelItem; } const DefaultPageLayout: FC = observer((props) => { - const { children, page } = props; + const { children, page, pageNav } = props; if (isTopNavbar()) { return renderTopNavbar(); @@ -29,10 +29,8 @@ const DefaultPageLayout: FC = observer((props) => { return renderLegacyNavbar(); function renderTopNavbar(): JSX.Element { - const matchingPageNav = (pages[page] || pages[DEFAULT_PAGE]).getPageNav(); - return ( - +
{children}
 ); diff --git a/grafana-plugin/src/models/alertgroup/alertgroup.ts b/grafana-plugin/src/models/alertgroup/alertgroup.ts index 5b978afb..f0068af1 100644 --- a/grafana-plugin/src/models/alertgroup/alertgroup.ts +++ b/grafana-plugin/src/models/alertgroup/alertgroup.ts @@ -312,9 +312,11 @@ export class AlertGroupStore extends BaseStore { } @action - getAlert(pk: Alert['pk']) { - return makeRequest(`${this.path}${pk}`, {}).then((alert: Alert) => { + async getAlert(pk: Alert['pk']) { + return await makeRequest(`${this.path}${pk}`, {}).then((alert: Alert) => { this.alerts.set(pk, alert); + + return alert; }); } diff --git a/grafana-plugin/src/pages/incident/Incident.tsx b/grafana-plugin/src/pages/incident/Incident.tsx index 49d44ee9..60cb731e 100644 --- a/grafana-plugin/src/pages/incident/Incident.tsx +++ b/grafana-plugin/src/pages/incident/Incident.tsx @@ -64,7 +64,10 @@ import PagedUsers from './parts/PagedUsers'; const cx = cn.bind(styles); const INTEGRATION_NAME_LENGTH_LIMIT = 30; -interface IncidentPageProps extends WithStoreProps, PageProps, RouteComponentProps<{ id: string }> {} +interface IncidentPageProps extends WithStoreProps, PageProps, RouteComponentProps<{ id: string }> { + pageTitle: string; + setPageTitle: (value: string) => void; +} interface IncidentPageState extends PageBaseState { showIntegrationSettings?: boolean; @@ -89,6 +92,12 @@ class IncidentPage extends React.Component store.alertGroupStore.updateSilenceOptions(); } + componentWillUnmount(): void { + const { setPageTitle } = this.props; + + setPageTitle(undefined); + } + componentDidUpdate(prevProps: IncidentPageProps) { if (this.props.match.params.id !== prevProps.match.params.id) { this.update(); @@ -103,10 +112,14 @@ class IncidentPage extends React.Component match: { params: { id }, }, + setPageTitle, } = this.props; store.alertGroupStore .getAlert(id) + .then((alertGroup) => { + setPageTitle(`#${alertGroup.inside_organization_number} ${alertGroup.render_for_web.title}`); + }) .catch((error) => this.setState({ errorData: { ...getWrongTeamResponseInfo(error) } })); }; @@ -238,6 +251,7 @@ class IncidentPage extends React.Component match: { params: { id }, }, + pageTitle, } = this.props; const { alerts } = store.alertGroupStore; @@ -261,7 +275,7 @@ class IncidentPage extends React.Component {/* @ts-ignore*/} - #{incident.inside_organization_number} {incident.render_for_web.title} + {pageTitle} {incident.root_alert_group && ( diff --git a/grafana-plugin/src/pages/index.tsx b/grafana-plugin/src/pages/index.tsx index bea68483..280c80e7 100644 --- a/grafana-plugin/src/pages/index.tsx +++ b/grafana-plugin/src/pages/index.tsx @@ -17,7 +17,7 @@ export type PageDefinition = { action?: UserAction; hideTitle: boolean; // dont't automatically render title above page content - getPageNav(): { text: string; description: string }; + getPageNav: (pageTitle: string) => NavModelItem; }; function getPath(name = '') { @@ -34,6 +34,20 @@ export const pages: { [id: string]: PageDefinition } = [ path: getPath('alert-groups'), action: UserActions.AlertGroupsRead, }, + { + icon: 'bell', + id: 'alert-group', + text: '', + showOrgSwitcher: true, + getParentItem: (pageTitle: string) => ({ + text: pageTitle, + url: `${PLUGIN_ROOT}/alert-groups`, + }), + hideFromBreadcrumbs: true, + hideFromTabs: true, + path: getPath('alert-group/:id?'), + action: UserActions.AlertGroupsRead, + }, { icon: 'users-alt', id: 'users', @@ -72,10 +86,10 @@ export const pages: { [id: string]: PageDefinition } = [ icon: 'calendar-alt', id: 'schedule', text: '', - parentItem: { - text: 'Schedule', + getParentItem: (pageTitle: string) => ({ + text: pageTitle, url: `${PLUGIN_ROOT}/schedules`, - }, + }), hideFromBreadcrumbs: true, hideFromTabs: true, path: getPath('schedule/:id?'), @@ -139,10 +153,10 @@ export const pages: { [id: string]: PageDefinition } = [ if (!current.action || (current.action && isUserActionAllowed(current.action))) { prev[current.id] = { ...current, - getPageNav: () => + getPageNav: (pageTitle: string) => ({ text: isTopNavbar() ? '' : current.text, - parentItem: current.parentItem, + parentItem: current.getParentItem ? current.getParentItem(pageTitle) : undefined, hideFromBreadcrumbs: current.hideFromBreadcrumbs, hideFromTabs: current.hideFromTabs, } as NavModelItem), diff --git a/grafana-plugin/src/pages/schedule/Schedule.tsx b/grafana-plugin/src/pages/schedule/Schedule.tsx index 9bbb3993..11195d07 100644 --- a/grafana-plugin/src/pages/schedule/Schedule.tsx +++ b/grafana-plugin/src/pages/schedule/Schedule.tsx @@ -39,7 +39,10 @@ import styles from './Schedule.module.css'; const cx = cn.bind(styles); -interface SchedulePageProps extends PageProps, WithStoreProps, RouteComponentProps<{ id: string }> {} +interface SchedulePageProps extends PageProps, WithStoreProps, RouteComponentProps<{ id: string }> { + pageTitle: string; + setPageTitle: (value: string) => void; +} interface SchedulePageState extends PageBaseState { startMoment: dayjs.Dayjs; @@ -100,9 +103,11 @@ class SchedulePage extends React.Component } componentWillUnmount() { - const { store } = this.props; + const { store, setPageTitle } = this.props; store.scheduleStore.clearPreview(); + + setPageTitle(undefined); } render() { @@ -181,7 +186,7 @@ class SchedulePage extends React.Component level={2} onTextChange={this.handleNameChange} > - {schedule?.name} + {pageTitle} {schedule && } @@ -359,10 +364,14 @@ class SchedulePage extends React.Component match: { params: { id: scheduleId }, }, + setPageTitle, } = this.props; + const { scheduleStore } = store; - return scheduleStore.loadItem(scheduleId); + return scheduleStore.loadItem(scheduleId).then((schedule) => { + setPageTitle(schedule?.name); + }); }; handleShowForm = async (shiftId: Shift['id'] | 'new') => { @@ -397,13 +406,17 @@ class SchedulePage extends React.Component match: { params: { id: scheduleId }, }, + setPageTitle, } = this.props; const schedule = store.scheduleStore.items[scheduleId]; store.scheduleStore .update(scheduleId, { type: schedule.type, name: value }) - .then(() => store.scheduleStore.loadItem(scheduleId)); + .then(() => store.scheduleStore.loadItem(scheduleId)) + .then((schedule) => { + setPageTitle(schedule?.name); + }); }; updateEvents = () => { @@ -412,6 +425,7 @@ class SchedulePage extends React.Component match: { params: { id: scheduleId }, }, + setPageTitle, } = this.props; const { startMoment } = this.state; @@ -423,6 +437,9 @@ class SchedulePage extends React.Component store.scheduleStore .loadItem(scheduleId) // to refresh current oncall users + .then((schedule) => { + setPageTitle(schedule?.name); + }) .catch((error) => this.setState({ errorData: { ...getWrongTeamResponseInfo(error) } })); store.scheduleStore.updateRelatedUsers(scheduleId); // to refresh related users diff --git a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx index f04b8866..86494341 100644 --- a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx +++ b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx @@ -38,6 +38,7 @@ import Users from 'pages/users/Users'; import { rootStore } from 'state'; import { useStore } from 'state/useStore'; import { isUserActionAllowed } from 'utils/authorization'; +import { DEFAULT_PAGE } from 'utils/consts'; dayjs.extend(utc); dayjs.extend(timezone); @@ -72,6 +73,8 @@ export const Root = observer((props: AppRootProps) => { const [basicDataLoaded, setBasicDataLoaded] = useState(false); + const [pageTitle, setPageTitle] = useState(''); + useEffect(() => { runQueuedUpdateData(0); }, []); @@ -103,8 +106,12 @@ export const Root = observer((props: AppRootProps) => { const userHasAccess = pagePermissionAction ? isUserActionAllowed(pagePermissionAction) : true; const query = getQueryParams(); + const getPageNav = () => { + return (pages[page] || pages[DEFAULT_PAGE]).getPageNav(pageTitle); + }; + return ( - + {!isTopNavbar() && ( <>
@@ -128,7 +135,7 @@ export const Root = observer((props: AppRootProps) => { - + @@ -146,7 +153,7 @@ export const Root = observer((props: AppRootProps) => { - + From fbec331ae50e28da190bca28f4577291b7b37d06 Mon Sep 17 00:00:00 2001 From: Rares Mardare Date: Fri, 22 Sep 2023 09:12:00 +0300 Subject: [PATCH 05/14] fixed build --- grafana-plugin/src/pages/schedule/Schedule.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/grafana-plugin/src/pages/schedule/Schedule.tsx b/grafana-plugin/src/pages/schedule/Schedule.tsx index 11195d07..3b95bcd8 100644 --- a/grafana-plugin/src/pages/schedule/Schedule.tsx +++ b/grafana-plugin/src/pages/schedule/Schedule.tsx @@ -114,6 +114,7 @@ class SchedulePage extends React.Component const { store, query, + pageTitle, match: { params: { id: scheduleId }, }, From 2f1cc43b0e1b79197555b62e71fa51bad1589493 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 26 Sep 2023 08:54:41 -0600 Subject: [PATCH 06/14] Fix public actions endpoint to handle empty user field (#3053) # What this PR does Fixes a regression that was introduced when actions were remapped to new webhooks. Validation in the serializer was making the user field no longer accept blank and None values. This PR makes those values accepted again. ## Which issue(s) this PR fixes https://github.com/grafana/terraform-provider-grafana/issues/1025 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 4 ++ engine/apps/public_api/serializers/action.py | 21 ++++++---- .../apps/public_api/serializers/webhooks.py | 20 ++++++--- .../public_api/tests/test_custom_actions.py | 41 +++++++++++++++---- engine/apps/public_api/tests/test_webhooks.py | 39 ++++++++++++++++++ 5 files changed, 104 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27ffc083..4d186aee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Unify breadcrumbs behaviour with other Grafana Apps and main core ([#1906](https://github.com/grafana/oncall/issues/1906)) +### Fixed + +- Fix regression in public actions endpoint handling user field by @mderynck ([#3053](https://github.com/grafana/oncall/pull/3053)) + ## v1.3.38 (2023-09-19) ### Fixed diff --git a/engine/apps/public_api/serializers/action.py b/engine/apps/public_api/serializers/action.py index 853369f8..d9af4ff9 100644 --- a/engine/apps/public_api/serializers/action.py +++ b/engine/apps/public_api/serializers/action.py @@ -9,7 +9,7 @@ from common.api_helpers.utils import CurrentTeamDefault class ActionCreateSerializer(WebhookCreateSerializer): team_id = TeamPrimaryKeyRelatedField(allow_null=True, default=CurrentTeamDefault(), source="team") - user = serializers.CharField(required=False, source="username") + user = serializers.CharField(required=False, source="username", allow_null=True, allow_blank=True) trigger_type = WebhookTriggerTypeField(required=False) forward_whole_payload = serializers.BooleanField(required=False, source="forward_all") @@ -23,7 +23,6 @@ class ActionCreateSerializer(WebhookCreateSerializer): "team_id", "user", "data", - "user", "password", "authorization_header", "trigger_template", @@ -37,6 +36,12 @@ class ActionCreateSerializer(WebhookCreateSerializer): extra_kwargs = { "name": {"required": True, "allow_null": False, "allow_blank": False}, "url": {"required": True, "allow_null": False, "allow_blank": False}, + "data": {"required": False, "allow_null": True, "allow_blank": True}, + "password": {"required": False, "allow_null": True, "allow_blank": True}, + "authorization_header": {"required": False, "allow_null": True, "allow_blank": True}, + "trigger_template": {"required": False, "allow_null": True, "allow_blank": True}, + "headers": {"required": False, "allow_null": True, "allow_blank": True}, + "integration_filter": {"required": False, "allow_null": True}, } validators = [UniqueTogetherValidator(queryset=Webhook.objects.all(), fields=["name", "organization"])] @@ -51,13 +56,13 @@ class ActionUpdateSerializer(ActionCreateSerializer): extra_kwargs = { "name": {"required": False, "allow_null": False, "allow_blank": False}, "is_webhook_enabled": {"required": False, "allow_null": False}, - "user": {"required": False, "allow_null": True, "allow_blank": False}, - "password": {"required": False, "allow_null": True, "allow_blank": False}, - "authorization_header": {"required": False, "allow_null": True, "allow_blank": False}, - "trigger_template": {"required": False, "allow_null": True, "allow_blank": False}, - "headers": {"required": False, "allow_null": True, "allow_blank": False}, + "user": {"required": False, "allow_null": True, "allow_blank": True}, + "password": {"required": False, "allow_null": True, "allow_blank": True}, + "authorization_header": {"required": False, "allow_null": True, "allow_blank": True}, + "trigger_template": {"required": False, "allow_null": True, "allow_blank": True}, + "headers": {"required": False, "allow_null": True, "allow_blank": True}, "url": {"required": False, "allow_null": False, "allow_blank": False}, - "data": {"required": False, "allow_null": True, "allow_blank": False}, + "data": {"required": False, "allow_null": True, "allow_blank": True}, "forward_whole_payload": {"required": False, "allow_null": False}, "http_method": {"required": False, "allow_null": False, "allow_blank": False}, "integration_filter": {"required": False, "allow_null": True}, diff --git a/engine/apps/public_api/serializers/webhooks.py b/engine/apps/public_api/serializers/webhooks.py index 44a92634..70f3d6a4 100644 --- a/engine/apps/public_api/serializers/webhooks.py +++ b/engine/apps/public_api/serializers/webhooks.py @@ -78,6 +78,14 @@ class WebhookCreateSerializer(serializers.ModelSerializer): "name": {"required": True, "allow_null": False, "allow_blank": False}, "url": {"required": True, "allow_null": False, "allow_blank": False}, "http_method": {"required": True, "allow_null": False, "allow_blank": False}, + "username": {"required": False, "allow_null": True, "allow_blank": True}, + "password": {"required": False, "allow_null": True, "allow_blank": True}, + "authorization_header": {"required": False, "allow_null": True, "allow_blank": True}, + "trigger_template": {"required": False, "allow_null": True, "allow_blank": True}, + "headers": {"required": False, "allow_null": True, "allow_blank": True}, + "data": {"required": False, "allow_null": True, "allow_blank": True}, + "forward_all": {"required": False, "allow_null": False}, + "integration_filter": {"required": False, "allow_null": True}, } validators = [UniqueTogetherValidator(queryset=Webhook.objects.all(), fields=["name", "organization"])] @@ -157,13 +165,13 @@ class WebhookUpdateSerializer(WebhookCreateSerializer): extra_kwargs = { "name": {"required": False, "allow_null": False, "allow_blank": False}, "is_webhook_enabled": {"required": False, "allow_null": False}, - "username": {"required": False, "allow_null": True, "allow_blank": False}, - "password": {"required": False, "allow_null": True, "allow_blank": False}, - "authorization_header": {"required": False, "allow_null": True, "allow_blank": False}, - "trigger_template": {"required": False, "allow_null": True, "allow_blank": False}, - "headers": {"required": False, "allow_null": True, "allow_blank": False}, + "username": {"required": False, "allow_null": True, "allow_blank": True}, + "password": {"required": False, "allow_null": True, "allow_blank": True}, + "authorization_header": {"required": False, "allow_null": True, "allow_blank": True}, + "trigger_template": {"required": False, "allow_null": True, "allow_blank": True}, + "headers": {"required": False, "allow_null": True, "allow_blank": True}, "url": {"required": False, "allow_null": False, "allow_blank": False}, - "data": {"required": False, "allow_null": True, "allow_blank": False}, + "data": {"required": False, "allow_null": True, "allow_blank": True}, "forward_all": {"required": False, "allow_null": False}, "http_method": {"required": False, "allow_null": False, "allow_blank": False}, "integration_filter": {"required": False, "allow_null": True}, diff --git a/engine/apps/public_api/tests/test_custom_actions.py b/engine/apps/public_api/tests/test_custom_actions.py index f763b7a3..62504c38 100644 --- a/engine/apps/public_api/tests/test_custom_actions.py +++ b/engine/apps/public_api/tests/test_custom_actions.py @@ -160,17 +160,44 @@ def test_get_custom_action( @pytest.mark.django_db -def test_create_custom_action(make_organization_and_user_with_token): +@pytest.mark.parametrize( + "data", + [ + ( + { + "name": "Test outgoing webhook", + "url": "https://example.com", + } + ), + ( + { + "name": "Test outgoing webhook", + "url": "https://example.com", + "user": None, + "password": None, + "data": None, + "authorization_header": None, + "forward_whole_payload": True, + } + ), + ( + { + "name": "Test outgoing webhook", + "url": "https://example.com", + "user": "", + "password": "", + "data": "", + "authorization_header": "", + "forward_whole_payload": True, + } + ), + ], +) +def test_create_custom_action(make_organization_and_user_with_token, data): organization, user, token = make_organization_and_user_with_token() client = APIClient() url = reverse("api-public:actions-list") - - data = { - "name": "Test outgoing webhook", - "url": "https://example.com", - } - response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") custom_action = Webhook.objects.get(public_primary_key=response.data["id"]) diff --git a/engine/apps/public_api/tests/test_webhooks.py b/engine/apps/public_api/tests/test_webhooks.py index 0e6feb3f..efe7870f 100644 --- a/engine/apps/public_api/tests/test_webhooks.py +++ b/engine/apps/public_api/tests/test_webhooks.py @@ -163,6 +163,45 @@ def test_create_webhook(make_organization_and_user_with_token): assert response.data == expected_result +@pytest.mark.django_db +@pytest.mark.parametrize( + "optional_value", + [ + None, + "", + ], +) +def test_create_webhook_optional_fields(make_organization_and_user_with_token, optional_value): + organization, user, token = make_organization_and_user_with_token() + client = APIClient() + + url = reverse("api-public:webhooks-list") + + data = { + "name": "Test outgoing webhook with nested data", + "url": "https://example.com", + "http_method": "POST", + "trigger_type": "acknowledge", + "data": optional_value, + "username": optional_value, + "password": optional_value, + "authorization_header": optional_value, + "trigger_template": optional_value, + "headers": optional_value, + "forward_all": True, + "is_webhook_enabled": True, + "integration_filter": optional_value, + } + + response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + webhook = Webhook.objects.get(public_primary_key=response.data["id"]) + + expected_result = _get_expected_result(webhook) + + assert response.status_code == status.HTTP_201_CREATED + assert response.json() == expected_result + + @pytest.mark.django_db def test_create_webhook_nested_data(make_organization_and_user_with_token): organization, user, token = make_organization_and_user_with_token() From cc337d1473a0e90277c40f47a99af56041e1b523 Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Tue, 26 Sep 2023 16:31:26 +0100 Subject: [PATCH 07/14] Add mobile app auth on integration & escalation chain endpoints (#3056) --- engine/apps/api/views/alert_receive_channel.py | 6 +++++- engine/apps/api/views/escalation_chain.py | 6 +++++- engine/apps/api/views/resolution_note.py | 6 +++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/engine/apps/api/views/alert_receive_channel.py b/engine/apps/api/views/alert_receive_channel.py index 595f2033..d58de905 100644 --- a/engine/apps/api/views/alert_receive_channel.py +++ b/engine/apps/api/views/alert_receive_channel.py @@ -20,6 +20,7 @@ from apps.api.serializers.alert_receive_channel import ( from apps.api.throttlers import DemoAlertThrottler from apps.auth_token.auth import PluginAuthentication from apps.integrations.legacy_prefix import has_legacy_prefix, remove_legacy_prefix +from apps.mobile_app.auth import MobileAppAuthTokenAuthentication from common.api_helpers.exceptions import BadRequest from common.api_helpers.filters import ByTeamModelFieldFilterMixin, TeamModelMultipleChoiceFilter from common.api_helpers.mixins import ( @@ -72,7 +73,10 @@ class AlertReceiveChannelView( UpdateSerializerMixin, ModelViewSet, ): - authentication_classes = (PluginAuthentication,) + authentication_classes = ( + MobileAppAuthTokenAuthentication, + PluginAuthentication, + ) permission_classes = (IsAuthenticated, RBACPermission) model = AlertReceiveChannel diff --git a/engine/apps/api/views/escalation_chain.py b/engine/apps/api/views/escalation_chain.py index 1a51c010..c125b031 100644 --- a/engine/apps/api/views/escalation_chain.py +++ b/engine/apps/api/views/escalation_chain.py @@ -15,6 +15,7 @@ from apps.api.serializers.escalation_chain import ( FilterEscalationChainSerializer, ) from apps.auth_token.auth import PluginAuthentication +from apps.mobile_app.auth import MobileAppAuthTokenAuthentication from apps.user_management.models import Team from common.api_helpers.exceptions import BadRequest from common.api_helpers.filters import ByTeamModelFieldFilterMixin, ModelFieldFilterMixin, TeamModelMultipleChoiceFilter @@ -38,7 +39,10 @@ class EscalationChainViewSet( ListSerializerMixin, viewsets.ModelViewSet, ): - authentication_classes = (PluginAuthentication,) + authentication_classes = ( + MobileAppAuthTokenAuthentication, + PluginAuthentication, + ) permission_classes = (IsAuthenticated, RBACPermission) rbac_permissions = { diff --git a/engine/apps/api/views/resolution_note.py b/engine/apps/api/views/resolution_note.py index bd0b0ef1..ef6c8e9d 100644 --- a/engine/apps/api/views/resolution_note.py +++ b/engine/apps/api/views/resolution_note.py @@ -6,11 +6,15 @@ from apps.alerts.tasks import send_update_resolution_note_signal from apps.api.permissions import RBACPermission from apps.api.serializers.resolution_note import ResolutionNoteSerializer, ResolutionNoteUpdateSerializer from apps.auth_token.auth import PluginAuthentication +from apps.mobile_app.auth import MobileAppAuthTokenAuthentication from common.api_helpers.mixins import PublicPrimaryKeyMixin, TeamFilteringMixin, UpdateSerializerMixin class ResolutionNoteView(TeamFilteringMixin, PublicPrimaryKeyMixin, UpdateSerializerMixin, ModelViewSet): - authentication_classes = (PluginAuthentication,) + authentication_classes = ( + MobileAppAuthTokenAuthentication, + PluginAuthentication, + ) permission_classes = (IsAuthenticated, RBACPermission) rbac_permissions = { From 9ead70a4ed575e89aa3cd55192a05bc9354da5c3 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Tue, 26 Sep 2023 14:04:53 -0300 Subject: [PATCH 08/14] Add schedules enable_web_overrides option to public API (#3062) Related updates (to be merged afterwards): - https://github.com/grafana/amixr-api-go-client/pull/14 - https://github.com/grafana/terraform-provider-grafana/pull/1056 --- CHANGELOG.md | 1 + .../sources/oncall-api-reference/schedules.md | 1 + .../serializers/schedules_calendar.py | 4 ++ .../apps/public_api/tests/test_schedules.py | 46 +++++++++++++++++++ 4 files changed, 52 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d186aee..55151194 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Unify breadcrumbs behaviour with other Grafana Apps and main core ([#1906](https://github.com/grafana/oncall/issues/1906)) +- Add `enable_web_overrides` option to schedules public API ([#3062](https://github.com/grafana/oncall/pull/3062)) ### Fixed diff --git a/docs/sources/oncall-api-reference/schedules.md b/docs/sources/oncall-api-reference/schedules.md index f9335ff5..9e89f422 100644 --- a/docs/sources/oncall-api-reference/schedules.md +++ b/docs/sources/oncall-api-reference/schedules.md @@ -47,6 +47,7 @@ The above command returns JSON structured in the following way: | `time_zone` | No | Optional | Schedule time zone. Is used for manually added on-call shifts in Schedules with type `calendar`. Default time zone is `UTC`. For more information about time zones, see [time zones](https://en.wikipedia.org/wiki/List_of_tz_database_time_zones). | | `ical_url_primary` | No | If type = `ical` | URL of external iCal calendar for schedule with type `ical`. | | `ical_url_overrides` | No | Optional | URL of external iCal calendar for schedule with any type. Events from this calendar override events from primary calendar or from on-call shifts. | +| `enable_web_overrides` | No | Optional | Whether to enable web overrides or not. Setting specific for API/Terraform based schedules (`calendar` type). | | `slack` | No | Optional | Dictionary with Slack-specific settings for a schedule. Includes `channel_id` and `user_group_id` fields, that take a channel ID and a user group ID from Slack. | | `shifts` | No | Optional | List of shifts. Used for manually added on-call shifts in Schedules with type `calendar`. | diff --git a/engine/apps/public_api/serializers/schedules_calendar.py b/engine/apps/public_api/serializers/schedules_calendar.py index a71b82bc..8f85208c 100644 --- a/engine/apps/public_api/serializers/schedules_calendar.py +++ b/engine/apps/public_api/serializers/schedules_calendar.py @@ -28,9 +28,11 @@ class ScheduleCalendarSerializer(ScheduleBaseSerializer): "on_call_now", "shifts", "ical_url_overrides", + "enable_web_overrides", ] extra_kwargs = { "ical_url_overrides": {"required": False, "allow_null": True}, + "enable_web_overrides": {"required": False, "allow_null": True}, } def validate_shifts(self, shifts): @@ -61,10 +63,12 @@ class ScheduleCalendarUpdateSerializer(ScheduleCalendarSerializer): "on_call_now", "shifts", "ical_url_overrides", + "enable_web_overrides", ] extra_kwargs = { "name": {"required": False}, "ical_url_overrides": {"required": False, "allow_null": True}, + "enable_web_overrides": {"required": False, "allow_null": True}, } def update(self, instance, validated_data): diff --git a/engine/apps/public_api/tests/test_schedules.py b/engine/apps/public_api/tests/test_schedules.py index 3834c8df..2e1ed13e 100644 --- a/engine/apps/public_api/tests/test_schedules.py +++ b/engine/apps/public_api/tests/test_schedules.py @@ -94,6 +94,7 @@ def test_get_calendar_schedule( "user_group_id": None, }, "ical_url_overrides": None, + "enable_web_overrides": False, } assert response.status_code == status.HTTP_200_OK @@ -130,6 +131,7 @@ def test_create_calendar_schedule(make_organization_and_user_with_token): "user_group_id": None, }, "ical_url_overrides": None, + "enable_web_overrides": False, } assert response.status_code == status.HTTP_201_CREATED @@ -180,6 +182,7 @@ def test_create_calendar_schedule_with_shifts(make_organization_and_user_with_to "user_group_id": None, }, "ical_url_overrides": None, + "enable_web_overrides": False, } assert response.status_code == status.HTTP_201_CREATED @@ -227,6 +230,7 @@ def test_update_calendar_schedule( "user_group_id": None, }, "ical_url_overrides": None, + "enable_web_overrides": False, } assert response.status_code == status.HTTP_200_OK @@ -236,6 +240,45 @@ def test_update_calendar_schedule( assert response.json() == result +@pytest.mark.django_db +def test_update_calendar_schedule_enable_web_overrides( + make_organization_and_user_with_token, + make_schedule, +): + organization, user, token = make_organization_and_user_with_token() + client = APIClient() + + schedule = make_schedule( + organization, + schedule_class=OnCallScheduleCalendar, + ) + + url = reverse("api-public:schedules-detail", kwargs={"pk": schedule.public_primary_key}) + + data = { + "enable_web_overrides": True, + } + response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + + result = { + "id": schedule.public_primary_key, + "team_id": None, + "name": schedule.name, + "type": "calendar", + "time_zone": "UTC", + "on_call_now": [], + "shifts": [], + "slack": {"channel_id": None, "user_group_id": None}, + "ical_url_overrides": None, + "enable_web_overrides": True, + } + + assert response.status_code == status.HTTP_200_OK + schedule.refresh_from_db() + assert schedule.enable_web_overrides + assert response.json() == result + + @pytest.mark.django_db def test_get_web_schedule( make_organization_and_user_with_token, @@ -363,6 +406,7 @@ def test_update_ical_url_overrides_calendar_schedule( "user_group_id": None, }, "ical_url_overrides": ICAL_URL, + "enable_web_overrides": False, } assert response.status_code == status.HTTP_200_OK @@ -418,6 +462,7 @@ def test_update_calendar_schedule_with_custom_event( "user_group_id": None, }, "ical_url_overrides": None, + "enable_web_overrides": False, } assert response.status_code == status.HTTP_200_OK @@ -732,6 +777,7 @@ def test_get_schedule_list( "shifts": [], "slack": {"channel_id": slack_channel_id, "user_group_id": user_group_id}, "ical_url_overrides": None, + "enable_web_overrides": False, }, { "id": schedule_ical.public_primary_key, From a898835eb40c2f19889bfbb0c77ba2dcaea0b413 Mon Sep 17 00:00:00 2001 From: Matvey Kukuy Date: Wed, 27 Sep 2023 14:18:00 +0300 Subject: [PATCH 09/14] Fixing ratelimit texts --- engine/apps/integrations/mixins/ratelimit_mixin.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/engine/apps/integrations/mixins/ratelimit_mixin.py b/engine/apps/integrations/mixins/ratelimit_mixin.py index 9256d4b3..808b4276 100644 --- a/engine/apps/integrations/mixins/ratelimit_mixin.py +++ b/engine/apps/integrations/mixins/ratelimit_mixin.py @@ -180,14 +180,12 @@ class IntegrationRateLimitMixin(RateLimitMixin, View): "because too many alerts were sent from your {integration} integration. " "Rate-limiting is activated so you will continue to receive alerts from other integrations. " "Read more about rate limits in our docs. " - "To increase your capacity, reach out to our support team." ) TEXT_WORKSPACE = ( "Rate-limiting has been applied to your account " "because too many alerts were sent from multiple integrations. " "Read more about rate limits in our docs. " - "To increase your capacity, reach out to our support team." ) @ratelimit( From bba6eb333ee9861ff0c918f8355bf45a2aa533ce Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Wed, 27 Sep 2023 09:35:52 -0300 Subject: [PATCH 10/14] Add db indexes to user table (#3067) Add composite indexes based on existing queries/usage, ensuring partial index prefixes are useful too. - `is_active` filtering is set in the default `User` manager - most of our user queries are per `organization` - multiple cases filter by `username` or `email` (most notably schedule related queries, given the low-level backend ical representation) Also rework how users are fetched from DB when getting users from schedules ical representation (which was particularly slow when regex filtering by required permission). Related to https://github.com/grafana/oncall-private/issues/2163 --- CHANGELOG.md | 4 +++ engine/apps/schedules/ical_utils.py | 25 ++++++++++--------- .../migrations/0015_auto_20230926_2203.py | 21 ++++++++++++++++ engine/apps/user_management/models/user.py | 4 +++ 4 files changed, 42 insertions(+), 12 deletions(-) create mode 100644 engine/apps/user_management/migrations/0015_auto_20230926_2203.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 55151194..c59e1908 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix regression in public actions endpoint handling user field by @mderynck ([#3053](https://github.com/grafana/oncall/pull/3053)) +### Changed + +- Rework how users are fetched from DB when getting users from schedules ical representation ([#3067](https://github.com/grafana/oncall/pull/3067)) + ## v1.3.38 (2023-09-19) ### Fixed diff --git a/engine/apps/schedules/ical_utils.py b/engine/apps/schedules/ical_utils.py index cdd02ffc..285ad176 100644 --- a/engine/apps/schedules/ical_utils.py +++ b/engine/apps/schedules/ical_utils.py @@ -67,11 +67,10 @@ IcalEvents = typing.List[IcalEvent] def users_in_ical( usernames_from_ical: typing.List[str], organization: "Organization", -) -> "UserQuerySet": +) -> typing.List["User"]: """ This method returns a sequence of `User` objects, filtered by users whose username, or case-insensitive e-mail, - is present in `usernames_from_ical`. If `include_viewers` is set to `True`, users are further filtered down - based on their granted permissions. + is present in `usernames_from_ical`. Parameters ---------- @@ -80,24 +79,26 @@ def users_in_ical( organization : apps.user_management.models.organization.Organization The organization in question """ - from apps.user_management.models import User + required_permission = RBACPermission.Permissions.SCHEDULES_WRITE emails_from_ical = [username.lower() for username in usernames_from_ical] - # users_found_in_ical = organization.users users_found_in_ical = organization.users.filter( - **User.build_permissions_query(RBACPermission.Permissions.SCHEDULES_WRITE, organization) - ) - - users_found_in_ical = users_found_in_ical.filter( (Q(username__in=usernames_from_ical) | Q(email__lower__in=emails_from_ical)) ).distinct() - return users_found_in_ical + if organization.is_rbac_permissions_enabled: + # it is more efficient to check permissions on the subset of users filtered above + # than performing a regex query for the required permission + users_found_in_ical = [u for u in users_found_in_ical if {"action": required_permission.value} in u.permissions] + else: + users_found_in_ical = users_found_in_ical.filter(role__lte=required_permission.fallback_role.value) + + return list(users_found_in_ical) @timed_lru_cache(timeout=100) -def memoized_users_in_ical(usernames_from_ical: typing.List[str], organization: "Organization") -> UserQuerySet: +def memoized_users_in_ical(usernames_from_ical: typing.List[str], organization: "Organization") -> typing.List["User"]: # using in-memory cache instead of redis to avoid pickling python objects return users_in_ical(usernames_from_ical, organization) @@ -354,7 +355,7 @@ def list_users_to_notify_from_ical_for_period( schedule: "OnCallSchedule", start_datetime: datetime.datetime, end_datetime: datetime.datetime, -) -> UserQuerySet: +) -> typing.List["User"]: users_found_in_ical: typing.Sequence["User"] = [] events = schedule.final_events(start_datetime, end_datetime) usernames = [] diff --git a/engine/apps/user_management/migrations/0015_auto_20230926_2203.py b/engine/apps/user_management/migrations/0015_auto_20230926_2203.py new file mode 100644 index 00000000..15ce31e5 --- /dev/null +++ b/engine/apps/user_management/migrations/0015_auto_20230926_2203.py @@ -0,0 +1,21 @@ +# Generated by Django 3.2.20 on 2023-09-26 22:03 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('user_management', '0014_auto_20230728_0802'), + ] + + operations = [ + migrations.AddIndex( + model_name='user', + index=models.Index(fields=['is_active', 'organization', 'username'], name='user_manage_is_acti_385fc4_idx'), + ), + migrations.AddIndex( + model_name='user', + index=models.Index(fields=['is_active', 'organization', 'email'], name='user_manage_is_acti_7e930d_idx'), + ), + ] diff --git a/engine/apps/user_management/models/user.py b/engine/apps/user_management/models/user.py index 2043f0f9..8c05bb62 100644 --- a/engine/apps/user_management/models/user.py +++ b/engine/apps/user_management/models/user.py @@ -169,6 +169,10 @@ class User(models.Model): # Including is_active to unique_together and setting is_active to None allows to # have multiple deleted users with the same user_id, but user_id is unique among active users unique_together = ("user_id", "organization", "is_active") + indexes = [ + models.Index(fields=["is_active", "organization", "username"]), + models.Index(fields=["is_active", "organization", "email"]), + ] public_primary_key = models.CharField( max_length=20, From b5a8b8b1687df75eabc51e2952f9d6d517ae7d99 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Wed, 27 Sep 2023 07:22:52 -0600 Subject: [PATCH 11/14] Add webhook presets (#2996) # What this PR does Add a system similar to how we select integrations when creating webhooks so that the user has a description of what webhookds do and does not have to write complex templates for common webhook use cases. Presets allow us to create the contents of the webhooks in code and define which fields are controlled by the preset. Some specifics: - Newly created webhooks must choose between Simple, Advanced or another predefined system - Simple is always an escalation step and will post the entire payload to the given URL - Advanced is the same as no preset which is our current view where all fields are available - There are no changes for all existing webhooks with empty preset fields - Once a webhook is created with a preset the preset cannot be changed - Fields in the webhook that are populated by code will give a validation error if they are modified - In the public API webhooks with presets are returned for viewing but cannot be created or modified. This restriction is in place because the Web UI provides the context for which fields to use with a preset. The public API is for interacting with webhooks where all fields are defined. To define a preset create a file with metadata and an override function. The metadata drives validation and what to display in the UI. There are two functions one is connected to the pre_save hook of the Webhook model for persistent changes, the other replaces parameters at execution time for ephemeral changes. See the simple and advanced presets as an example. The file must be listed in settings in `INSTALLED_WEBHOOK_PRESETS` to be enabled at runtime.. ## Which issue(s) this PR fixes ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --------- Co-authored-by: Joey Orlando --- CHANGELOG.md | 1 + docs/sources/outgoing-webhooks/_index.md | 6 +- engine/apps/api/serializers/webhook.py | 81 +++- engine/apps/api/tests/test_webhook_presets.py | 161 ++++++++ engine/apps/api/tests/test_webhooks.py | 74 +++- engine/apps/api/views/webhooks.py | 8 + .../apps/public_api/serializers/webhooks.py | 11 + engine/apps/public_api/tests/test_webhooks.py | 70 ++++ .../migrations/0011_auto_20230920_1813.py | 23 ++ engine/apps/webhooks/models/webhook.py | 5 +- engine/apps/webhooks/presets/__init__.py | 0 engine/apps/webhooks/presets/advanced.py | 19 + engine/apps/webhooks/presets/preset.py | 36 ++ .../apps/webhooks/presets/preset_options.py | 30 ++ engine/apps/webhooks/presets/simple.py | 32 ++ engine/apps/webhooks/tasks/trigger_webhook.py | 9 +- .../webhooks/tests/test_webhook_presets.py | 160 ++++++++ engine/conftest.py | 10 + engine/settings/base.py | 5 + .../CommonWebhookPresetIcons.config.tsx | 3 + .../OutgoingWebhookForm.config.tsx | 387 ++++++++++-------- .../OutgoingWebhookForm.module.css | 30 +- .../OutgoingWebhookForm.tsx | 176 ++++++-- .../WebhookPresetIcons.config.tsx | 5 + .../outgoing_webhook/outgoing_webhook.ts | 11 +- .../outgoing_webhook.types.ts | 9 + .../outgoing_webhooks/OutgoingWebhooks.tsx | 2 +- .../src/state/rootBaseStore/index.ts | 1 + 28 files changed, 1150 insertions(+), 215 deletions(-) create mode 100644 engine/apps/api/tests/test_webhook_presets.py create mode 100644 engine/apps/webhooks/migrations/0011_auto_20230920_1813.py create mode 100644 engine/apps/webhooks/presets/__init__.py create mode 100644 engine/apps/webhooks/presets/advanced.py create mode 100644 engine/apps/webhooks/presets/preset.py create mode 100644 engine/apps/webhooks/presets/preset_options.py create mode 100644 engine/apps/webhooks/presets/simple.py create mode 100644 engine/apps/webhooks/tests/test_webhook_presets.py create mode 100644 grafana-plugin/src/containers/OutgoingWebhookForm/CommonWebhookPresetIcons.config.tsx create mode 100644 grafana-plugin/src/containers/OutgoingWebhookForm/WebhookPresetIcons.config.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index c59e1908..71eef3ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Presets for webhooks @mderynck ([#2996](https://github.com/grafana/oncall/pull/2996)) - Unify breadcrumbs behaviour with other Grafana Apps and main core ([#1906](https://github.com/grafana/oncall/issues/1906)) - Add `enable_web_overrides` option to schedules public API ([#3062](https://github.com/grafana/oncall/pull/3062)) diff --git a/docs/sources/outgoing-webhooks/_index.md b/docs/sources/outgoing-webhooks/_index.md index 19cbf10c..8469466e 100644 --- a/docs/sources/outgoing-webhooks/_index.md +++ b/docs/sources/outgoing-webhooks/_index.md @@ -30,8 +30,10 @@ Jinja2 templates to customize the request being sent. ## Creating an outgoing webhook To create an outgoing webhook navigate to **Outgoing Webhooks** and click **+ Create**. On this screen outgoing -webhooks can be viewed, edited and deleted. To create the outgoing webhook populate the required fields and -click **Create Webhook** +webhooks can be viewed, edited and deleted. To create the outgoing webhook click **New Outgoing Webhook** and then +select a preset based on what you want to do. A simple webhook will POST alert group data as a selectable escalation +step to the specified url. If you require more customization use the advanced webhook which provides all of the +fields described below. ### Outgoing webhook fields diff --git a/engine/apps/api/serializers/webhook.py b/engine/apps/api/serializers/webhook.py index 2a38200c..832292ce 100644 --- a/engine/apps/api/serializers/webhook.py +++ b/engine/apps/api/serializers/webhook.py @@ -4,7 +4,8 @@ from rest_framework import serializers from rest_framework.validators import UniqueTogetherValidator from apps.webhooks.models import Webhook, WebhookResponse -from apps.webhooks.models.webhook import WEBHOOK_FIELD_PLACEHOLDER +from apps.webhooks.models.webhook import PUBLIC_WEBHOOK_HTTP_METHODS, WEBHOOK_FIELD_PLACEHOLDER +from apps.webhooks.presets.preset_options import WebhookPresetOptions from common.api_helpers.custom_fields import TeamPrimaryKeyRelatedField from common.api_helpers.utils import CurrentOrganizationDefault, CurrentTeamDefault, CurrentUserDefault from common.jinja_templater import apply_jinja_template @@ -31,9 +32,9 @@ class WebhookSerializer(serializers.ModelSerializer): organization = serializers.HiddenField(default=CurrentOrganizationDefault()) team = TeamPrimaryKeyRelatedField(allow_null=True, default=CurrentTeamDefault()) user = serializers.HiddenField(default=CurrentUserDefault()) - trigger_type = serializers.CharField(required=True) forward_all = serializers.BooleanField(allow_null=True, required=False) last_response_log = serializers.SerializerMethodField() + trigger_type = serializers.CharField(allow_null=True) trigger_type_name = serializers.SerializerMethodField() class Meta: @@ -59,11 +60,8 @@ class WebhookSerializer(serializers.ModelSerializer): "trigger_type_name", "last_response_log", "integration_filter", + "preset", ] - extra_kwargs = { - "name": {"required": True, "allow_null": False, "allow_blank": False}, - "url": {"required": True, "allow_null": False, "allow_blank": False}, - } validators = [UniqueTogetherValidator(queryset=Webhook.objects.all(), fields=["name", "organization"])] @@ -78,6 +76,16 @@ class WebhookSerializer(serializers.ModelSerializer): def to_internal_value(self, data): webhook = self.instance + # Some fields are conditionally required, add none values for missing required fields + if webhook and webhook.preset and "preset" not in data: + data["preset"] = webhook.preset + for key in ["url", "http_method", "trigger_type"]: + if key not in data: + if self.instance: + data[key] = getattr(self.instance, key) + else: + data[key] = None + # If webhook is being copied instance won't exist to copy values from if not webhook and "id" in data: webhook = Webhook.objects.get( @@ -111,10 +119,29 @@ class WebhookSerializer(serializers.ModelSerializer): return self._validate_template_field(headers) def validate_url(self, url): + if self.is_field_controlled("url"): + return url + if not url: - return None + raise serializers.ValidationError(detail="This field is required.") return self._validate_template_field(url) + def validate_http_method(self, http_method): + if self.is_field_controlled("http_method"): + return http_method + + if http_method not in PUBLIC_WEBHOOK_HTTP_METHODS: + raise serializers.ValidationError(detail=f"This field must be one of {PUBLIC_WEBHOOK_HTTP_METHODS}.") + return http_method + + def validate_trigger_type(self, trigger_type): + if self.is_field_controlled("trigger_type"): + return trigger_type + + if not trigger_type or int(trigger_type) not in Webhook.ALL_TRIGGER_TYPES: + raise serializers.ValidationError(detail="This field is required.") + return trigger_type + def validate_data(self, data): if not data: return None @@ -125,6 +152,29 @@ class WebhookSerializer(serializers.ModelSerializer): return False return data + def validate_preset(self, preset): + if self.instance and self.instance.preset != preset: + raise serializers.ValidationError(detail="This field once set cannot be modified.") + + if preset: + if preset not in WebhookPresetOptions.WEBHOOK_PRESETS: + raise serializers.ValidationError(detail=f"{preset} is not a valid preset id.") + + preset_metadata = WebhookPresetOptions.WEBHOOK_PRESETS[preset].metadata + for controlled_field in preset_metadata.controlled_fields: + if controlled_field in self.initial_data: + if self.instance: + if self.initial_data[controlled_field] != getattr(self.instance, controlled_field): + raise serializers.ValidationError( + detail=f"{controlled_field} is controlled by preset, cannot update" + ) + elif self.initial_data[controlled_field] is not None: + raise serializers.ValidationError( + detail=f"{controlled_field} is controlled by preset, cannot create" + ) + + return preset + def get_last_response_log(self, obj): return WebhookResponseSerializer(obj.responses.all().last()).data @@ -133,3 +183,20 @@ class WebhookSerializer(serializers.ModelSerializer): if obj.trigger_type is not None: trigger_type_name = Webhook.TRIGGER_TYPES[int(obj.trigger_type)][1] return trigger_type_name + + def is_field_controlled(self, field_name): + if self.instance: + if not self.instance.preset: + return False + elif "preset" not in self.initial_data: + return False + + preset_id = self.instance.preset if self.instance else self.initial_data["preset"] + if preset_id: + if preset_id not in WebhookPresetOptions.WEBHOOK_PRESETS: + raise serializers.ValidationError(detail=f"unknown preset {preset_id} referenced") + + preset = WebhookPresetOptions.WEBHOOK_PRESETS[preset_id] + if field_name not in preset.metadata.controlled_fields: + return False + return True diff --git a/engine/apps/api/tests/test_webhook_presets.py b/engine/apps/api/tests/test_webhook_presets.py new file mode 100644 index 00000000..e87f7587 --- /dev/null +++ b/engine/apps/api/tests/test_webhook_presets.py @@ -0,0 +1,161 @@ +import pytest +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient + +from apps.webhooks.models import Webhook +from apps.webhooks.models.webhook import WEBHOOK_FIELD_PLACEHOLDER +from apps.webhooks.tests.test_webhook_presets import ( + TEST_WEBHOOK_LOGO, + TEST_WEBHOOK_PRESET_CONTROLLED_FIELDS, + TEST_WEBHOOK_PRESET_DESCRIPTION, + TEST_WEBHOOK_PRESET_ID, + TEST_WEBHOOK_PRESET_NAME, + TEST_WEBHOOK_PRESET_URL, +) + + +@pytest.mark.django_db +def test_get_webhook_preset_options( + make_organization_and_user_with_plugin_token, webhook_preset_api_setup, make_user_auth_headers +): + organization, user, token = make_organization_and_user_with_plugin_token() + client = APIClient() + url = reverse("api-internal:webhooks-preset-options") + + response = client.get(url, format="json", **make_user_auth_headers(user, token)) + + assert response.status_code == status.HTTP_200_OK + assert response.data[0]["id"] == TEST_WEBHOOK_PRESET_ID + assert response.data[0]["name"] == TEST_WEBHOOK_PRESET_NAME + assert response.data[0]["logo"] == TEST_WEBHOOK_LOGO + assert response.data[0]["description"] == TEST_WEBHOOK_PRESET_DESCRIPTION + assert response.data[0]["controlled_fields"] == TEST_WEBHOOK_PRESET_CONTROLLED_FIELDS + + +@pytest.mark.django_db +def test_create_webhook_from_preset( + make_organization_and_user_with_plugin_token, webhook_preset_api_setup, make_user_auth_headers +): + organization, user, token = make_organization_and_user_with_plugin_token() + client = APIClient() + url = reverse("api-internal:webhooks-list") + + data = { + "name": "the_webhook", + "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, + "team": None, + "password": "secret_password", + "preset": TEST_WEBHOOK_PRESET_ID, + } + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + webhook = Webhook.objects.get(public_primary_key=response.data["id"]) + + expected_response = data | { + "id": webhook.public_primary_key, + "url": TEST_WEBHOOK_PRESET_URL, + "data": organization.org_title, + "username": None, + "password": WEBHOOK_FIELD_PLACEHOLDER, + "authorization_header": None, + "forward_all": True, + "headers": None, + "http_method": "GET", + "integration_filter": None, + "is_webhook_enabled": True, + "is_legacy": False, + "last_response_log": { + "request_data": "", + "request_headers": "", + "timestamp": None, + "content": "", + "status_code": None, + "request_trigger": "", + "url": "", + "event_data": "", + }, + "trigger_template": None, + "trigger_type": str(data["trigger_type"]), + "trigger_type_name": "Alert Group Created", + } + + assert response.status_code == status.HTTP_201_CREATED + assert response.json() == expected_response + assert webhook.password == data["password"] + + +@pytest.mark.django_db +def test_invalid_create_webhook_with_preset( + make_organization_and_user_with_plugin_token, webhook_preset_api_setup, make_user_auth_headers +): + organization, user, token = make_organization_and_user_with_plugin_token() + client = APIClient() + url = reverse("api-internal:webhooks-list") + + data = { + "name": "the_webhook", + "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, + "url": "https://test12345.com", + "preset": TEST_WEBHOOK_PRESET_ID, + } + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["preset"][0] == "url is controlled by preset, cannot create" + + +@pytest.mark.django_db +def test_update_webhook_from_preset( + make_organization_and_user_with_plugin_token, webhook_preset_api_setup, make_user_auth_headers, make_custom_webhook +): + organization, user, token = make_organization_and_user_with_plugin_token() + webhook = make_custom_webhook( + name="the_webhook", + organization=organization, + trigger_type=Webhook.TRIGGER_ALERT_GROUP_CREATED, + preset=TEST_WEBHOOK_PRESET_ID, + ) + + client = APIClient() + url = reverse("api-internal:webhooks-detail", kwargs={"pk": webhook.public_primary_key}) + + data = { + "name": "the_webhook 2", + } + response = client.put(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_200_OK + assert response.json()["name"] == data["name"] + + webhook.refresh_from_db() + assert webhook.name == data["name"] + assert webhook.url == TEST_WEBHOOK_PRESET_URL + assert webhook.http_method == "GET" + assert webhook.data == organization.org_title + + +@pytest.mark.django_db +def test_invalid_update_webhook_from_preset( + make_organization_and_user_with_plugin_token, webhook_preset_api_setup, make_user_auth_headers, make_custom_webhook +): + organization, user, token = make_organization_and_user_with_plugin_token() + webhook = make_custom_webhook( + name="the_webhook", + organization=organization, + trigger_type=Webhook.TRIGGER_ALERT_GROUP_CREATED, + preset=TEST_WEBHOOK_PRESET_ID, + ) + + client = APIClient() + url = reverse("api-internal:webhooks-detail", kwargs={"pk": webhook.public_primary_key}) + + data = { + "preset": "some_other_preset", + } + response = client.put(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["preset"][0] == "This field once set cannot be modified." + + data = { + "data": "some_other_data", + } + response = client.put(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST diff --git a/engine/apps/api/tests/test_webhooks.py b/engine/apps/api/tests/test_webhooks.py index e6c2624d..867e366f 100644 --- a/engine/apps/api/tests/test_webhooks.py +++ b/engine/apps/api/tests/test_webhooks.py @@ -66,6 +66,7 @@ def test_get_list_webhooks(webhook_internal_api_setup, make_user_auth_headers): "trigger_template": None, "trigger_type": "0", "trigger_type_name": "Escalation step", + "preset": None, } ] @@ -108,6 +109,7 @@ def test_get_detail_webhook(webhook_internal_api_setup, make_user_auth_headers): "trigger_template": None, "trigger_type": "0", "trigger_type_name": "Escalation step", + "preset": None, } response = client.get(url, format="json", **make_user_auth_headers(user, token)) @@ -124,7 +126,8 @@ def test_create_webhook(webhook_internal_api_setup, make_user_auth_headers): data = { "name": "the_webhook", "url": TEST_URL, - "trigger_type": str(Webhook.TRIGGER_ALERT_GROUP_CREATED), + "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, + "http_method": "POST", "team": None, } response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) @@ -152,7 +155,9 @@ def test_create_webhook(webhook_internal_api_setup, make_user_auth_headers): "event_data": "", }, "trigger_template": None, + "trigger_type": str(data["trigger_type"]), "trigger_type_name": "Alert Group Created", + "preset": None, } assert response.status_code == status.HTTP_201_CREATED assert response.json() == expected_response @@ -179,7 +184,8 @@ def test_create_valid_templated_field(webhook_internal_api_setup, make_user_auth "name": "webhook_with_valid_data", "url": TEST_URL, field_name: value, - "trigger_type": str(Webhook.TRIGGER_ALERT_GROUP_CREATED), + "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, + "http_method": "POST", "team": None, } @@ -209,7 +215,9 @@ def test_create_valid_templated_field(webhook_internal_api_setup, make_user_auth "event_data": "", }, "trigger_template": None, + "trigger_type": str(data["trigger_type"]), "trigger_type_name": "Alert Group Created", + "preset": None, } # update expected value for changed field expected_response[field_name] = value @@ -236,7 +244,8 @@ def test_create_invalid_templated_field(webhook_internal_api_setup, make_user_au "name": "webhook_with_valid_data", "url": TEST_URL, field_name: value, - "trigger_type": str(Webhook.TRIGGER_ALERT_GROUP_CREATED), + "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, + "http_method": "POST", "team": None, } @@ -253,7 +262,8 @@ def test_update_webhook(webhook_internal_api_setup, make_user_auth_headers): data = { "name": "github_button_updated", "url": "https://github.com/", - "trigger_type": str(Webhook.TRIGGER_ALERT_GROUP_CREATED), + "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, + "http_method": "POST", "team": None, } response = client.put( @@ -547,7 +557,8 @@ def test_webhook_field_masking(webhook_internal_api_setup, make_user_auth_header data = { "name": "the_webhook", "url": TEST_URL, - "trigger_type": str(Webhook.TRIGGER_ALERT_GROUP_CREATED), + "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, + "http_method": "POST", "team": None, "password": "secret_password", "authorization_header": "auth 1234", @@ -579,7 +590,9 @@ def test_webhook_field_masking(webhook_internal_api_setup, make_user_auth_header "event_data": "", }, "trigger_template": None, + "trigger_type": str(data["trigger_type"]), "trigger_type_name": "Alert Group Created", + "preset": None, } assert response.status_code == status.HTTP_201_CREATED @@ -598,7 +611,8 @@ def test_webhook_copy(webhook_internal_api_setup, make_user_auth_headers): data = { "name": "the_webhook", "url": TEST_URL, - "trigger_type": str(Webhook.TRIGGER_ALERT_GROUP_CREATED), + "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, + "http_method": "POST", "team": None, "password": "secret_password", "authorization_header": "auth 1234", @@ -635,7 +649,9 @@ def test_webhook_copy(webhook_internal_api_setup, make_user_auth_headers): "event_data": "", }, "trigger_template": None, + "trigger_type": str(data["trigger_type"]), "trigger_type_name": "Alert Group Created", + "preset": None, } assert response3.status_code == status.HTTP_201_CREATED @@ -644,3 +660,49 @@ def test_webhook_copy(webhook_internal_api_setup, make_user_auth_headers): assert webhook.authorization_header == data["authorization_header"] assert webhook.id != to_copy["id"] assert webhook.user == user + + +@pytest.mark.django_db +def test_create_invalid_missing_fields(webhook_internal_api_setup, make_user_auth_headers): + user, token, webhook = webhook_internal_api_setup + client = APIClient() + url = reverse("api-internal:webhooks-list") + + data = {"url": TEST_URL, "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, "http_method": "POST"} + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["name"][0] == "This field is required." + + data = {"name": "test webhook 1", "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, "http_method": "POST"} + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["url"][0] == "This field is required." + + data = {"name": "test webhook 2", "url": TEST_URL, "http_method": "POST"} + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["trigger_type"][0] == "This field is required." + + data = { + "name": "test webhook 3", + "url": TEST_URL, + "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, + } + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["http_method"][0] == "This field must be one of ['GET', 'POST', 'PUT', 'DELETE', 'OPTIONS']." + + data = { + "name": "test webhook 3", + "url": TEST_URL, + "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, + "http_method": "TOAST", + } + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["http_method"][0] == "This field must be one of ['GET', 'POST', 'PUT', 'DELETE', 'OPTIONS']." + + data = {"name": "test webhook 3", "url": TEST_URL, "trigger_type": 2000000, "http_method": "POST"} + response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["trigger_type"][0] == "This field is required." diff --git a/engine/apps/api/views/webhooks.py b/engine/apps/api/views/webhooks.py index f3674cf2..bd7dc8d7 100644 --- a/engine/apps/api/views/webhooks.py +++ b/engine/apps/api/views/webhooks.py @@ -1,4 +1,5 @@ import json +from dataclasses import asdict from django.core.exceptions import ObjectDoesNotExist from django_filters import rest_framework as filters @@ -14,6 +15,7 @@ from apps.api.permissions import RBACPermission from apps.api.serializers.webhook import WebhookResponseSerializer, WebhookSerializer from apps.auth_token.auth import PluginAuthentication from apps.webhooks.models import Webhook, WebhookResponse +from apps.webhooks.presets.preset_options import WebhookPresetOptions from apps.webhooks.utils import apply_jinja_template_for_json from common.api_helpers.exceptions import BadRequest from common.api_helpers.filters import ByTeamModelFieldFilterMixin, ModelFieldFilterMixin, TeamModelMultipleChoiceFilter @@ -52,6 +54,7 @@ class WebhooksView(TeamFilteringMixin, PublicPrimaryKeyMixin, ModelViewSet): "destroy": [RBACPermission.Permissions.OUTGOING_WEBHOOKS_WRITE], "responses": [RBACPermission.Permissions.OUTGOING_WEBHOOKS_READ], "preview_template": [RBACPermission.Permissions.OUTGOING_WEBHOOKS_WRITE], + "preset_options": [RBACPermission.Permissions.OUTGOING_WEBHOOKS_READ], } model = Webhook @@ -179,3 +182,8 @@ class WebhooksView(TeamFilteringMixin, PublicPrimaryKeyMixin, ModelViewSet): response = {"preview": result} return Response(response, status=status.HTTP_200_OK) + + @action(methods=["get"], detail=False) + def preset_options(self, request): + result = [asdict(preset) for preset in WebhookPresetOptions.WEBHOOK_PRESET_CHOICES] + return Response(result) diff --git a/engine/apps/public_api/serializers/webhooks.py b/engine/apps/public_api/serializers/webhooks.py index 70f3d6a4..eb25a53c 100644 --- a/engine/apps/public_api/serializers/webhooks.py +++ b/engine/apps/public_api/serializers/webhooks.py @@ -12,6 +12,8 @@ from common.api_helpers.utils import CurrentOrganizationDefault, CurrentTeamDefa from common.jinja_templater import apply_jinja_template from common.jinja_templater.apply_jinja_template import JinjaTemplateError, JinjaTemplateWarning +PRESET_VALIDATION_MESSAGE = "Preset webhooks must be modified through web UI" + INTEGRATION_FILTER_MESSAGE = "integration_filter must be a list of valid integration ids" @@ -73,6 +75,7 @@ class WebhookCreateSerializer(serializers.ModelSerializer): "http_method", "trigger_type", "integration_filter", + "preset", ] extra_kwargs = { "name": {"required": True, "allow_null": False, "allow_blank": False}, @@ -157,6 +160,14 @@ class WebhookCreateSerializer(serializers.ModelSerializer): raise serializers.ValidationError(INTEGRATION_FILTER_MESSAGE) return integration_filter + def validate_preset(self, preset): + raise serializers.ValidationError(PRESET_VALIDATION_MESSAGE) + + def validate(self, data): + if self.instance and self.instance.preset: + raise serializers.ValidationError(PRESET_VALIDATION_MESSAGE) + return data + class WebhookUpdateSerializer(WebhookCreateSerializer): trigger_type = WebhookTriggerTypeField(required=False) diff --git a/engine/apps/public_api/tests/test_webhooks.py b/engine/apps/public_api/tests/test_webhooks.py index efe7870f..0c9f9b78 100644 --- a/engine/apps/public_api/tests/test_webhooks.py +++ b/engine/apps/public_api/tests/test_webhooks.py @@ -5,7 +5,9 @@ from django.urls import reverse from rest_framework import status from rest_framework.test import APIClient +from apps.public_api.serializers.webhooks import PRESET_VALIDATION_MESSAGE from apps.webhooks.models import Webhook +from apps.webhooks.tests.test_webhook_presets import TEST_WEBHOOK_PRESET_ID def _get_expected_result(webhook): @@ -25,6 +27,7 @@ def _get_expected_result(webhook): "http_method": webhook.http_method, "trigger_type": Webhook.PUBLIC_TRIGGER_TYPES_MAP[webhook.trigger_type], "integration_filter": webhook.integration_filter, + "preset": webhook.preset, } @@ -357,3 +360,70 @@ def test_webhook_validate_integration_filters( assert response.status_code == 200 assert response.data["integration_filter"] == data["integration_filter"] assert webhook.integration_filter == data["integration_filter"] + + +@pytest.mark.django_db +def test_get_webhook_with_preset( + make_organization_and_user_with_token, + make_custom_webhook, + webhook_preset_api_setup, +): + organization, user, token = make_organization_and_user_with_token() + client = APIClient() + + webhook = make_custom_webhook(organization=organization, preset=TEST_WEBHOOK_PRESET_ID) + url = reverse("api-public:webhooks-list") + response = client.get(url, format="json", HTTP_AUTHORIZATION=f"{token}") + + expected_payload = { + "count": 1, + "next": None, + "previous": None, + "results": [_get_expected_result(webhook)], + "current_page_number": 1, + "page_size": 50, + "total_pages": 1, + } + + assert response.status_code == status.HTTP_200_OK + assert response.data == expected_payload + + +@pytest.mark.django_db +def test_webhook_block_preset_create( + make_organization_and_user_with_token, + webhook_preset_api_setup, +): + organization, user, token = make_organization_and_user_with_token() + client = APIClient() + url = reverse("api-public:webhooks-list") + + data = { + "name": "Test outgoing webhook with nested data", + "trigger_type": "acknowledge", + "preset": TEST_WEBHOOK_PRESET_ID, + } + + response = client.post(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["preset"][0] == PRESET_VALIDATION_MESSAGE + + +@pytest.mark.django_db +def test_webhook_block_preset_update( + make_organization_and_user_with_token, + make_custom_webhook, + webhook_preset_api_setup, +): + organization, user, token = make_organization_and_user_with_token() + client = APIClient() + webhook = make_custom_webhook(organization=organization, preset=TEST_WEBHOOK_PRESET_ID) + webhook.refresh_from_db() + + url = reverse("api-public:webhooks-detail", kwargs={"pk": webhook.public_primary_key}) + data = { + "name": "Test rename preset webhook", + } + response = client.put(url, data=data, format="json", HTTP_AUTHORIZATION=f"{token}") + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["non_field_errors"][0] == PRESET_VALIDATION_MESSAGE diff --git a/engine/apps/webhooks/migrations/0011_auto_20230920_1813.py b/engine/apps/webhooks/migrations/0011_auto_20230920_1813.py new file mode 100644 index 00000000..76fbcd5f --- /dev/null +++ b/engine/apps/webhooks/migrations/0011_auto_20230920_1813.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.20 on 2023-09-20 18:13 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('webhooks', '0010_alter_webhook_trigger_type'), + ] + + operations = [ + migrations.AddField( + model_name='webhook', + name='preset', + field=models.CharField(blank=True, default=None, max_length=100, null=True), + ), + migrations.AlterField( + model_name='webhook', + name='http_method', + field=models.CharField(default='POST', max_length=32, null=True), + ), + ] diff --git a/engine/apps/webhooks/models/webhook.py b/engine/apps/webhooks/models/webhook.py index fb8956af..eee3a367 100644 --- a/engine/apps/webhooks/models/webhook.py +++ b/engine/apps/webhooks/models/webhook.py @@ -94,6 +94,8 @@ class Webhook(models.Model): (TRIGGER_UNACKNOWLEDGE, "Unacknowledged"), ) + ALL_TRIGGER_TYPES = [i[0] for i in TRIGGER_TYPES] + PUBLIC_TRIGGER_TYPES_MAP = { TRIGGER_ESCALATION_STEP: "escalation", TRIGGER_ALERT_GROUP_CREATED: "alert group created", @@ -137,11 +139,12 @@ class Webhook(models.Model): url = models.TextField(null=True, default=None) data = models.TextField(null=True, default=None) forward_all = models.BooleanField(default=True) - http_method = models.CharField(max_length=32, default="POST") + http_method = models.CharField(max_length=32, default="POST", null=True) trigger_type = models.IntegerField(choices=TRIGGER_TYPES, default=TRIGGER_ESCALATION_STEP, null=True) is_webhook_enabled = models.BooleanField(null=True, default=True) integration_filter = models.JSONField(default=None, null=True, blank=True) is_legacy = models.BooleanField(null=True, default=False) + preset = models.CharField(max_length=100, null=True, blank=True, default=None) class Meta: unique_together = ("name", "organization") diff --git a/engine/apps/webhooks/presets/__init__.py b/engine/apps/webhooks/presets/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/engine/apps/webhooks/presets/advanced.py b/engine/apps/webhooks/presets/advanced.py new file mode 100644 index 00000000..1983943e --- /dev/null +++ b/engine/apps/webhooks/presets/advanced.py @@ -0,0 +1,19 @@ +from apps.webhooks.models import Webhook +from apps.webhooks.presets.preset import WebhookPreset, WebhookPresetMetadata + + +class AdvancedWebhookPreset(WebhookPreset): + def _metadata(self) -> WebhookPresetMetadata: + return WebhookPresetMetadata( + id="advanced_webhook", + name="Advanced", + logo="webhook", + description="An advanced webhook with all available settings and template options.", + controlled_fields=[], + ) + + def override_parameters_before_save(self, webhook: Webhook): + pass + + def override_parameters_at_runtime(self, webhook: Webhook): + pass diff --git a/engine/apps/webhooks/presets/preset.py b/engine/apps/webhooks/presets/preset.py new file mode 100644 index 00000000..8e946476 --- /dev/null +++ b/engine/apps/webhooks/presets/preset.py @@ -0,0 +1,36 @@ +from abc import ABC, abstractmethod +from dataclasses import dataclass +from typing import List + +from django.utils.functional import cached_property + +from apps.webhooks.models import Webhook + + +@dataclass +class WebhookPresetMetadata: + id: str + name: str + logo: str + description: str + controlled_fields: List[str] + + +class WebhookPreset(ABC): + @cached_property + def metadata(self) -> WebhookPresetMetadata: + return self._metadata() + + @abstractmethod + def _metadata(self) -> WebhookPresetMetadata: + raise NotImplementedError + + @abstractmethod + def override_parameters_before_save(self, webhook: Webhook): + """Implement this to write parameters before the webhook is saved to the database""" + pass + + @abstractmethod + def override_parameters_at_runtime(self, webhook: Webhook): + """Implement this to write parameters before the webhook is executed (These will not be persisted)""" + pass diff --git a/engine/apps/webhooks/presets/preset_options.py b/engine/apps/webhooks/presets/preset_options.py new file mode 100644 index 00000000..cc267f11 --- /dev/null +++ b/engine/apps/webhooks/presets/preset_options.py @@ -0,0 +1,30 @@ +from importlib import import_module + +from django.conf import settings +from django.db.models.signals import pre_save +from django.dispatch import receiver + +from apps.webhooks.models import Webhook + + +class WebhookPresetOptions: + WEBHOOK_PRESETS = {} + for webhook_preset_config in settings.INSTALLED_WEBHOOK_PRESETS: + module_path, class_name = webhook_preset_config.rsplit(".", 1) + module = import_module(module_path) + preset = getattr(module, class_name)() + WEBHOOK_PRESETS[preset.metadata.id] = preset + + WEBHOOK_PRESET_CHOICES = [webhook_preset.metadata for webhook_preset in WEBHOOK_PRESETS.values()] + + +@receiver(pre_save, sender=Webhook) +def listen_for_webhook_save(sender: Webhook, instance: Webhook, raw: bool, *args, **kwargs) -> None: + if instance.preset: + if instance.preset in WebhookPresetOptions.WEBHOOK_PRESETS: + WebhookPresetOptions.WEBHOOK_PRESETS[instance.preset].override_parameters_before_save(instance) + else: + raise NotImplementedError(f"Webhook references unknown preset implementation {instance.preset}") + + +pre_save.connect(listen_for_webhook_save, Webhook) diff --git a/engine/apps/webhooks/presets/simple.py b/engine/apps/webhooks/presets/simple.py new file mode 100644 index 00000000..dc1db970 --- /dev/null +++ b/engine/apps/webhooks/presets/simple.py @@ -0,0 +1,32 @@ +from apps.webhooks.models import Webhook +from apps.webhooks.presets.preset import WebhookPreset, WebhookPresetMetadata + + +class SimpleWebhookPreset(WebhookPreset): + def _metadata(self) -> WebhookPresetMetadata: + return WebhookPresetMetadata( + id="simple_webhook", + name="Simple", + logo="webhook", + description="A simple webhook which sends the alert group data to a given URL. Triggered as an escalation step.", + controlled_fields=[ + "trigger_type", + "http_method", + "integration_filter", + "headers", + "username", + "password", + "authorization_header", + "trigger_template", + "forward_all", + "data", + ], + ) + + def override_parameters_before_save(self, webhook: Webhook): + webhook.http_method = "POST" + webhook.trigger_type = Webhook.TRIGGER_ESCALATION_STEP + webhook.forward_all = True + + def override_parameters_at_runtime(self, webhook: Webhook): + pass diff --git a/engine/apps/webhooks/tasks/trigger_webhook.py b/engine/apps/webhooks/tasks/trigger_webhook.py index 579624bf..68bfdf22 100644 --- a/engine/apps/webhooks/tasks/trigger_webhook.py +++ b/engine/apps/webhooks/tasks/trigger_webhook.py @@ -11,6 +11,7 @@ from apps.base.models import UserNotificationPolicyLogRecord from apps.user_management.models import User from apps.webhooks.models import Webhook, WebhookResponse from apps.webhooks.models.webhook import WEBHOOK_FIELD_PLACEHOLDER +from apps.webhooks.presets.preset_options import WebhookPresetOptions from apps.webhooks.utils import ( InvalidWebhookData, InvalidWebhookHeaders, @@ -116,6 +117,12 @@ def make_request(webhook, alert_group, data): exception = error = None try: + if webhook.preset: + if webhook.preset not in WebhookPresetOptions.WEBHOOK_PRESETS: + raise Exception(f"Invalid preset {webhook.preset}") + else: + WebhookPresetOptions.WEBHOOK_PRESETS[webhook.preset].override_parameters_at_runtime(webhook) + if not webhook.check_integration_filter(alert_group): status["request_trigger"] = NOT_FROM_SELECTED_INTEGRATION return False, status, None, None @@ -168,7 +175,7 @@ def execute_webhook(webhook_pk, alert_group_id, user_id, escalation_policy_id): try: webhook = Webhook.objects.get(pk=webhook_pk) except Webhook.DoesNotExist: - logger.warn(f"Webhook {webhook_pk} does not exist") + logger.warning(f"Webhook {webhook_pk} does not exist") return try: diff --git a/engine/apps/webhooks/tests/test_webhook_presets.py b/engine/apps/webhooks/tests/test_webhook_presets.py new file mode 100644 index 00000000..70c95151 --- /dev/null +++ b/engine/apps/webhooks/tests/test_webhook_presets.py @@ -0,0 +1,160 @@ +from unittest.mock import patch + +import pytest + +from apps.webhooks.models import Webhook +from apps.webhooks.presets.preset import WebhookPreset, WebhookPresetMetadata +from apps.webhooks.tasks.trigger_webhook import make_request +from apps.webhooks.tests.test_trigger_webhook import MockResponse + +TEST_WEBHOOK_PRESET_URL = "https://test123.com" +TEST_WEBHOOK_PRESET_NAME = "Test Webhook" +TEST_WEBHOOK_PRESET_ID = "test_webhook" +TEST_WEBHOOK_LOGO = "test_logo" +TEST_WEBHOOK_PRESET_DESCRIPTION = "Description of test webhook preset" +TEST_WEBHOOK_PRESET_CONTROLLED_FIELDS = ["url", "http_method", "data", "authorization_header"] +TEST_WEBHOOK_AUTHORIZATION_HEADER = "Test Auth header 12345" +INVALID_PRESET_ID = "invalid_preset_id" + + +class TestWebhookPreset(WebhookPreset): + def _metadata(self) -> WebhookPresetMetadata: + return WebhookPresetMetadata( + id=TEST_WEBHOOK_PRESET_ID, + name=TEST_WEBHOOK_PRESET_NAME, + logo=TEST_WEBHOOK_LOGO, + description=TEST_WEBHOOK_PRESET_DESCRIPTION, + controlled_fields=TEST_WEBHOOK_PRESET_CONTROLLED_FIELDS, + ) + + def override_parameters_before_save(self, webhook: Webhook): + webhook.data = webhook.organization.org_title + webhook.url = TEST_WEBHOOK_PRESET_URL + webhook.http_method = "GET" + + def override_parameters_at_runtime(self, webhook: Webhook): + webhook.authorization_header = TEST_WEBHOOK_AUTHORIZATION_HEADER + + +@pytest.mark.django_db +def test_create_webhook_from_preset(make_organization, webhook_preset_api_setup, make_custom_webhook): + organization = make_organization() + webhook = make_custom_webhook( + name="the_webhook", + organization=organization, + trigger_type=Webhook.TRIGGER_ALERT_GROUP_CREATED, + preset=TEST_WEBHOOK_PRESET_ID, + ) + + webhook.refresh_from_db() + assert webhook.url == TEST_WEBHOOK_PRESET_URL + assert webhook.http_method == "GET" + assert webhook.data == organization.org_title + assert webhook.authorization_header is None + + +@pytest.mark.django_db +def test_create_webhook_from_invalid_preset(make_organization, webhook_preset_api_setup, make_custom_webhook): + organization = make_organization() + expected = None + try: + make_custom_webhook( + name="the_webhook", + organization=organization, + trigger_type=Webhook.TRIGGER_ALERT_GROUP_CREATED, + preset=INVALID_PRESET_ID, + ) + except NotImplementedError as e: + expected = e + + assert expected.args[0] == f"Webhook references unknown preset implementation {INVALID_PRESET_ID}" + + +@pytest.mark.django_db +def test_update_webhook_from_preset(make_organization, webhook_preset_api_setup, make_custom_webhook): + organization = make_organization() + webhook = make_custom_webhook( + name="the_webhook", + organization=organization, + trigger_type=Webhook.TRIGGER_ALERT_GROUP_CREATED, + preset=TEST_WEBHOOK_PRESET_ID, + ) + + webhook.refresh_from_db() + webhook.http_method = "POST" + webhook.save() + + webhook.refresh_from_db() + assert webhook.http_method == "GET" + + +@pytest.mark.django_db +def test_update_webhook_from_invalid_preset(make_organization, webhook_preset_api_setup, make_custom_webhook): + organization = make_organization() + webhook = make_custom_webhook( + name="the_webhook", + organization=organization, + trigger_type=Webhook.TRIGGER_ALERT_GROUP_CREATED, + preset=TEST_WEBHOOK_PRESET_ID, + ) + webhook.refresh_from_db() + webhook.preset = INVALID_PRESET_ID + + try: + webhook.save() + except NotImplementedError as e: + expected = e + + assert expected.args[0] == f"Webhook references unknown preset implementation {INVALID_PRESET_ID}" + + webhook.refresh_from_db() + assert webhook.preset == TEST_WEBHOOK_PRESET_ID + + +@pytest.mark.django_db +def test_webhook_preset_runtime_override(make_organization, webhook_preset_api_setup, make_custom_webhook): + organization = make_organization() + webhook = make_custom_webhook( + name="the_webhook", + organization=organization, + trigger_type=Webhook.TRIGGER_ALERT_GROUP_CREATED, + preset=TEST_WEBHOOK_PRESET_ID, + ) + + with patch.object(webhook, "build_url"): + response = MockResponse() + with patch.object(webhook, "make_request", return_value=response) as mock_make_request: + triggered, webhook_status, error, exception = make_request(webhook, None, None) + assert mock_make_request.call_args.args[1]["headers"]["Authorization"] == TEST_WEBHOOK_AUTHORIZATION_HEADER + assert triggered + assert error is None + assert exception is None + + webhook.refresh_from_db() + assert webhook.authorization_header is None + + +@pytest.mark.django_db +def test_webhook_invalid_preset_runtime_override(make_organization, webhook_preset_api_setup, make_custom_webhook): + organization = make_organization() + webhook = make_custom_webhook( + name="the_webhook", + organization=organization, + trigger_type=Webhook.TRIGGER_ALERT_GROUP_CREATED, + ) + webhook.refresh_from_db() + + expected_error = f"Invalid preset {INVALID_PRESET_ID}" + Webhook.objects.filter(id=webhook.id).update(preset=INVALID_PRESET_ID) + webhook.refresh_from_db() + with patch.object(webhook, "build_url"): + with patch.object(webhook, "make_request") as mock_make_request: + triggered, webhook_status, error, exception = make_request(webhook, None, None) + mock_make_request.assert_not_called() + assert triggered + assert webhook_status["content"] == expected_error + assert error == expected_error + assert exception.args[0] == expected_error + + webhook.refresh_from_db() + assert webhook.authorization_header is None diff --git a/engine/conftest.py b/engine/conftest.py index aca32ffe..270b1ec2 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -86,7 +86,9 @@ from apps.telegram.tests.factories import ( ) from apps.user_management.models.user import User, listen_for_user_model_save from apps.user_management.tests.factories import OrganizationFactory, RegionFactory, TeamFactory, UserFactory +from apps.webhooks.presets.preset_options import WebhookPresetOptions from apps.webhooks.tests.factories import CustomWebhookFactory, WebhookResponseFactory +from apps.webhooks.tests.test_webhook_presets import TEST_WEBHOOK_PRESET_ID, TestWebhookPreset register(OrganizationFactory) register(UserFactory) @@ -907,3 +909,11 @@ def shift_swap_request_setup( return ssr, beneficiary, benefactor return _shift_swap_request_setup + + +@pytest.fixture() +def webhook_preset_api_setup(): + WebhookPresetOptions.WEBHOOK_PRESETS = {TEST_WEBHOOK_PRESET_ID: TestWebhookPreset()} + WebhookPresetOptions.WEBHOOK_PRESET_CHOICES = [ + preset.metadata for preset in WebhookPresetOptions.WEBHOOK_PRESETS.values() + ] diff --git a/engine/settings/base.py b/engine/settings/base.py index 3c1d1694..b8023ca9 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -723,6 +723,11 @@ INSTALLED_ONCALL_INTEGRATIONS = [ "config_integrations.direct_paging", ] +INSTALLED_WEBHOOK_PRESETS = [ + "apps.webhooks.presets.simple.SimpleWebhookPreset", + "apps.webhooks.presets.advanced.AdvancedWebhookPreset", +] + if IS_OPEN_SOURCE: INSTALLED_APPS += ["apps.oss_installation", "apps.zvonok"] # noqa diff --git a/grafana-plugin/src/containers/OutgoingWebhookForm/CommonWebhookPresetIcons.config.tsx b/grafana-plugin/src/containers/OutgoingWebhookForm/CommonWebhookPresetIcons.config.tsx new file mode 100644 index 00000000..5ce2cc07 --- /dev/null +++ b/grafana-plugin/src/containers/OutgoingWebhookForm/CommonWebhookPresetIcons.config.tsx @@ -0,0 +1,3 @@ +import { ReactElement } from 'react'; + +export const commonWebhookPresetIconsConfig: { [id: string]: () => ReactElement } = {}; diff --git a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.config.tsx b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.config.tsx index 78fab274..d299ca94 100644 --- a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.config.tsx +++ b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.config.tsx @@ -4,6 +4,7 @@ import { SelectableValue } from '@grafana/data'; import Emoji from 'react-emoji-render'; import { FormItem, FormItemType } from 'components/GForm/GForm.types'; +import { OutgoingWebhookPreset } from 'models/outgoing_webhook/outgoing_webhook.types'; import { KeyValuePair } from 'utils'; import { generateAssignToTeamInputDescription } from 'utils/consts'; @@ -18,182 +19,226 @@ export const WebhookTriggerType = { Unacknowledged: new KeyValuePair('7', 'Unacknowledged'), }; -export const form: { name: string; fields: FormItem[] } = { - name: 'OutgoingWebhook', - fields: [ - { - name: 'name', - type: FormItemType.Input, - validation: { required: true }, - }, - { - name: 'is_webhook_enabled', - label: 'Enabled', - normalize: (value) => Boolean(value), - type: FormItemType.Switch, - }, - { - name: 'team', - label: 'Assign to Team', - description: `${generateAssignToTeamInputDescription( - 'Outgoing Webhooks' - )} This setting does not effect execution of the webhook.`, - type: FormItemType.GSelect, - extra: { - modelName: 'grafanaTeamStore', - displayField: 'name', - valueField: 'id', - showSearch: true, - allowClear: true, +export function createForm(presets: OutgoingWebhookPreset[]): { name: string; fields: FormItem[] } { + return { + name: 'OutgoingWebhook', + fields: [ + { + name: 'name', + type: FormItemType.Input, + validation: { required: true }, }, - }, - { - name: 'trigger_type', - label: 'Trigger Type', - description: 'The type of event which will cause this webhook to execute.', - type: FormItemType.Select, - extra: { - options: [ - { - value: WebhookTriggerType.EscalationStep.key, - label: WebhookTriggerType.EscalationStep.value, - }, - { - value: WebhookTriggerType.AlertGroupCreated.key, - label: WebhookTriggerType.AlertGroupCreated.value, - }, - { - value: WebhookTriggerType.Acknowledged.key, - label: WebhookTriggerType.Acknowledged.value, - }, - { - value: WebhookTriggerType.Resolved.key, - label: WebhookTriggerType.Resolved.value, - }, - { - value: WebhookTriggerType.Silenced.key, - label: WebhookTriggerType.Silenced.value, - }, - { - value: WebhookTriggerType.Unsilenced.key, - label: WebhookTriggerType.Unsilenced.value, - }, - { - value: WebhookTriggerType.Unresolved.key, - label: WebhookTriggerType.Unresolved.value, - }, - { - value: WebhookTriggerType.Unacknowledged.key, - label: WebhookTriggerType.Unacknowledged.value, - }, - ], + { + name: 'is_webhook_enabled', + label: 'Enabled', + normalize: (value) => Boolean(value), + type: FormItemType.Switch, }, - validation: { required: true }, - normalize: (value) => value, - }, - { - name: 'http_method', - label: 'HTTP Method', - type: FormItemType.Select, - extra: { - options: [ - { - value: 'GET', - label: 'GET', - }, - { - value: 'POST', - label: 'POST', - }, - { - value: 'PUT', - label: 'PUT', - }, - { - value: 'DELETE', - label: 'DELETE', - }, - { - value: 'OPTIONS', - label: 'OPTIONS', - }, - ], + { + name: 'team', + label: 'Assign to Team', + description: `${generateAssignToTeamInputDescription( + 'Outgoing Webhooks' + )} This setting does not effect execution of the webhook.`, + type: FormItemType.GSelect, + extra: { + modelName: 'grafanaTeamStore', + displayField: 'name', + valueField: 'id', + showSearch: true, + allowClear: true, + placeholder: 'Choose (Optional)', + }, }, - validation: { required: true }, - normalize: (value) => value, - }, - { - name: 'integration_filter', - label: 'Integrations', - type: FormItemType.MultiSelect, - isVisible: (data) => { - return data.trigger_type !== WebhookTriggerType.EscalationStep.key; + { + name: 'trigger_type', + label: 'Trigger Type', + description: 'The type of event which will cause this webhook to execute.', + type: FormItemType.Select, + extra: { + placeholder: 'Choose (Required)', + options: [ + { + value: WebhookTriggerType.EscalationStep.key, + label: WebhookTriggerType.EscalationStep.value, + }, + { + value: WebhookTriggerType.AlertGroupCreated.key, + label: WebhookTriggerType.AlertGroupCreated.value, + }, + { + value: WebhookTriggerType.Acknowledged.key, + label: WebhookTriggerType.Acknowledged.value, + }, + { + value: WebhookTriggerType.Resolved.key, + label: WebhookTriggerType.Resolved.value, + }, + { + value: WebhookTriggerType.Silenced.key, + label: WebhookTriggerType.Silenced.value, + }, + { + value: WebhookTriggerType.Unsilenced.key, + label: WebhookTriggerType.Unsilenced.value, + }, + { + value: WebhookTriggerType.Unresolved.key, + label: WebhookTriggerType.Unresolved.value, + }, + { + value: WebhookTriggerType.Unacknowledged.key, + label: WebhookTriggerType.Unacknowledged.value, + }, + ], + }, + isVisible: (data) => { + return isPresetFieldVisible(data.preset, presets, 'trigger_type'); + }, + normalize: (value) => value, }, - extra: { - modelName: 'alertReceiveChannelStore', - displayField: 'verbal_name', - valueField: 'id', - showSearch: true, - getOptionLabel: (item: SelectableValue) => , + { + name: 'http_method', + label: 'HTTP Method', + type: FormItemType.Select, + extra: { + placeholder: 'Choose (Required)', + options: [ + { + value: 'GET', + label: 'GET', + }, + { + value: 'POST', + label: 'POST', + }, + { + value: 'PUT', + label: 'PUT', + }, + { + value: 'DELETE', + label: 'DELETE', + }, + { + value: 'OPTIONS', + label: 'OPTIONS', + }, + ], + }, + isVisible: (data) => isPresetFieldVisible(data.preset, presets, 'http_method'), + normalize: (value) => value, }, - validation: { required: true }, - description: - 'Integrations that this webhook applies to. If this is empty the webhook will execute for all integrations', - }, - { - name: 'url', - label: 'Webhook URL', - type: FormItemType.Monaco, - validation: { required: true }, - extra: { - height: 30, + { + name: 'integration_filter', + label: 'Integrations', + type: FormItemType.MultiSelect, + isVisible: (data) => { + return ( + isPresetFieldVisible(data.preset, presets, 'integration_filter') && + data.trigger_type !== WebhookTriggerType.EscalationStep.key + ); + }, + extra: { + placeholder: 'Choose (Optional)', + modelName: 'alertReceiveChannelStore', + displayField: 'verbal_name', + valueField: 'id', + showSearch: true, + getOptionLabel: (item: SelectableValue) => , + }, + description: + 'Integrations that this webhook applies to. If this is empty the webhook will execute for all integrations', }, - }, - { - name: 'headers', - label: 'Webhook Headers', - description: 'Request headers should be in JSON format.', - type: FormItemType.Monaco, - extra: { - rows: 3, + { + name: 'url', + label: 'Webhook URL', + type: FormItemType.Monaco, + extra: { + height: 30, + }, + isVisible: (data) => { + return isPresetFieldVisible(data.preset, presets, 'url'); + }, }, - }, - { - name: 'username', - type: FormItemType.Input, - }, - { - name: 'password', - type: FormItemType.Password, - }, - { - name: 'authorization_header', - description: - 'Value of the Authorization header, do not need to prefix with "Authorization:". For example: Bearer AbCdEf123456', - type: FormItemType.Password, - }, - { - name: 'trigger_template', - type: FormItemType.Monaco, - description: - 'Trigger template is used to conditionally execute the webhook based on incoming data. The trigger template must be empty or evaluate to true or 1 for the webhook to be sent', - extra: { - rows: 2, + { + name: 'headers', + label: 'Webhook Headers', + description: 'Request headers should be in JSON format.', + type: FormItemType.Monaco, + extra: { + rows: 3, + }, + isVisible: (data) => { + return isPresetFieldVisible(data.preset, presets, 'headers'); + }, }, - }, - { - name: 'forward_all', - normalize: (value) => Boolean(value), - type: FormItemType.Switch, - description: "Forwards whole payload of the alert group and context data to the webhook's url as POST/PUT data", - }, - { - name: 'data', - getDisabled: (data) => Boolean(data?.forward_all), - type: FormItemType.Monaco, - description: - 'Available variables: {{ event }}, {{ user }}, {{ alert_group }}, {{ alert_group_id }}, {{ alert_payload }}, {{ integration }}, {{ notified_users }}, {{ users_to_be_notified }}, {{ responses }}', - extra: {}, - }, - ], -}; + { + name: 'username', + type: FormItemType.Input, + isVisible: (data) => { + return isPresetFieldVisible(data.preset, presets, 'username'); + }, + }, + { + name: 'password', + type: FormItemType.Password, + isVisible: (data) => { + return isPresetFieldVisible(data.preset, presets, 'password'); + }, + }, + { + name: 'authorization_header', + description: + 'Value of the Authorization header, do not need to prefix with "Authorization:". For example: Bearer AbCdEf123456', + type: FormItemType.Password, + isVisible: (data) => { + return isPresetFieldVisible(data.preset, presets, 'authorization_header'); + }, + }, + { + name: 'trigger_template', + type: FormItemType.Monaco, + description: + 'Trigger template is used to conditionally execute the webhook based on incoming data. The trigger template must be empty or evaluate to true or 1 for the webhook to be sent', + extra: { + rows: 2, + }, + isVisible: (data) => { + return isPresetFieldVisible(data.preset, presets, 'trigger_template'); + }, + }, + { + name: 'forward_all', + normalize: (value) => (value ? Boolean(value) : value), + type: FormItemType.Switch, + description: "Forwards whole payload of the alert group and context data to the webhook's url as POST/PUT data", + isVisible: (data) => { + return isPresetFieldVisible(data.preset, presets, 'forward_all'); + }, + }, + { + name: 'data', + getDisabled: (data) => Boolean(data?.forward_all), + type: FormItemType.Monaco, + description: + 'Available variables: {{ event }}, {{ user }}, {{ alert_group }}, {{ alert_group_id }}, {{ alert_payload }}, {{ integration }}, {{ notified_users }}, {{ users_to_be_notified }}, {{ responses }}', + extra: {}, + isVisible: (data) => { + return isPresetFieldVisible(data.preset, presets, 'data'); + }, + }, + ], + }; +} + +function isPresetFieldVisible(presetId: string, presets: OutgoingWebhookPreset[], fieldName: string) { + if (presetId == null) { + return true; + } + const selectedPreset = presets.find((item) => item.id === presetId); + if (selectedPreset && selectedPreset.controlled_fields.includes(fieldName)) { + return false; + } + return true; +} diff --git a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.module.css b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.module.css index a4613c64..3a41fb24 100644 --- a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.module.css +++ b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.module.css @@ -3,7 +3,7 @@ } .title { - margin: 16px 0 0 16px; + margin: 0 0 0 16px; } .content { @@ -28,3 +28,31 @@ .webhooks__drawerContent .cursor.monaco-mouse-cursor-text { display: none !important; } + +.cards { + display: flex; + flex-wrap: wrap; + gap: 24px; + overflow: auto; + scroll-snap-type: y mandatory; + width: 100%; +} + +.card { + width: 100%; + height: 106px; + scroll-snap-align: start; + scroll-snap-stop: normal; + display: flex; + flex-direction: row; + align-items: center; + justify-content: flex-start; + cursor: pointer; + position: relative; + gap: 20px; +} + +.search-integration { + width: 100%; + margin-bottom: 24px; +} diff --git a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.tsx b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.tsx index a006e5d1..61bfc8fe 100644 --- a/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.tsx +++ b/grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.tsx @@ -1,24 +1,39 @@ -import React, { useCallback, useState } from 'react'; +import React, { ChangeEvent, useCallback, useState } from 'react'; -import { Button, ConfirmModal, ConfirmModalProps, Drawer, HorizontalGroup, Tab, TabsBar } from '@grafana/ui'; +import { + Button, + ConfirmModal, + ConfirmModalProps, + Drawer, + EmptySearchResult, + HorizontalGroup, + Input, + Tab, + TabsBar, + VerticalGroup, +} from '@grafana/ui'; import cn from 'classnames/bind'; import { observer } from 'mobx-react'; import { useHistory } from 'react-router-dom'; +import Block from 'components/GBlock/Block'; import GForm from 'components/GForm/GForm'; import { FormItem, FormItemType } from 'components/GForm/GForm.types'; +import IntegrationLogo from 'components/IntegrationLogo/IntegrationLogo'; +import { logoCoors } from 'components/IntegrationLogo/IntegrationLogo.config'; import Text from 'components/Text/Text'; +import { webhookPresetIcons } from 'containers/OutgoingWebhookForm/WebhookPresetIcons.config'; import OutgoingWebhookStatus from 'containers/OutgoingWebhookStatus/OutgoingWebhookStatus'; import WebhooksTemplateEditor from 'containers/WebhooksTemplateEditor/WebhooksTemplateEditor'; import { WithPermissionControlTooltip } from 'containers/WithPermissionControl/WithPermissionControlTooltip'; -import { OutgoingWebhook } from 'models/outgoing_webhook/outgoing_webhook.types'; +import { OutgoingWebhook, OutgoingWebhookPreset } from 'models/outgoing_webhook/outgoing_webhook.types'; import { WebhookFormActionType } from 'pages/outgoing_webhooks/OutgoingWebhooks.types'; import { useStore } from 'state/useStore'; import { KeyValuePair } from 'utils'; import { UserActions } from 'utils/authorization'; import { PLUGIN_ROOT } from 'utils/consts'; -import { form } from './OutgoingWebhookForm.config'; +import { createForm } from './OutgoingWebhookForm.config'; import styles from 'containers/OutgoingWebhookForm/OutgoingWebhookForm.module.css'; @@ -45,10 +60,15 @@ const OutgoingWebhookForm = observer((props: OutgoingWebhookFormProps) => { const [activeTab, setActiveTab] = useState( action === WebhookFormActionType.EDIT_SETTINGS ? WebhookTabs.Settings.key : WebhookTabs.LastRun.key ); + const [showPresetsListDrawer, setShowPresetsListDrawer] = useState(id === 'new'); + const [showCreateWebhookDrawer, setShowCreateWebhookDrawer] = useState(false); + const [selectedPreset, setSelectedPreset] = useState(undefined); + const [filterValue, setFilterValue] = useState(''); const { outgoingWebhookStore } = useStore(); const isNew = action === WebhookFormActionType.NEW; const isNewOrCopy = isNew || action === WebhookFormActionType.COPY; + const form = createForm(outgoingWebhookStore.outgoingWebhookPresets); const handleSubmit = useCallback( (data: Partial) => { @@ -104,10 +124,17 @@ const OutgoingWebhookForm = observer((props: OutgoingWebhookFormProps) => { | { is_webhook_enabled: boolean; is_legacy: boolean; + preset: string; }; if (isNew) { - data = { is_webhook_enabled: true, is_legacy: false }; + data = { + is_webhook_enabled: true, + is_legacy: false, + preset: selectedPreset?.id, + trigger_type: null, + http_method: 'POST', + }; } else if (isNewOrCopy) { data = { ...outgoingWebhookStore.items[id], is_legacy: false, name: '' }; } else { @@ -123,27 +150,69 @@ const OutgoingWebhookForm = observer((props: OutgoingWebhookFormProps) => { } const formElement = ; + const createWebhookParameters = ( + <> + +
{renderWebhookForm()}
+ + {templateToEdit && ( + { + onFormChangeFn?.fn(value); + setTemplateToEdit(undefined); + }} + onHide={() => setTemplateToEdit(undefined)} + template={templateToEdit} + /> + )} + + ); - if (action === WebhookFormActionType.NEW || action === WebhookFormActionType.COPY) { - // show just the creation form, not the tabs + const presets = outgoingWebhookStore.outgoingWebhookPresets.filter((preset: OutgoingWebhookPreset) => + preset.name.toLowerCase().includes(filterValue.toLowerCase()) + ); + + if (action === WebhookFormActionType.NEW) { return ( <> - -
{renderWebhookForm()}
- - {templateToEdit && ( - { - onFormChangeFn?.fn(value); - setTemplateToEdit(undefined); - }} - onHide={() => setTemplateToEdit(undefined)} - template={templateToEdit} - /> + {showPresetsListDrawer && ( + +
+ + + Outgoing webhooks can send alert data to other systems. They can be triggered by various conditions + and can use templates to transform data to fit the recipient system. Presets listed below provide a + starting point to customize these connections. + + + {presets.length > 8 && ( +
+ ) => setFilterValue(e.currentTarget.value)} + /> +
+ )} + + + +
+ )} + {(showCreateWebhookDrawer || !showPresetsListDrawer) && createWebhookParameters} ); + } else if (action === WebhookFormActionType.COPY) { + return createWebhookParameters; } return ( @@ -200,6 +269,12 @@ const OutgoingWebhookForm = observer((props: OutgoingWebhookFormProps) => { ); + function onBlockClick(preset: OutgoingWebhookPreset) { + setSelectedPreset(preset); + setShowCreateWebhookDrawer(true); + setShowPresetsListDrawer(false); + } + function renderWebhookForm() { return ( <> @@ -207,9 +282,21 @@ const OutgoingWebhookForm = observer((props: OutgoingWebhookFormProps) => {
- + {id === 'new' ? ( + + ) : ( + + )} diff --git a/grafana-plugin/src/state/rootBaseStore/index.ts b/grafana-plugin/src/state/rootBaseStore/index.ts index 086e9402..fa9c8c72 100644 --- a/grafana-plugin/src/state/rootBaseStore/index.ts +++ b/grafana-plugin/src/state/rootBaseStore/index.ts @@ -130,6 +130,7 @@ export class RootBaseStore { this.userStore.updateNotificationPolicyOptions(), this.userStore.updateNotifyByOptions(), this.alertReceiveChannelStore.updateAlertReceiveChannelOptions(), + this.outgoingWebhookStore.updateOutgoingWebhookPresets(), this.escalationPolicyStore.updateWebEscalationPolicyOptions(), this.escalationPolicyStore.updateEscalationPolicyOptions(), this.escalationPolicyStore.updateNumMinutesInWindowOptions(), From ab56e53fe81ac9d8aeab5a2c252671c67b88aa25 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Wed, 27 Sep 2023 10:21:55 -0600 Subject: [PATCH 12/14] Allow deletion for removed presets (#3072) If a webhook preset is removed from configuration while there are still existing webhooks referencing it they will have the following behavior: - Webhook can be viewed - Webhook can be deleted - Webhook cannot be modified - Webhook will not execute Removing a preset from configuration effectively disables all existing webhooks referencing it while retaining their data. --- engine/apps/webhooks/presets/preset_options.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/apps/webhooks/presets/preset_options.py b/engine/apps/webhooks/presets/preset_options.py index cc267f11..d765746b 100644 --- a/engine/apps/webhooks/presets/preset_options.py +++ b/engine/apps/webhooks/presets/preset_options.py @@ -20,7 +20,7 @@ class WebhookPresetOptions: @receiver(pre_save, sender=Webhook) def listen_for_webhook_save(sender: Webhook, instance: Webhook, raw: bool, *args, **kwargs) -> None: - if instance.preset: + if instance.preset and not instance.deleted_at: if instance.preset in WebhookPresetOptions.WEBHOOK_PRESETS: WebhookPresetOptions.WEBHOOK_PRESETS[instance.preset].override_parameters_before_save(instance) else: From cfda0c8aaa24c206e24e43bc7701bceadc1473c0 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Wed, 27 Sep 2023 12:43:15 -0600 Subject: [PATCH 13/14] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71eef3ff..db1817f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## Unreleased +## v1.3.39 (2023-09-27) ### Added From 9126f214eb8009efdcff99f95036443e115fe5bb Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Wed, 27 Sep 2023 12:51:45 -0600 Subject: [PATCH 14/14] Update CHANGELOG.md Remove repeated entry --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db1817f7..95f4af44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Presets for webhooks @mderynck ([#2996](https://github.com/grafana/oncall/pull/2996)) -- Unify breadcrumbs behaviour with other Grafana Apps and main core ([#1906](https://github.com/grafana/oncall/issues/1906)) - Add `enable_web_overrides` option to schedules public API ([#3062](https://github.com/grafana/oncall/pull/3062)) ### Fixed