Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de19dda03f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adjusts the IRIS payment flow to support webhook-based notifications, centralize callback/webhook processing, and introduce a dedicated IRIS success page.
Changes:
- Add an
IrisNotificationProcessorto verify IRIS hashes, locate/create orders, and mark payments paid/failed. - Introduce an IRIS webhook endpoint and wire
webhook_urlinto IRIS session creation. - Add a custom IRIS success page (controller/layout/block/template) and redirect IRIS callbacks to it.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| Everypay/view/frontend/web/js/Payform.js | Adds md into the browser callback URL used by the IRIS method config. |
| Everypay/view/frontend/templates/iris/success.phtml | New IRIS success template displaying order increment and email. |
| Everypay/view/frontend/layout/everypay_iris_success.xml | New layout handle for the IRIS success page. |
| Everypay/Model/IrisNotificationProcessor.php | New centralized processor for webhook/callback verification + order/payment state updates. |
| Everypay/Controller/Iris/Webhook.php | New POST webhook controller delegating to the processor. |
| Everypay/Controller/Iris/Success.php | New success-page controller rendering the IRIS success page. |
| Everypay/Controller/Iris/CreateSession.php | Adds webhook_url and persists md/quote references into session + callback URL query. |
| Everypay/Controller/Iris/Callback.php | Refactors callback handling to delegate processing to the processor and redirect to custom success route. |
| Everypay/Block/Iris/Success.php | New block that loads the order from order_id request param for the success template. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (2)
Everypay/Block/Iris/Success.php:63
- This block uses the global ObjectManager to fetch the checkout session. Prefer constructor-injecting
\Magento\Checkout\Model\Session(or a session proxy) to keep dependencies explicit and improve testability.
Everypay/view/frontend/templates/iris/success.phtml:26 - Displaying the full customer email address on the success page exposes PII to anyone who can access the page/session (e.g., shared devices, cached pages, screenshots). Consider removing it, masking it (e.g., a***@Domain), or only rendering it for logged-in customers who match the order.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9273f4ab8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public function execute() | ||
| { | ||
| // Disable caching | ||
| $this->response->setNoCacheHeaders(); | ||
|
|
||
| // Check request method | ||
| $method = $_SERVER['REQUEST_METHOD'] ?? 'GET'; | ||
|
|
||
| if ($method === 'GET') { |
There was a problem hiding this comment.
execute() uses $_SERVER['REQUEST_METHOD'] to branch on GET vs POST. Prefer Magento's request API ($this->getRequest()->getMethod() / isPost() / isGet()) to avoid relying on PHP superglobals and improve testability/consistency.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7af2fb0c00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $hash = (string) $request->getParam('hash'); | ||
| if (empty($hash)) { |
There was a problem hiding this comment.
The new webhook flow calls processNotification() for requests that EveryPay sends as JSON, but extractPayload() only reads fields through $request->getParam(...), which in Magento only checks query/form parameters and does not decode the raw JSON body. In that webhook scenario hash/token/md remain empty, so we immediately return “Missing hash” and respond 400, causing successful IRIS payments that rely on webhook delivery to never be processed. Please decode php://input JSON (or otherwise map JSON body fields) before falling back to getParam.
Useful? React with 👍 / 👎.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…magento2 into feature/FIT-708-fixes
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $this->applyIrisMetadata($order, $token, $md, $payloadResult['hash']); | ||
|
|
||
| if ($orderAlreadyPaid && (!$token || !$existingToken || $existingToken === $token)) { | ||
| $this->logger->info('IRIS notification already processed', [ | ||
| 'source' => $source, |
There was a problem hiding this comment.
applyIrisMetadata() mutates the order/payment, but in the already_processed early-return branch those changes are never persisted. This means incoming iris_hash / iris_md (or a missing iris_token) can be set on the in-memory object and then lost. Consider saving the order after applying metadata when returning already_processed, or only applying metadata after the early-return check (and ensuring required fields are persisted elsewhere).
| $response = Payment::create($params); | ||
|
|
||
| $this->logger->debug('IRIS Payment Request', ['params' => $params]); | ||
| $this->logger->debug('IRIS Payment Response', ['response' => $response]); |
There was a problem hiding this comment.
The debug logs include the full payment creation request/response, which contains sensitive data (e.g., IRIS token, customer email/phone) and potentially secrets returned by the PSP. These values should be masked or omitted from logs to avoid leaking payment/PII in production log streams; log only non-sensitive identifiers (order_id, amount, currency) or redact fields before logging.
| $order = $this->orderFactory->create(); | ||
| $order->setQuoteId($quote->getId()); | ||
| $order->setStoreId($quote->getStoreId()); | ||
| $order->setCustomerId($quote->getCustomerId()); | ||
| $order->setCustomerEmail($quote->getCustomerEmail()); | ||
| $order->setCustomerFirstname($quote->getCustomerFirstname()); | ||
| $order->setCustomerLastname($quote->getCustomerLastname()); | ||
| $order->setCustomerIsGuest($quote->getCustomerIsGuest()); | ||
| $order->setState(Order::STATE_PENDING_PAYMENT); | ||
| $order->setStatus(Order::STATE_PENDING_PAYMENT); | ||
| $order->setSubtotal($quote->getSubtotal()); | ||
| $order->setBaseSubtotal($quote->getBaseSubtotal()); | ||
| $order->setGrandTotal($quote->getGrandTotal()); | ||
| $order->setBaseGrandTotal($quote->getBaseGrandTotal()); | ||
| $order->setShippingAmount($quote->getShippingAddress()->getShippingAmount()); | ||
| $order->setBaseShippingAmount($quote->getShippingAddress()->getBaseShippingAmount()); | ||
| $order->setShippingDescription($quote->getShippingAddress()->getShippingDescription()); | ||
| $order->setShippingMethod($quote->getShippingAddress()->getShippingMethod()); | ||
| $order->setTaxAmount($quote->getShippingAddress()->getTaxAmount()); | ||
| $order->setBaseTaxAmount($quote->getShippingAddress()->getBaseTaxAmount()); | ||
| $order->setShippingTaxAmount($quote->getShippingAddress()->getShippingTaxAmount()); | ||
| $order->setBaseShippingTaxAmount($quote->getShippingAddress()->getBaseShippingTaxAmount()); | ||
| $order->setDiscountAmount($quote->getShippingAddress()->getDiscountAmount()); | ||
| $order->setBaseDiscountAmount($quote->getShippingAddress()->getBaseDiscountAmount()); | ||
| $order->setDiscountDescription($quote->getShippingAddress()->getDiscountDescription()); | ||
| $order->setOrderCurrencyCode($quote->getQuoteCurrencyCode()); | ||
| $order->setBaseCurrencyCode($quote->getBaseCurrencyCode()); | ||
|
|
||
| foreach ($quote->getAllVisibleItems() as $quoteItem) { | ||
| $orderItem = $this->objectManager->create(Item::class); | ||
| $orderItem->setQuoteItemId($quoteItem->getId()); | ||
| $orderItem->setProductId($quoteItem->getProductId()); | ||
| $orderItem->setSku($quoteItem->getSku()); | ||
| $orderItem->setName($quoteItem->getName()); | ||
| $orderItem->setQtyOrdered($quoteItem->getQty()); | ||
| $orderItem->setPrice($quoteItem->getPrice()); | ||
| $orderItem->setBasePrice($quoteItem->getBasePrice()); | ||
| $orderItem->setRowTotal($quoteItem->getRowTotal()); | ||
| $orderItem->setBaseRowTotal($quoteItem->getBaseRowTotal()); | ||
| $order->addItem($orderItem); | ||
| } | ||
|
|
||
| $billingAddress = $this->objectManager->create(Address::class); | ||
| $billingAddress->setData($quote->getBillingAddress()->getData()); | ||
| $billingAddress->setAddressType(Address::TYPE_BILLING); | ||
| $order->setBillingAddress($billingAddress); | ||
|
|
||
| if (!$quote->isVirtual()) { | ||
| $shippingAddress = $this->objectManager->create(Address::class); | ||
| $shippingAddress->setData($quote->getShippingAddress()->getData()); | ||
| $shippingAddress->setAddressType(Address::TYPE_SHIPPING); | ||
| $order->setShippingAddress($shippingAddress); | ||
| } | ||
|
|
||
| $orderPayment = $this->objectManager->create(OrderPayment::class); | ||
| $orderPayment->setMethod('everypay'); | ||
| $order->setPayment($orderPayment); | ||
|
|
||
| $savedOrder = $this->orderRepository->save($order); |
There was a problem hiding this comment.
The manual order-creation fallback builds and saves an Order model directly, but it never assigns a reserved/increment order number (e.g., from the quote's reserved order ID / sales sequence). If CartManagement::placeOrder() fails and this path runs, it risks creating orders without a valid increment_id and bypassing Magento's normal submit pipeline (invoices, emails, inventory, totals consistency). Prefer fixing the underlying placeOrder() failure and removing this fallback, or switch to Magento's quote submission services that generate increment IDs and dispatch the standard order placement flow.
| $order = $this->orderFactory->create(); | |
| $order->setQuoteId($quote->getId()); | |
| $order->setStoreId($quote->getStoreId()); | |
| $order->setCustomerId($quote->getCustomerId()); | |
| $order->setCustomerEmail($quote->getCustomerEmail()); | |
| $order->setCustomerFirstname($quote->getCustomerFirstname()); | |
| $order->setCustomerLastname($quote->getCustomerLastname()); | |
| $order->setCustomerIsGuest($quote->getCustomerIsGuest()); | |
| $order->setState(Order::STATE_PENDING_PAYMENT); | |
| $order->setStatus(Order::STATE_PENDING_PAYMENT); | |
| $order->setSubtotal($quote->getSubtotal()); | |
| $order->setBaseSubtotal($quote->getBaseSubtotal()); | |
| $order->setGrandTotal($quote->getGrandTotal()); | |
| $order->setBaseGrandTotal($quote->getBaseGrandTotal()); | |
| $order->setShippingAmount($quote->getShippingAddress()->getShippingAmount()); | |
| $order->setBaseShippingAmount($quote->getShippingAddress()->getBaseShippingAmount()); | |
| $order->setShippingDescription($quote->getShippingAddress()->getShippingDescription()); | |
| $order->setShippingMethod($quote->getShippingAddress()->getShippingMethod()); | |
| $order->setTaxAmount($quote->getShippingAddress()->getTaxAmount()); | |
| $order->setBaseTaxAmount($quote->getShippingAddress()->getBaseTaxAmount()); | |
| $order->setShippingTaxAmount($quote->getShippingAddress()->getShippingTaxAmount()); | |
| $order->setBaseShippingTaxAmount($quote->getShippingAddress()->getBaseShippingTaxAmount()); | |
| $order->setDiscountAmount($quote->getShippingAddress()->getDiscountAmount()); | |
| $order->setBaseDiscountAmount($quote->getShippingAddress()->getBaseDiscountAmount()); | |
| $order->setDiscountDescription($quote->getShippingAddress()->getDiscountDescription()); | |
| $order->setOrderCurrencyCode($quote->getQuoteCurrencyCode()); | |
| $order->setBaseCurrencyCode($quote->getBaseCurrencyCode()); | |
| foreach ($quote->getAllVisibleItems() as $quoteItem) { | |
| $orderItem = $this->objectManager->create(Item::class); | |
| $orderItem->setQuoteItemId($quoteItem->getId()); | |
| $orderItem->setProductId($quoteItem->getProductId()); | |
| $orderItem->setSku($quoteItem->getSku()); | |
| $orderItem->setName($quoteItem->getName()); | |
| $orderItem->setQtyOrdered($quoteItem->getQty()); | |
| $orderItem->setPrice($quoteItem->getPrice()); | |
| $orderItem->setBasePrice($quoteItem->getBasePrice()); | |
| $orderItem->setRowTotal($quoteItem->getRowTotal()); | |
| $orderItem->setBaseRowTotal($quoteItem->getBaseRowTotal()); | |
| $order->addItem($orderItem); | |
| } | |
| $billingAddress = $this->objectManager->create(Address::class); | |
| $billingAddress->setData($quote->getBillingAddress()->getData()); | |
| $billingAddress->setAddressType(Address::TYPE_BILLING); | |
| $order->setBillingAddress($billingAddress); | |
| if (!$quote->isVirtual()) { | |
| $shippingAddress = $this->objectManager->create(Address::class); | |
| $shippingAddress->setData($quote->getShippingAddress()->getData()); | |
| $shippingAddress->setAddressType(Address::TYPE_SHIPPING); | |
| $order->setShippingAddress($shippingAddress); | |
| } | |
| $orderPayment = $this->objectManager->create(OrderPayment::class); | |
| $orderPayment->setMethod('everypay'); | |
| $order->setPayment($orderPayment); | |
| $savedOrder = $this->orderRepository->save($order); | |
| if (!$quote->getReservedOrderId()) { | |
| $quote->reserveOrderId(); | |
| } | |
| $this->quoteRepository->save($quote); | |
| $quoteManagement = $this->objectManager->create(\Magento\Quote\Model\QuoteManagement::class); | |
| $savedOrder = $quoteManagement->submit($quote); | |
| if (!$savedOrder || !$savedOrder->getEntityId()) { | |
| throw new LocalizedException(__('IRIS quote submission failed.')); | |
| } |
| protected function getCheckoutSession() | ||
| { | ||
| return ObjectManager::getInstance()->get(\Magento\Checkout\Model\Session::class); | ||
| } |
There was a problem hiding this comment.
This controller pulls the checkout session via ObjectManager::getInstance() instead of constructor injection. Elsewhere in this module (e.g., Everypay/Controller/Iris/Pending.php) dependencies are injected, which makes code easier to test and avoids hidden runtime coupling. Consider injecting \Magento\Checkout\Model\Session (or \Magento\Checkout\Model\SessionFactory) and removing the ObjectManager usage.
| public function findOrderByIrisReference($token, $md, $allowCreateFromQuote = false) | ||
| { | ||
| if (!empty($token)) { | ||
| try { | ||
| $orderCollection = $this->orderFactory->create()->getCollection() | ||
| ->join( | ||
| ['payment' => 'sales_order_payment'], | ||
| 'main_table.entity_id = payment.parent_id', | ||
| [] | ||
| ) | ||
| ->addFieldToFilter('payment.additional_information', ['like' => '%"iris_token":"' . $token . '"%']) |
There was a problem hiding this comment.
findOrderByIrisReference() builds a SQL LIKE pattern using the raw $token. In GET callbacks, token comes from an unverified query param, so a value containing % / _ can act as a wildcard and potentially match unrelated orders that have any iris_token stored. Consider either (a) not using the unverified token for lookup on GET (prefer the signed hash payload / md-with-quote-id), and/or (b) escaping LIKE wildcards in the token before building the pattern (and ideally querying a dedicated column rather than JSON-in-text).
| public function findOrderByIrisReference($token, $md, $allowCreateFromQuote = false) | |
| { | |
| if (!empty($token)) { | |
| try { | |
| $orderCollection = $this->orderFactory->create()->getCollection() | |
| ->join( | |
| ['payment' => 'sales_order_payment'], | |
| 'main_table.entity_id = payment.parent_id', | |
| [] | |
| ) | |
| ->addFieldToFilter('payment.additional_information', ['like' => '%"iris_token":"' . $token . '"%']) | |
| private function escapeLikeValue($value) | |
| { | |
| return str_replace( | |
| ['\\', '%', '_'], | |
| ['\\\\', '\\%', '\\_'], | |
| (string) $value | |
| ); | |
| } | |
| public function findOrderByIrisReference($token, $md, $allowCreateFromQuote = false) | |
| { | |
| if (!empty($token)) { | |
| try { | |
| $escapedToken = $this->escapeLikeValue($token); | |
| $orderCollection = $this->orderFactory->create()->getCollection() | |
| ->join( | |
| ['payment' => 'sales_order_payment'], | |
| 'main_table.entity_id = payment.parent_id', | |
| [] | |
| ) | |
| ->addFieldToFilter( | |
| 'payment.additional_information', | |
| ['like' => '%"iris_token":"' . $escapedToken . '"%'] | |
| ) |
No description provided.