PR Incident Filters UI Component #658
Conversation
WalkthroughThis change introduces support for configuring and managing incident pull request (PR) filters at the team level. It adds a new API endpoint, Redux state management, TypeScript types, UI components, and updates utility functions to handle incident PR filter settings, enabling teams to customize how incident PRs are identified and processed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DoraSettings as DoraMetricsConfigurationSettings
participant Modal as TeamIncidentPRsFilter Modal
participant Redux
participant API
participant Backend
User->>DoraSettings: Click "Incident PR Filter" menu item
DoraSettings->>Modal: Open TeamIncidentPRsFilter modal
Modal->>Redux: Dispatch fetchTeamIncidentPRsFilter (team_id)
Redux->>API: GET /internal/team/{team_id}/incident_prs_filter
API->>Backend: Fetch incident PR filter settings
Backend-->>API: Return settings
API-->>Redux: Return settings
Redux-->>Modal: Provide incident PR filter settings
User->>Modal: Edit and save filters
Modal->>Redux: Dispatch updateTeamIncidentPRsFilter (team_id, setting)
Redux->>API: PUT /internal/team/{team_id}/incident_prs_filter
API->>Backend: Update settings
Backend-->>API: Return updated settings
API-->>Redux: Return updated settings
Redux-->>Modal: Confirm update
Modal->>Redux: Dispatch fetchDoraMetrics (with updated filters)
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (7)
backend/analytics_server/mhq/service/code/pr_filter.py (1)
106-107: Changeiftoeliffor setting type checkThe current implementation uses
iffor both setting type checks, which means both conditions will be checked independently. Since these are mutually exclusive settings, usingelifwould be more appropriate and slightly more efficient.if setting_type == SettingType.EXCLUDED_PRS_SETTING: self._apply_excluded_pr_ids_setting(setting=setting) -if setting_type == SettingType.INCIDENT_PRS_SETTING: +elif setting_type == SettingType.INCIDENT_PRS_SETTING: self._apply_incident_prs_setting(setting=setting)web-server/pages/api/internal/team/[team_id]/incident_prs_filter.ts (1)
1-60: Consider adding error handling for specific error scenarios.While the current implementation leverages the global error handling from handleRequest, consider adding specific error handling for common scenarios like not found or validation failures specific to this endpoint.
endpoint.handle.GET(getSchema, async (req, res) => { const { team_id } = req.payload; - const { setting } = await handleRequest<TeamIncidentPRsSettingApiResponse>( - `/teams/${team_id}/settings`, - { - method: 'GET', - params: { - setting_type: 'INCIDENT_PRS_SETTING' - } - } - ); - return res.send({ setting } as TeamIncidentPRsSettingsResponse); + try { + const { setting } = await handleRequest<TeamIncidentPRsSettingApiResponse>( + `/teams/${team_id}/settings`, + { + method: 'GET', + params: { + setting_type: 'INCIDENT_PRS_SETTING' + } + } + ); + return res.send({ setting } as TeamIncidentPRsSettingsResponse); + } catch (error) { + // Handle specific error cases + if (error.response?.status === 404) { + return res.status(404).send({ + error: 'Incident PR settings not found for this team' + }); + } + throw error; // Let the global error handler deal with other errors + } });web-server/src/slices/team.ts (1)
272-295: Consider adding error handling for more resilient API interactions.While the current implementation leverages the default error handling from handleApi, consider adding specific error handling to provide more meaningful feedback to users.
export const fetchTeamIncidentPRsFilter = createAsyncThunk( 'teams/fetchTeamIncidentPRsFilter', async (params: { team_id: ID }) => { - return await handleApi<TeamIncidentPRsSettingsResponse>( - `/internal/team/${params.team_id}/incident_prs_filter`, - { - method: 'GET' - } - ); + try { + return await handleApi<TeamIncidentPRsSettingsResponse>( + `/internal/team/${params.team_id}/incident_prs_filter`, + { + method: 'GET' + } + ); + } catch (error) { + // Handle specific error cases, such as 404 or 500 + console.error('Failed to fetch incident PR filters:', error); + throw error; // Rethrow to let Redux handle it + } } );web-server/src/components/TeamIncidentPRsFilter.tsx (1)
238-252:Autocompletevalue is updated only on blur – useonChangeto keep the form reactiveRelying solely on
onBlurmeans that selecting an option with a mouse click updates the store only when the input loses focus, leading to surprising UX and possible validation mismatches (the user might press “Save” before blurring).<Autocomplete freeSolo options={regexOptions.map((option) => option.value)} value={filter.value} - onBlur={(e: any) => { - setValue(`setting.filters.${idx}.value`, e.target.value, { - shouldValidate: true, - shouldDirty: true - }); - }} + onChange={(_, newVal) => + setValue(`setting.filters.${idx}.value`, newVal ?? '', { + shouldValidate: true, + shouldDirty: true, + }) + } + onBlur={(e) => + setValue(`setting.filters.${idx}.value`, e.target.value, { + shouldValidate: true, + shouldDirty: true, + }) + }Using both
onChangeandonBlurensures the field stays in sync with user interactions.backend/analytics_server/mhq/service/settings/configuration_settings.py (1)
264-270: Serialising dataclasses back to JSON should useasdictfor symmetry
_adapt_incident_prs_setting_json_datare-assembles the dict manually. IfIncidentPRsSettinggains new attributes (e.g.,match_case), this function will silently drop them. Prefer:- return { - "include_revert_prs": specific_setting.include_revert_prs, - "filters": specific_setting.filters, - } + from dataclasses import asdict + return asdict(specific_setting)Keeps the adapter future-proof and eliminates duplication.
backend/analytics_server/mhq/service/incidents/incidents.py (1)
165-174: Regex extraction helper could short-circuit on invalid patterns
check_regexpresumably validates patterns; if it returnsFalsewe still callre.search, leading tore.error. Add an early bail-out:- if check_regex(regex_pattern): - match = re.search(regex_pattern, text) + if not check_regex(regex_pattern): + return None + match = re.search(regex_pattern, text)Avoids unexpected exceptions.
backend/analytics_server/mhq/api/incidents.py (1)
122-131: Potential runtime type confusion – ensurepr_filteris always a dict before casting
pr_filterarrives asOptional[str]coerced throughjson.loads, but could still beNone.
apply_pr_filterhandlesNone, yet later layers (asdict(pr_filter)inincidents.py) assume a dataclass. Guard early:pr_filter_data = pr_filter or {} pr_filter: PRFilter = apply_pr_filter( pr_filter_data, EntityType.TEAM, team_id, [...] )Prevents accidental
TypeErrorif JSON parsing fails andpr_filterbecomes an unexpected type.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
backend/analytics_server/mhq/api/incidents.py(7 hunks)backend/analytics_server/mhq/api/resources/settings_resource.py(2 hunks)backend/analytics_server/mhq/service/code/pr_filter.py(3 hunks)backend/analytics_server/mhq/service/incidents/incident_filter.py(2 hunks)backend/analytics_server/mhq/service/incidents/incidents.py(4 hunks)backend/analytics_server/mhq/service/incidents/models/adapter.py(1 hunks)backend/analytics_server/mhq/service/settings/configuration_settings.py(7 hunks)backend/analytics_server/mhq/service/settings/default_settings_data.py(1 hunks)backend/analytics_server/mhq/service/settings/models.py(1 hunks)backend/analytics_server/mhq/service/settings/setting_type_validator.py(1 hunks)backend/analytics_server/mhq/store/models/code/filter.py(2 hunks)backend/analytics_server/mhq/store/models/settings/configuration_settings.py(1 hunks)backend/analytics_server/mhq/store/repos/code.py(1 hunks)backend/analytics_server/tests/service/Incidents/test_incident_types_setting.py(1 hunks)backend/analytics_server/tests/service/Incidents/test_mean_time_to_recovery.py(1 hunks)backend/analytics_server/tests/service/Incidents/test_team_pr_incidents.py(1 hunks)web-server/pages/api/internal/team/[team_id]/incident_prs_filter.ts(1 hunks)web-server/src/components/DoraMetricsConfigurationSettings.tsx(4 hunks)web-server/src/components/TeamIncidentPRsFilter.tsx(1 hunks)web-server/src/slices/team.ts(5 hunks)web-server/src/types/resources.ts(1 hunks)web-server/src/utils/cockpitMetricUtils.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (12)
backend/analytics_server/mhq/api/resources/settings_resource.py (1)
backend/analytics_server/mhq/service/settings/models.py (1)
IncidentPRsSetting(56-58)
backend/analytics_server/mhq/service/settings/setting_type_validator.py (1)
backend/analytics_server/mhq/store/models/settings/configuration_settings.py (1)
SettingType(14-22)
backend/analytics_server/mhq/service/settings/default_settings_data.py (1)
backend/analytics_server/mhq/store/models/settings/configuration_settings.py (1)
SettingType(14-22)
web-server/src/components/DoraMetricsConfigurationSettings.tsx (2)
web-server/src/slices/team.ts (1)
fetchTeamIncidentPRsFilter(272-282)web-server/src/components/TeamIncidentPRsFilter.tsx (1)
TeamIncidentPRsFilter(92-377)
backend/analytics_server/mhq/service/incidents/incident_filter.py (3)
backend/analytics_server/mhq/service/settings/models.py (1)
IncidentPRsSetting(56-58)backend/analytics_server/mhq/store/models/incidents/enums.py (1)
IncidentType(29-32)backend/analytics_server/mhq/store/models/settings/configuration_settings.py (1)
SettingType(14-22)
backend/analytics_server/mhq/store/models/code/filter.py (1)
backend/analytics_server/mhq/store/models/code/pull_requests.py (1)
PullRequest(12-76)
backend/analytics_server/mhq/service/incidents/models/adapter.py (2)
backend/analytics_server/mhq/store/models/code/pull_requests.py (1)
PullRequest(12-76)backend/analytics_server/mhq/store/models/incidents/enums.py (2)
IncidentStatus(23-26)IncidentType(29-32)
web-server/src/utils/cockpitMetricUtils.ts (2)
web-server/src/api-helpers/axios.ts (1)
handleRequest(86-96)web-server/src/types/resources.ts (2)
MeanTimeToRestoreApiResponse(538-541)MeanTimeToRestoreApiTrendsResponse(543-546)
backend/analytics_server/mhq/service/code/pr_filter.py (2)
backend/analytics_server/mhq/service/settings/models.py (2)
ExcludedPRsSetting(30-31)IncidentPRsSetting(56-58)backend/analytics_server/mhq/store/models/settings/configuration_settings.py (1)
SettingType(14-22)
web-server/src/slices/team.ts (1)
web-server/src/types/resources.ts (2)
TeamIncidentPRsSettingsResponse(638-640)IncidentPRsSettings(623-629)
backend/analytics_server/mhq/service/settings/configuration_settings.py (2)
backend/analytics_server/mhq/service/settings/models.py (1)
IncidentPRsSetting(56-58)backend/analytics_server/mhq/store/models/settings/configuration_settings.py (1)
SettingType(14-22)
backend/analytics_server/tests/service/Incidents/test_incident_types_setting.py (4)
backend/analytics_server/mhq/service/incidents/incident_filter.py (3)
ConfigurationsIncidentFilterProcessor(65-126)apply(32-42)apply(80-89)backend/analytics_server/mhq/store/models/settings/configuration_settings.py (1)
SettingType(14-22)backend/analytics_server/mhq/service/settings/models.py (2)
IncidentTypesSetting(35-36)IncidentPRsSetting(56-58)backend/analytics_server/mhq/store/models/incidents/enums.py (1)
IncidentType(29-32)
🪛 Biome (1.9.4)
web-server/src/components/TeamIncidentPRsFilter.tsx
[error] 185-185: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: All file linting
🔇 Additional comments (34)
backend/analytics_server/mhq/store/repos/code.py (1)
310-327: Implementation looks clean and follows established patterns.The new method
get_prs_merged_in_interval_by_numbersis well-structured and consistent with other similar methods in the class. It properly reuses existing filtering methods and adds specific filtering by PR numbers. The method signature includes appropriate type hints and follows the repository's pattern of using the@rollback_on_excdecorator for error handling.backend/analytics_server/mhq/store/models/settings/configuration_settings.py (1)
20-20: LGTM - Addition follows enum pattern.The new
INCIDENT_PRS_SETTINGenum value is correctly added to theSettingTypeenum, following the established naming convention.backend/analytics_server/mhq/service/settings/default_settings_data.py (1)
32-36: Default configuration is appropriately structured.The default configuration for the incident PR settings initializes with sensible defaults - not including revert PRs and starting with an empty filters list. This follows the pattern established for other setting types in this function.
backend/analytics_server/mhq/service/settings/setting_type_validator.py (1)
22-23: Validator implementation is consistent with existing pattern.The addition of validation support for the
INCIDENT_PRS_SETTINGtype follows the established pattern in the function and ensures proper handling of the new setting type.backend/analytics_server/tests/service/Incidents/test_mean_time_to_recovery.py (1)
19-19: Updated constructor call for compatibility with new dependencies.The test has been correctly updated to reflect the new
IncidentServiceconstructor signature, which now requires three arguments instead of two. This change aligns with the extended service dependency onCodeRepoServicefor incident PR filtering functionality.Also applies to: 27-27
backend/analytics_server/mhq/api/resources/settings_resource.py (2)
8-8: Added import for new IncidentPRsSetting class.The import statement has been added correctly to support the new incident PR filtering functionality.
59-64: Support added for IncidentPRsSetting in API response serialization.The implementation correctly adds the
include_revert_prsboolean andfilterslist to the response when handling instances ofIncidentPRsSetting. This follows the established pattern for other setting types in this file.web-server/src/components/DoraMetricsConfigurationSettings.tsx (5)
6-6: Import added for TeamIncidentPRsFilter component.The import statement has been correctly added to support the new UI for configuring incident PR filters.
13-16: Updated imports for team-related actions.The imports have been refactored to use named imports in a cleaner format, adding the new action for fetching incident PR filters.
27-27: Added dispatch to fetch incident PR filter settings.This dispatch ensures that incident PR filter settings are loaded when the component mounts, similar to the existing pattern for production branches.
40-47: Added callback for opening the incident PR filter modal.The implementation correctly follows the existing pattern for modal creation, providing a consistent user experience with other configuration modals.
90-97: Added menu item for configuring incident PR filters.The new menu item is correctly implemented, following the same pattern as the existing menu item for production branches.
backend/analytics_server/mhq/service/settings/models.py (2)
49-53: New dataclass IncidentPRFilter defined.This dataclass is well-structured for representing a key-value pair for regex filters, with appropriate field types for string values.
55-58: New dataclass IncidentPRsSetting defined.This setting class extends
BaseSettingand includes appropriate fields for managing incident PR filtering options, with proper typing for the boolean flag and list of filters.backend/analytics_server/mhq/store/models/code/filter.py (2)
16-16: Addition ofincident_pr_filterslooks goodThe new attribute allows for storing a list of dictionaries that will be used for filtering PRs related to incidents.
102-102: LGTM - Addingincident_pr_filtersto conditions dictionaryProperly adds the new query condition to the existing conditions dictionary.
backend/analytics_server/mhq/service/incidents/incident_filter.py (3)
7-7: LGTM - ImportedIncidentPRsSettingcorrectlyAppropriate import added for the new setting type that will be used in the filter processing.
10-10: LGTM - ImportedIncidentTypefor filtering logicAdded the necessary import for the enum that will be used in the filtering logic.
112-125: LGTM - Good implementation of revert PR filtering based on settingsThe implementation correctly filters out the
REVERT_PRincident type when theinclude_revert_prsflag is set toFalsein theIncidentPRsSetting. The code also properly handles type checking to ensure the setting is of the expected type.backend/analytics_server/mhq/service/code/pr_filter.py (2)
4-4: LGTM - Added import for newIncidentPRsSettingCorrectly added import for the new setting type.
117-120: LGTM - Proper implementation of applying incident PR filtersThe method correctly appends the filters from the setting to the PR filter's incident_pr_filters list, initializing it if it was previously None.
web-server/src/types/resources.ts (3)
623-629: Well-structured type definition for incident PR filters.The
IncidentPRsSettingstype is well-defined with clear fields that capture both the inclusion of revert PRs and custom filters with field/value pairs.
631-636: Type follows API response pattern consistently.The API response type includes standard fields (timestamps, team_id) alongside the settings payload, maintaining consistency with other similar response types in the codebase.
638-640: Simplified response type looks good.This simplified response type focuses only on the settings data, appropriate for frontend consumption after API processing.
web-server/pages/api/internal/team/[team_id]/incident_prs_filter.ts (3)
10-25: Strong validation schemas for API inputs.The Yup schemas effectively validate both the path parameters and request body, ensuring proper data structure and types. The nested schema for filters ensures each filter has required field and value properties.
27-41: GET handler implementation is clean and follows best practices.The implementation properly extracts the team_id from the validated request payload, makes an appropriate backend request with setting_type parameter, and returns a well-structured response.
43-58: PUT handler follows consistent pattern for settings updates.The implementation correctly extracts both team_id and setting from the validated request payload, makes the appropriate backend request with setting_type and setting_data, and returns a properly structured response.
web-server/src/utils/cockpitMetricUtils.ts (2)
114-115: Successfully integrated PR filters into MTTR metrics.The changes correctly incorporate the PR filter parameter into all mean time to restore API requests, ensuring consistent filtering across current/previous stats and trends. This maintains consistency with how filters are applied to other metrics in the file.
Also applies to: 120-123, 129-130, 136-137
101-102: Consistent parameter extraction for PR filter.The PR filter is correctly extracted from the parameters alongside the existing time objects.
web-server/src/slices/team.ts (4)
17-19: Added new types for PR filter functionality.Imports have been properly updated to include the necessary types for incident PR filters.
37-37: State management extended for incident PR filters.The state definition and initial state values have been properly updated to support the new incident PR filters feature.
Also applies to: 53-53
121-136: Redux reducers follow established patterns.The implementation adds new reducer cases for incident PR filter operations following the existing pattern with addFetchCasesToReducer, maintaining consistency.
272-295: Well-implemented thunks for incident PR filter operations.The thunks for fetching and updating incident PR filters follow Redux best practices and maintain consistency with existing code patterns.
backend/analytics_server/mhq/service/incidents/incidents.py (1)
148-151:pr_filtervsresolution_prs_filter– verify intentYou compute
resolution_prs_filter(which already mergesEXCLUDED_PRS_SETTING&INCIDENT_PRS_SETTING) but then pass the originalpr_filtertoget_prs_merged_in_interval_by_numbers.
If the original came from the API it lacksINCIDENT_PRS_SETTING, producing asymmetric results.Please confirm which filter should be applied and adjust accordingly.
backend/analytics_server/mhq/service/incidents/models/adapter.py
Outdated
Show resolved
Hide resolved
backend/analytics_server/tests/service/Incidents/test_team_pr_incidents.py
Outdated
Show resolved
Hide resolved
backend/analytics_server/tests/service/Incidents/test_team_pr_incidents.py
Outdated
Show resolved
Hide resolved
backend/analytics_server/tests/service/Incidents/test_incident_types_setting.py
Outdated
Show resolved
Hide resolved
backend/analytics_server/mhq/service/settings/configuration_settings.py
Outdated
Show resolved
Hide resolved
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
web-server/src/components/TeamIncidentPRsFilter.tsx (4)
247-256: Use onChange instead of onInputChange for better control.The
onInputChangefires on every keystroke including when navigating options. For better control and consistency, useonChange:<Autocomplete freeSolo options={regexOptions.map((option) => option.value)} value={filter.value} - onInputChange={(_, value) => { + onChange={(_, value) => { - setValue(`setting.filters.${idx}.value`, value, { + setValue(`setting.filters.${idx}.value`, value || '', { shouldValidate: true, shouldDirty: true }); }}
291-295: Avoid usingall: 'unset'style reset.Using
style={{ all: 'unset' }}removes all default styles which could break accessibility and expected behavior. Consider using MUI's styling system:return ( <ListItem {...props} key={option} - style={{ all: 'unset' }} + disablePadding + sx={{ display: 'block' }} >
367-375: Extract complex disabled logic for better readability.The button's disabled condition is complex and hard to parse. Consider extracting it:
+ const isEmptyFilter = settings.filters.length === 1 && + !settings.filters[0].field && + !settings.filters[0].value; + const isSaveDisabled = !isDirty || (!isValid && !isEmptyFilter); + <LoadingButton type="submit" variant="outlined" color="primary" - disabled={ - !isDirty || - !( - isValid || - (settings.filters.length === 1 && - !settings.filters[0].field && - !settings.filters[0].value) - ) - } + disabled={isSaveDisabled} loading={isSaving.value}
92-390: Consider adding unit tests for regex validation logic.This component introduces critical business logic for incident PR filtering. Consider adding comprehensive unit tests to cover:
- Regex validation edge cases
- Form state management
- Save/update flow
- Error scenarios
Would you like me to help generate unit tests for this component or open an issue to track this task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
web-server/pages/api/internal/team/[team_id]/incident_prs_filter.ts(1 hunks)web-server/src/components/DoraMetricsConfigurationSettings.tsx(4 hunks)web-server/src/components/TeamIncidentPRsFilter.tsx(1 hunks)web-server/src/slices/team.ts(5 hunks)web-server/src/types/resources.ts(1 hunks)web-server/src/utils/cockpitMetricUtils.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
web-server/src/slices/team.ts (1)
web-server/src/types/resources.ts (2)
TeamIncidentPRsSettingsResponse(638-640)IncidentPRsSettings(623-629)
web-server/src/utils/cockpitMetricUtils.ts (2)
web-server/src/api-helpers/axios.ts (1)
handleRequest(86-96)web-server/src/types/resources.ts (2)
MeanTimeToRestoreApiResponse(538-541)MeanTimeToRestoreApiTrendsResponse(543-546)
🔇 Additional comments (19)
web-server/src/components/DoraMetricsConfigurationSettings.tsx (5)
6-6: LGTM! Clean component import.The import follows the established pattern and integrates well with the existing codebase structure.
13-16: LGTM! Consistent import pattern.The import structure maintains consistency with the existing
fetchTeamReposProductionBranchesimport pattern.
27-27: LGTM! Proper integration in useEffect.The dispatch call is correctly placed alongside the existing production branches fetch, ensuring both configurations are loaded when the component mounts.
40-46: LGTM! Consistent modal implementation.The callback follows the exact same pattern as
openProductionBranchSelectorModal, maintaining code consistency and reusability.
90-97: LGTM! Well-structured menu item.The menu item implementation is consistent with the existing production branch configuration item, following the same click handler pattern.
web-server/src/types/resources.ts (3)
623-629: LGTM! Well-designed settings type.The
IncidentPRsSettingstype is well-structured with clear field names and appropriate types. The filters array design allows for flexible field-value pair filtering.
631-636: LGTM! Consistent API response type.The
TeamIncidentPRsSettingApiResponsefollows the established pattern of other team setting API response types in the file, maintaining consistency.
638-640: LGTM! Clean response type.The simplified
TeamIncidentPRsSettingsResponsetype appropriately extracts only the setting data, following good API design principles.web-server/pages/api/internal/team/[team_id]/incident_prs_filter.ts (4)
10-13: LGTM! Proper path validation.The UUID validation for
team_idis crucial for security and prevents injection attacks. Good use of Yup's built-in UUID validator.
15-25: LGTM! Comprehensive PUT schema validation.The schema properly validates the setting structure including:
- Boolean validation for
include_revert_prs- Array validation for filters
- Required string validation for field and value
This ensures data integrity and prevents malformed requests.
29-41: LGTM! Clean GET handler implementation.The GET handler properly:
- Extracts team_id from validated payload
- Uses appropriate setting_type parameter
- Returns consistent response structure
43-58: LGTM! Proper PUT handler implementation.The PUT handler correctly:
- Extracts both team_id and setting from payload
- Sends setting_data to internal API
- Returns consistent response structure matching the type definitions
web-server/src/utils/cockpitMetricUtils.ts (5)
101-103: LGTM! Proper parameter integration.The
prFilterparameter is correctly extracted from the function parameters and will be properly applied to the API requests.
114-114: LGTM! Consistent filter application.The
prFilteris properly spread into the request parameters, ensuring incident PR filters are applied to the current stats request.
121-123: LGTM! Complete filter integration.Both
prevStatsTimeObjectandprFilterare correctly spread into the parameters for the previous stats request, maintaining consistency.
129-129: LGTM! Trends filtering applied correctly.The
prFilteris properly included in the current trends request parameters.
136-138: LGTM! Complete trends filter integration.Both time object and PR filter are correctly applied to the previous trends request, ensuring all API calls in the function respect the incident PR filters.
web-server/src/slices/team.ts (1)
17-295: Redux implementation looks good!The incident PR filters feature is well-integrated into the team slice:
- New state field and types are properly added
- Async thunks follow the established patterns
- API endpoints are consistent with the naming convention
- Error handling is managed through the existing
addFetchCasesToReducerutilityweb-server/src/components/TeamIncidentPRsFilter.tsx (1)
37-62: Well-designed schema validation!The Yup schema correctly validates that regex patterns contain exactly one
(\d+)capture group, which ensures the backend can reliably extract PR numbers from matched strings.
web-server/pages/api/internal/team/[team_id]/incident_prs_filter.ts
Outdated
Show resolved
Hide resolved
Linked Issue(s)
Ref #557
Further Comments
This is a stacked PR on top of #656
Proposed changes (including videos or screenshots)
Incident.PR.Filters.UI.Video.mp4
Summary by CodeRabbit