Fixes errors in oncall end-of-shift feedback reminders plus opens api…

… to fetch feedback (#3817)

src/dispatch/api.py

@@ -24,6 +24,7 @@
 from dispatch.entity.views import router as entity_router
 from dispatch.entity_type.views import router as entity_type_router
 from dispatch.feedback.incident.views import router as feedback_router
+from dispatch.feedback.service.views import router as service_feedback_router
 from dispatch.incident.priority.views import router as incident_priority_router
 from dispatch.incident.severity.views import router as incident_severity_router
 from dispatch.incident.type.views import router as incident_type_router
@@ -203,6 +204,9 @@ def get_organization_path(organization: OrganizationSlug):
 authenticated_organization_api_router.include_router(
     feedback_router, prefix="/feedback", tags=["feedback"]
 )
+authenticated_organization_api_router.include_router(
+    service_feedback_router, prefix="/service_feedback", tags=["service_feedback"]
+)
 authenticated_organization_api_router.include_router(
     notification_router, prefix="/notifications", tags=["notifications"]
 )

src/dispatch/feedback/service/models.py

@@ -5,10 +5,11 @@
 from sqlalchemy import Column, Integer, ForeignKey, DateTime, String
 from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy_utils import TSVectorType
+from sqlalchemy.orm import relationship
 
 from dispatch.database.core import Base
-from dispatch.individual.models import IndividualContactRead
-from dispatch.models import DispatchBase, TimeStampMixin, FeedbackMixin, PrimaryKey
+from dispatch.individual.models import IndividualContactReadMinimal
+from dispatch.models import DispatchBase, TimeStampMixin, FeedbackMixin, PrimaryKey, Pagination
 from dispatch.project.models import ProjectRead
 
 from .enums import ServiceFeedbackRating
@@ -26,6 +27,7 @@ class ServiceFeedback(TimeStampMixin, FeedbackMixin, Base):
 
     # Relationships
     individual_contact_id = Column(Integer, ForeignKey("individual_contact.id"))
+    individual = relationship("IndividualContact")
 
     search_vector = Column(
         TSVectorType(
@@ -37,14 +39,14 @@ class ServiceFeedback(TimeStampMixin, FeedbackMixin, Base):
 
     @hybrid_property
     def project(self):
-        return self.service.project
+        return self.individual.project
 
 
 # Pydantic models
 class ServiceFeedbackBase(DispatchBase):
     feedback: Optional[str] = Field(None, nullable=True)
     hours: Optional[int]
-    individual: Optional[IndividualContactRead]
+    individual: Optional[IndividualContactReadMinimal]
     rating: ServiceFeedbackRating = ServiceFeedbackRating.little_effort
     schedule: Optional[str]
     shift_end_at: Optional[datetime]
@@ -64,6 +66,6 @@ class ServiceFeedbackRead(ServiceFeedbackBase):
     project: Optional[ProjectRead]
 
 
-class ServiceFeedbackPagination(DispatchBase):
+class ServiceFeedbackPagination(Pagination):
     items: List[ServiceFeedbackRead]
     total: int

src/dispatch/feedback/service/reminder/service.py

@@ -3,7 +3,6 @@
 
 from .models import (
     ServiceFeedbackReminder,
-    ServiceFeedbackReminderCreate,
     ServiceFeedbackReminderUpdate,
 )
 from dispatch.individual.models import IndividualContact
@@ -24,11 +23,11 @@ def get_all_expired_reminders_by_project_id(
     )
 
 
-def create(*, db_session, reminder_in: ServiceFeedbackReminderCreate) -> ServiceFeedbackReminder:
+def create(*, db_session, reminder_in: ServiceFeedbackReminder) -> ServiceFeedbackReminder:
     """Creates a new service feedback reminder."""
     reminder = ServiceFeedbackReminder(**reminder_in.dict())
 
-    db_session.add(reminder_in)
+    db_session.add(reminder)
     db_session.commit()
     return reminder
 
@@ -55,5 +54,6 @@ def delete(*, db_session, reminder_id: int):
         .filter(ServiceFeedbackReminder.id == reminder_id)
         .one_or_none()
     )
-    db_session.delete(reminder)
-    db_session.commit()
+    if reminder:
+        db_session.delete(reminder)
+        db_session.commit()

src/dispatch/feedback/service/scheduled.py

@@ -28,15 +28,15 @@
 @timer
 @scheduled_project_task
 def oncall_shift_feedback_ucan(db_session: SessionLocal, project: Project):
-    oncall_shift_feedback(db_session=db_session, project=project)
+    oncall_shift_feedback(db_session=db_session, project=project, hour=16)
     find_expired_reminders_and_send(db_session=db_session, project=project)
 
 
 @scheduler.add(every(1).day.at("06:00"), name="oncall-shift-feedback-emea")
 @timer
 @scheduled_project_task
 def oncall_shift_feedback_emea(db_session: SessionLocal, project: Project):
-    oncall_shift_feedback(db_session=db_session, project=project)
+    oncall_shift_feedback(db_session=db_session, project=project, hour=6)
     find_expired_reminders_and_send(db_session=db_session, project=project)
 
 
@@ -60,13 +60,18 @@ def find_expired_reminders_and_send(*, db_session: SessionLocal, project: Projec
 
 
 def find_schedule_and_send(
-    *, db_session: SessionLocal, project: Project, oncall_plugin: PluginInstance, schedule_id: str
+    *,
+    db_session: SessionLocal,
+    project: Project,
+    oncall_plugin: PluginInstance,
+    schedule_id: str,
+    hour: int,
 ):
     """
     Given PagerDuty schedule_id, determine if the shift ended for the previous oncall person and
     send the health metrics feedback request
     """
-    current_oncall = oncall_plugin.instance.did_oncall_just_go_off_shift(schedule_id)
+    current_oncall = oncall_plugin.instance.did_oncall_just_go_off_shift(schedule_id, hour)
 
     individual = individual_service.get_by_email_and_project(
         db_session=db_session, email=current_oncall["email"], project_id=project.id
@@ -82,7 +87,7 @@ def find_schedule_and_send(
     )
 
 
-def oncall_shift_feedback(db_session: SessionLocal, project: Project):
+def oncall_shift_feedback(db_session: SessionLocal, project: Project, hour: int):
     """
     Experimental: collects feedback from individuals participating in an oncall service that has health metrics enabled
     when their oncall shift ends. For now, only for one project and schedule.
@@ -115,4 +120,5 @@ def oncall_shift_feedback(db_session: SessionLocal, project: Project):
                 project=project,
                 oncall_plugin=oncall_plugin,
                 schedule_id=schedule_id,
+                hour=hour,
             )

src/dispatch/feedback/service/views.py

@@ -0,0 +1,14 @@
+from fastapi import APIRouter
+
+from dispatch.database.service import search_filter_sort_paginate, CommonParameters
+
+from .models import ServiceFeedbackPagination
+
+
+router = APIRouter()
+
+
+@router.get("", response_model=ServiceFeedbackPagination)
+def get_feedback_entries(commons: CommonParameters):
+    """Get all feedback entries, or only those matching a given search term."""
+    return search_filter_sort_paginate(model="ServiceFeedback", **commons)

src/dispatch/plugins/dispatch_pagerduty/plugin.py

@@ -83,12 +83,13 @@ def page(
             incident_description=incident_description,
         )
 
-    def did_oncall_just_go_off_shift(self, schedule_id: str) -> Optional[dict]:
+    def did_oncall_just_go_off_shift(self, schedule_id: str, hour: int) -> Optional[dict]:
         client = APISession(self.configuration.api_key.get_secret_value())
         client.url = self.configuration.pagerduty_api_url
         return oncall_shift_check(
             client=client,
             schedule_id=schedule_id,
+            hour=hour,
         )
 
     def get_schedule_id_from_service_id(self, service_id: str) -> Optional[str]:

src/dispatch/plugins/dispatch_pagerduty/service.py

@@ -177,9 +177,12 @@ def get_oncall_at_time(client: APISession, schedule_id: str, utctime: str) -> Op
         raise e
 
 
-def oncall_shift_check(client: APISession, schedule_id: str) -> Optional[dict]:
+def oncall_shift_check(client: APISession, schedule_id: str, hour: int) -> Optional[dict]:
     """Determines whether the oncall person just went off shift and returns their email."""
     now = datetime.utcnow()
+    # in case scheduler is late, replace hour with exact one for shift comparison
+    now = now.replace(hour=hour, minute=0, second=0, microsecond=0)
+
     # compare oncall person scheduled 18 hours ago vs 2 hours from now
     previous_shift = (now - timedelta(hours=18)).isoformat(timespec="minutes") + "Z"
     next_shift = (now + timedelta(hours=2)).isoformat(timespec="minutes") + "Z"

src/dispatch/plugins/dispatch_slack/feedback/interactive.py

@@ -186,7 +186,7 @@ def handle_incident_feedback_submission_event(
     ack_incident_feedback_submission_event(ack=ack)
     incident = incident_service.get(db_session=db_session, incident_id=context["subject"].id)
 
-    feedback = form_data.get(IncidentFeedbackNotificationBlockIds.feedback_input)
+    feedback = form_data.get(IncidentFeedbackNotificationBlockIds.feedback_input, "")
     rating = form_data.get(IncidentFeedbackNotificationBlockIds.rating_select, {}).get("value")
 
     feedback_in = FeedbackCreate(
@@ -276,6 +276,7 @@ def oncall_shift_feedback_input(
             multiline=True,
             placeholder="How would you describe your experience?",
         ),
+        optional=True,
         label=label,
         **kwargs,
     )
@@ -381,7 +382,7 @@ def handle_oncall_shift_feedback_submission_event(
 
     ack_oncall_shift_feedback_submission_event(ack=ack)
 
-    feedback = form_data.get(ServiceFeedbackNotificationBlockIds.feedback_input)
+    feedback = form_data.get(ServiceFeedbackNotificationBlockIds.feedback_input, "")
     rating = form_data.get(ServiceFeedbackNotificationBlockIds.rating_select, {}).get("value")
 
     # metadata is organization_slug|project_id|schedule_id|shift_end_at|reminder_id
@@ -447,7 +448,7 @@ def handle_oncall_shift_feedback_submission_event(
         ]
         try:
             plugin.instance.send_direct(
-                individual.email,
+                user.email,
                 notification_text,
                 notification_template,
                 MessageType.service_feedback,

src/dispatch/static/dispatch/src/service_feedback/api.js

@@ -0,0 +1,9 @@
+import API from "@/api"
+
+const resource = "/service_feedback"
+
+export default {
+  getAll(options) {
+    return API.get(`${resource}`, { params: { ...options } })
+  },
+}