Skip to content

Commit a3e10a1

Browse files
committed
review comments: move to true toggle logic
Signed-off-by: Jordan Dubrick <[email protected]>
1 parent e2e9dd8 commit a3e10a1

File tree

4 files changed

+14
-121
lines changed

4 files changed

+14
-121
lines changed

docs/openapi.json

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -394,18 +394,8 @@
394394
"feedback"
395395
],
396396
"summary": "Feedback Toggle",
397-
"description": "Handle feedback toggle requests.\n\nReturns the current enbaled status of the feedback functionality after it has been\nupdated to the desired value from the request.\n\nReturns:\n StatusResponse: Indicates whether feedback is enabled.",
397+
"description": "Handle feedback toggle requests.\n\nReturns the current enabled status of the feedback functionality after it has been\nupdated to the opposite value from the previously set value.\n\nReturns:\n StatusResponse: Indicates whether feedback is enabled.",
398398
"operationId": "feedback_toggle_v1_feedback_toggle_post",
399-
"requestBody": {
400-
"content": {
401-
"application/json": {
402-
"schema": {
403-
"$ref": "#/components/schemas/FeedbackToggleRequest"
404-
}
405-
}
406-
},
407-
"required": true
408-
},
409399
"responses": {
410400
"200": {
411401
"description": "Successful Response",
@@ -416,16 +406,6 @@
416406
}
417407
}
418408
}
419-
},
420-
"422": {
421-
"description": "Validation Error",
422-
"content": {
423-
"application/json": {
424-
"schema": {
425-
"$ref": "#/components/schemas/HTTPValidationError"
426-
}
427-
}
428-
}
429409
}
430410
}
431411
}
@@ -1488,23 +1468,6 @@
14881468
}
14891469
]
14901470
},
1491-
"FeedbackToggleRequest": {
1492-
"properties": {
1493-
"status": {
1494-
"type": "boolean",
1495-
"title": "Status",
1496-
"description": "Desired state of feedback enablement, must be False or True",
1497-
"default": false,
1498-
"examples": [
1499-
true,
1500-
false
1501-
]
1502-
}
1503-
},
1504-
"type": "object",
1505-
"title": "FeedbackToggleRequest",
1506-
"description": "Model representing a feedback toggle request.\n\nAttributes:\n status: Boolean controlling what the Feedback status should be.\n\nExample:\n ```python\n feedback_request = FeedbackRequest(\n status=false\n )\n ```"
1507-
},
15081471
"ForbiddenResponse": {
15091472
"properties": {
15101473
"detail": {

src/app/endpoints/feedback.py

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
UnauthorizedResponse,
2121
ForbiddenResponse,
2222
)
23-
from models.requests import FeedbackToggleRequest
2423
from utils.suid import get_suid
2524

2625
logger = logging.getLogger(__name__)
@@ -178,32 +177,20 @@ def feedback_status() -> StatusResponse:
178177

179178

180179
@router.post("/toggle")
181-
def feedback_toggle(feedback_toggle_request: FeedbackToggleRequest) -> StatusResponse:
180+
def feedback_toggle() -> StatusResponse:
182181
"""
183182
Handle feedback toggle requests.
184183
185-
Returns the current enbaled status of the feedback functionality after it has been
186-
updated to the desired value from the request.
184+
Returns the current enabled status of the feedback functionality after it has been
185+
updated to the opposite value from the previously set value.
187186
188187
Returns:
189188
StatusResponse: Indicates whether feedback is enabled.
190189
"""
191-
fetched_status = toggle_feedback_status(feedback_toggle_request)
192-
return StatusResponse(functionality="feedback", status={"enabled": fetched_status})
193-
194-
195-
def toggle_feedback_status(req: FeedbackToggleRequest) -> bool:
196-
"""
197-
Toggles the feedback boolean stored in the configuration.
198-
199-
This toggling is temporary until the service is interrupted as it will not edit
200-
any configuration files.
201-
202-
Parameters:
203-
req (FeedbackToggleRequest): The request received with a boolean for the new value.
204-
205-
Returns:
206-
bool: The current enabled status of feedback.
207-
"""
208-
configuration.user_data_collection_configuration.feedback_enabled = req.get_value()
209-
return is_feedback_enabled()
190+
flipped_feedback_status = not is_feedback_enabled()
191+
configuration.user_data_collection_configuration.feedback_enabled = (
192+
flipped_feedback_status
193+
)
194+
return StatusResponse(
195+
functionality="feedback", status={"enabled": flipped_feedback_status}
196+
)

src/models/requests.py

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -380,28 +380,3 @@ def check_feedback_provided(self) -> Self:
380380
"'sentiment', 'user_feedback', or 'categories'"
381381
)
382382
return self
383-
384-
385-
class FeedbackToggleRequest(BaseModel):
386-
"""Model representing a feedback toggle request.
387-
388-
Attributes:
389-
status: Boolean controlling what the Feedback status should be.
390-
391-
Example:
392-
```python
393-
feedback_request = FeedbackRequest(
394-
status=false
395-
)
396-
```
397-
"""
398-
399-
status: bool = Field(
400-
False,
401-
description="Desired state of feedback enablement, must be False or True",
402-
examples=[True, False],
403-
)
404-
405-
def get_value(self) -> bool:
406-
"""Return the value of the status attribute."""
407-
return self.status

tests/unit/app/endpoints/test_feedback.py

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
feedback_status,
1313
feedback_toggle,
1414
)
15-
from models.requests import FeedbackToggleRequest
1615
from tests.unit.utils.auth_helpers import mock_authorization_resolvers
1716

1817

@@ -207,43 +206,12 @@ def test_feedback_status():
207206
assert response.status == {"enabled": True}
208207

209208

210-
def test_feedback_toggle_enabled():
211-
"""Test that feedback_toggle processes feedback toggle for disabled status payloads."""
209+
def test_feedback_toggle():
210+
"""Test that feedback_toggle correctly toggles the feedback enabled bool."""
212211

213212
configuration.user_data_collection_configuration.feedback_enabled = True
214213

215-
feedback_toggle_request = FeedbackToggleRequest(status=False)
216-
217-
response = feedback_toggle(feedback_toggle_request=feedback_toggle_request)
214+
response = feedback_toggle()
218215

219216
assert response.functionality == "feedback"
220217
assert response.status == {"enabled": False}
221-
222-
223-
def test_feedback_toggle_disabled():
224-
"""Test that feedback_toggle processes feedback toggle for enabled status payloads."""
225-
226-
configuration.user_data_collection_configuration.feedback_enabled = False
227-
228-
feedback_toggle_request = FeedbackToggleRequest(status=True)
229-
230-
response = feedback_toggle(feedback_toggle_request=feedback_toggle_request)
231-
232-
assert response.functionality == "feedback"
233-
assert response.status == {"enabled": True}
234-
235-
236-
def test_feedback_toggle_equivalent():
237-
"""
238-
Test that feedback_toggle processes feedback toggle for status payloads with the same value
239-
as what is presently set.
240-
"""
241-
242-
configuration.user_data_collection_configuration.feedback_enabled = True
243-
244-
feedback_toggle_request = FeedbackToggleRequest(status=True)
245-
246-
response = feedback_toggle(feedback_toggle_request=feedback_toggle_request)
247-
248-
assert response.functionality == "feedback"
249-
assert response.status == {"enabled": True}

0 commit comments

Comments
 (0)