-
-
Notifications
You must be signed in to change notification settings - Fork 230
Add a new column Status Confirmed On in payments report #37169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
|
||
| status_update = { | ||
| PaymentProperties.PAYMENT_STATUS: status, | ||
| PaymentProperties.PAYMENT_STATUS_CONFIRMED_ON: datetime.utcnow().isoformat(), | ||
| } | ||
|
|
||
| if error_code is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The _handle_payment_status_retry() function omits the new PAYMENT_STATUS_CONFIRMED_ON field in the dictionary it returns during intermediate retries after a request error.
Severity: HIGH | Confidence: High
🔍 Detailed Analysis
When a PaymentRequestError occurs during a payment status check, the _handle_payment_status_retry() function is called. If the retry count has not been exceeded, the function returns a dictionary that only includes the PAYMENT_STATUS_ATTEMPT_COUNT. This return value is missing the newly introduced PAYMENT_STATUS_CONFIRMED_ON field. As a result, when bulk_update_cases() processes this incomplete data, the case property for the confirmation timestamp is not updated. This leaves the timestamp stale, which contradicts the feature's goal of showing when the status was last fetched.
💡 Suggested Fix
Update _handle_payment_status_retry() to include the PAYMENT_STATUS_CONFIRMED_ON field with the current UTC timestamp in the dictionary it returns during intermediate retries. This will ensure consistency with other status update paths.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: corehq/apps/integration/payments/services.py#L394-L400
Potential issue: When a `PaymentRequestError` occurs during a payment status check, the
`_handle_payment_status_retry()` function is called. If the retry count has not been
exceeded, the function returns a dictionary that only includes the
`PAYMENT_STATUS_ATTEMPT_COUNT`. This return value is missing the newly introduced
`PAYMENT_STATUS_CONFIRMED_ON` field. As a result, when `bulk_update_cases()` processes
this incomplete data, the case property for the confirmation timestamp is not updated.
This leaves the timestamp stale, which contradicts the feature's goal of showing when
the status was last fetched.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8082363
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intentional, as the payment status is still pending confirmation from the provider due to a request error. Once the maximum retry count is exceeded, we update PAYMENT_STATUS_CONFIRMED_ON with the timestamp of the last retry.
Product Description
Adds a new column Status Confirmed On in payments report. This column displays when the status of payment was last fetched from the provider.
Technical Summary
Ticket.
Feature Flag
MTN_MOBILE_WORKER_VERIFICATION
Safety Assurance
Safety story
A minor change for adding a new column. Changes behind an unused FF.
Automated test coverage
Test cases are updated.
QA Plan
None
Rollback instructions
Labels & Review