fix adjustments #25
Merged
fix adjustments #25
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
karle0wne
commented
Oct 31, 2024
src/main/java/dev/vality/disputes/schedule/service/AdjustmentExtractor.java
Show resolved
Hide resolved
karle0wne
commented
Oct 31, 2024
| .orElse(String.format(DISPUTE_MASK, dispute.getId())); | ||
| } | ||
|
|
||
| public boolean isCashFlowAdjustmentByDisputeExist(InvoicePayment invoicePayment, Dispute dispute) { |
There was a problem hiding this comment.
2 метода здесь - найти корректировки которые теоретически _уже _ породил диспутс апи, идемпотености кейс
karle0wne
commented
Oct 31, 2024
Comment on lines
73
to
106
| if (!adjustmentExtractor.isCashFlowAdjustmentByDisputeExist(invoicePayment, dispute) | ||
| && !Objects.equals(dispute.getAmount(), dispute.getChangedAmount())) { | ||
| var params = invoicePaymentCashFlowAdjustmentParamsConverter.convert(dispute); | ||
| var paymentAdjustment = createAdjustment(dispute, params); | ||
| if (paymentAdjustment == null) { | ||
| log.error("Trying to set failed Dispute status with INVOICE_NOT_FOUND error reason {}", dispute.getId()); | ||
| disputeDao.update(dispute.getId(), DisputeStatus.failed, ErrorReason.INVOICE_NOT_FOUND); | ||
| log.debug("Dispute status has been set to failed {}", dispute.getId()); | ||
| var errorReason = ErrorReason.INVOICE_NOT_FOUND; | ||
| updateFailed(dispute, errorReason); | ||
| return; | ||
| } | ||
| } catch (InvoicingPaymentStatusPendingException e) { | ||
| // в теории 0%, что сюда попадает выполнение кода, но если попадет, то: | ||
| // платеж с не финальным статусом будет заблочен для создания корректировок на стороне хелгейта | ||
| // и тогда диспут будет пулиться, пока платеж не зафиналится, | ||
| // и тк никакой записи в коде выше нет, то пуллинг не проблема | ||
| // а запрос в checkDisputeStatus по идемпотентности просто вернет тот же success | ||
| log.error("Error when hg.createPaymentAdjustment() got payments status pending {}", dispute.getId(), e); | ||
| return; | ||
| } else { | ||
| log.info("Creating CashFlowAdjustment was skipped {}", dispute); | ||
| } | ||
| if (!adjustmentExtractor.isCapturedAdjustmentByDisputeExist(invoicePayment, dispute)) { | ||
| if (status.isSetCaptured()) { | ||
| var params = invoicePaymentFailedAdjustmentParamsConverter.convert(dispute); | ||
| var paymentAdjustment = createAdjustment(dispute, params); | ||
| if (paymentAdjustment == null) { | ||
| updateFailed(dispute, ErrorReason.INVOICE_NOT_FOUND); | ||
| return; | ||
| } | ||
| } | ||
| var params = invoicePaymentCapturedAdjustmentParamsConverter.convert(dispute); | ||
| var paymentAdjustment = createAdjustment(dispute, params); | ||
| if (paymentAdjustment == null) { | ||
| updateFailed(dispute, ErrorReason.INVOICE_NOT_FOUND); | ||
| return; | ||
| } | ||
| } else { | ||
| log.info("Creating CapturedAdjustment was skipped {}", dispute); | ||
| } | ||
| log.info("Trying to set succeeded Dispute status {}", dispute); | ||
| disputeDao.update(dispute.getId(), DisputeStatus.succeeded); | ||
| log.debug("Dispute status has been set to succeeded {}", dispute.getId()); | ||
| } |
There was a problem hiding this comment.
логика по результату обсуждения:
- обновили корретировкой сумму (если не совпадает)
- перевели корретировкой в успех
оценка потенциальных конфликтов \ коллизий:
- сторонний источник изменял корректировкой сумму (тг бот (через админскую ручку)\КЦ\др) - не влияет тк по словам коллег для сверок и как знание используется последняя корректировка
- сторонний источник изменял корректировкой статус (тг бот (через админскую ручку)\КЦ\др) - не влияет тк 1. обрабатыываем только финальынй статус 2.фейково фейлим через доп.корректировку в случае успеха платежа и накатываем сверху корректирвку на успех платежа - в итоге в любом случае успех будет
There was a problem hiding this comment.
убрал кетчер исключения тк мы перезапускаем шедулер так и так
There was a problem hiding this comment.
еще тут момент , даже если исполнятся 2 запроса корректировки до финализации и сервис выкинет исключение, затем пойдет заново проводить 2 корректировки (на сумму и фейл) , на результат это не повлияет
по крайней мере дубли на сумме ничего страшного, тк последняя сумма считается знанием
а с фейлом будет так если по порядку - 1) накатили фейл на платеж 2) исключение 3) накатили сумму 4) платеж в успехе? нет 5) скипнули шаг с фейлом и накатили успех и так по кругу если будут исключения
echerniak
reviewed
Oct 31, 2024
.../dev/vality/disputes/schedule/converter/InvoicePaymentCashFlowAdjustmentParamsConverter.java
Show resolved
Hide resolved
...va/dev/vality/disputes/schedule/converter/InvoicePaymentFailedAdjustmentParamsConverter.java
Outdated
Show resolved
Hide resolved
karle0wne
commented
Oct 31, 2024
| @@ -10,7 +10,7 @@ public interface CallbackNotifier { | |||
|
|
|||
| void sendDisputePoolingExpired(Dispute dispute); | |||
|
|
|||
| void sendDisputeReadyForCreateAdjustment(List<Dispute> disputes); | |||
| void sendDisputesReadyForCreateAdjustment(List<Dispute> disputes); | |||
There was a problem hiding this comment.
- в AccessService проверяется платеж, если не финальный статус, то возвращается 400 наружу (то есть не даем создать диспут в нефинальном статусе платежа) . все таки когда даем создать диспут, не учитывая статус платежа мы создаем пространство для коллизий: 1) платеж%пендинг диспут%создали , затем платеж%успех диспут%идет_в_провайдера - диспут успех на успех платежа? 2) платеж%пендинг диспут%создали , затем платеж%успех диспут%успех(сразу) - то есть это спор с тем что платеж в пендинге был? 3) более выраженный кейс когда создается диспут при платеже в captured для изменения суммы , иначе нам как то разелять , что отдельно есть запрос captured для изменения суммы и отдельно запрос когда платеж был pending а стал captured (диспут для которого надо в успех финалить) . будет проще - пендинг - нехуй лезть, убьет
- преименовал sendDisputeReady
- переименовал CreateAdjustments* в Adjustments
- сложил основные сервисы в пакет .core.*
- убрал проверку в пакете .core.*
if (!status.isSetCaptured() && !status.isSetCancelled() && !status.isSetFailed()) {(тк переносим на AccessService - при такой логике тест testSkipDisputeWhenPaymentNonFinalStatus не актуален
echerniak
approved these changes
Oct 31, 2024
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.
No description provided.