Skip to content
This repository was archived by the owner on Sep 3, 2025. It is now read-only.

Conversation

@whitdog47
Copy link
Contributor

@whitdog47 whitdog47 commented May 14, 2025

This PR creates two endpoints to update the signal defintion, one that allows the signal filters (snooze, deduplication) to be overwritten (a new endpoint just for the UI), and one that only allows addition of new signal filters (the existing endpoint). It also fixes a discrepancy with the cli check that wouldn't allow a database init and restore if the db didn't previously exist.

@whitdog47 whitdog47 requested a review from Copilot May 14, 2025 00:28
@whitdog47 whitdog47 self-assigned this May 14, 2025
@whitdog47 whitdog47 added the bug Something isn't working label May 14, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces two endpoints for updating signal definitions while handling signal filters differently and fixes a CLI check to allow database initialization and restore when the database does not already exist.

  • Updated the API endpoint for updating signals.
  • Separated update endpoints for UI (with filters) and API (without filters).
  • Refactored exception-handling in signal views and service functions and improved model definitions for user updates.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/dispatch/static/dispatch/src/signal/api.js Adjusted the API endpoint for updating signals to include "/update/".
src/dispatch/signal/views.py Added two separate update endpoints and refactored error handling.
src/dispatch/signal/service.py Updated the signal update function to conditionally update filters.
src/dispatch/cli.py Changed the CLI confirmation flow to return early for non-drop commands.
src/dispatch/auth/models.py Updated Pydantic models for user updates with Optional types.
Comments suppressed due to low confidence (3)

src/dispatch/auth/models.py:213

  • [nitpick] Consider providing a default value (e.g., = None) for the 'organizations' field in the UserUpdate model for consistency with the other Optional fields.
organizations: Optional[list[UserOrganization]]

src/dispatch/static/dispatch/src/signal/api.js:21

  • Ensure that changing the update endpoint path to include '/update/' is consistent with the overall API design and routing standards across the project.
return API.put(`${resource}/update/${signalId}`, payload)

src/dispatch/signal/service.py:540

  • Confirm that SignalFilter objects are hashable and have an appropriate equality implementation, as they are used within a set for filter updates.
filter_set = set() if update_filters else set(signal.filters)

@whitdog47 whitdog47 requested a review from wssheldon May 14, 2025 16:24
@whitdog47 whitdog47 merged commit a2250ff into main May 14, 2025
11 checks passed
@whitdog47 whitdog47 deleted the fix/keep-existing-filters-on-signal-update branch May 14, 2025 20:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants