Fix: Daily experiments task failing due to method missing from MobitelDeliveryDetails #5509
+77
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danySam
changed the title
danySam
changed the title
danySam
reviewed
Dec 10, 2024
There was a problem hiding this comment.
@aagrawalrtsl I think we should add a test to the point where it was failing as well in NotificationExperiment#notification_result
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Story card: sc-13928
Because
The monitoring stage in the experimentation daily rake task is failing to send experiment notifications. Hence, the actual SMS sending part, which happens after monitoring, was not triggered.
The error was from a missing method "result" in MobitelDeliveryDetail class.
This addresses
We are adding the "result" method, which is used to determine the status of the notification. Since Mobitel does not give back the actual state of the notification, this is being hardcoded to "sent," assuming the API call to Mobitel is successful and the message was sent.
Test instructions
If we are using Mobitel as an SMS vendor, then `rake "experiments:conduct_daily" should complete without errors, and we should see SMSes being sent out.