Skip to content

Conversation

@smarcet
Copy link
Collaborator

@smarcet smarcet commented Nov 6, 2025

@smarcet smarcet requested review from Copilot and romanetar November 6, 2025 01:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for synchronizing payment profile data from an external payments service via RabbitMQ message queue. The implementation adds an ExternalId field to track external payment profiles and implements message consumers to handle create, update, and delete events.

  • Added database migration to add ExternalId column to PaymentGatewayProfile table
  • Implemented RabbitMQ consumer infrastructure for payments sync with handlers for create, update, and delete operations
  • Added PaymentsApi client to fetch payment profile data from external service

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
readme.md Added documentation for the new payments_sync_consumer queue worker
database/migrations/model/Version20251106013928.php Database migration adding ExternalId column to PaymentGatewayProfile table
config/queue.php Configuration for payments_sync_consumer RabbitMQ connection
app/Services/Apis/PaymentsApi.php New API client for fetching payment profiles from external service
app/Services/Apis/IPaymentsApi.php Interface for PaymentsApi
app/Repositories/Summit/DoctrinePaymentGatewayProfileRepository.php Added getByExternalId method to query profiles by external ID
app/Models/Foundation/Summit/Repositories/IPaymentGatewayProfileRepository.php Added getByExternalId interface method
app/Models/Foundation/Summit/Registration/Payment/PaymentGatewayProfile.php Added external_id property with getter/setter methods
app/Models/Foundation/Summit/Factories/PaymentGatewayProfileFactory.php Added support for populating external_id field
app/Jobs/Payments/UpdatePaymentProfileMQJob.php Job handler for payment profile update events
app/Jobs/Payments/PaymentsMQJob.php RabbitMQ job dispatcher routing events to appropriate handlers
app/Jobs/Payments/EventTypes.php Constants defining payment profile event types
app/Jobs/Payments/DeletePaymentProfileMQJob.php Job handler for payment profile delete events
app/Jobs/Payments/CreatePaymentProfileMQJob.php Job handler for payment profile create events

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

$builder = new Builder($schema);
if($schema->hasTable("PaymentGatewayProfile") && !$builder->hasColumn("PaymentGatewayProfile", "ExternalId")) {
$builder->table("PaymentGatewayProfile", function (Table $table) {
$table->string("ExternalId", )->setNotnull(false)->setDefault('NULL');
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The string() method is missing the length parameter. Based on similar migrations in the codebase, this should specify a length value (e.g., 255). Additionally, setDefault('NULL') should be setDefault(null) for a proper NULL default value in the database.

Suggested change
$table->string("ExternalId", )->setNotnull(false)->setDefault('NULL');
$table->string("ExternalId", 255)->setNotnull(false)->setDefault(null);

Copilot uses AI. Check for mistakes.
/**
* @var string
*/
private $scopes;
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $scopes property is declared but never used in the class. Consider removing it to reduce code clutter.

Suggested change
private $scopes;

Copilot uses AI. Check for mistakes.
private $scopes;

/**
* ExternalUserApi constructor.
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constructor comment says 'ExternalUserApi constructor' but this is the PaymentsApi class. Update the comment to 'PaymentsApi constructor'.

Suggested change
* ExternalUserApi constructor.
* PaymentsApi constructor.

Copilot uses AI. Check for mistakes.
$test_publishable_key = $params['test_publishable_key'] ?? null;
$test_secret_key = $params['test_secret_key'] ?? null;

$external_id = $params['external_id'] ?? null;
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable $external_id is extracted but never used. This assignment can be removed since the value is accessed directly via $params['external_id'] in line 60-61.

Suggested change
$external_id = $params['external_id'] ?? null;

Copilot uses AI. Check for mistakes.

/**
* Class EventTypes
* @package App\Jobs\SponsorServices
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @package tag incorrectly references 'App\Jobs\SponsorServices' but this class is in the 'App\Jobs\Payments' namespace. Update to '@Package App\Jobs\Payments'.

Suggested change
* @package App\Jobs\SponsorServices
* @package App\Jobs\Payments

Copilot uses AI. Check for mistakes.
$id = intval($data['id']);
$summit_id = intval($data['summit_id']);
$response = $this->payments_api->getPaymentProfile($summit_id, $id);
Log::debug("UpdatePaymentProfileMQJob::handle", ['response' => $response]);
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log statement references 'UpdatePaymentProfileMQJob::handle' but this is in CreatePaymentProfileMQJob. Change to 'CreatePaymentProfileMQJob::handle' for accurate logging.

Suggested change
Log::debug("UpdatePaymentProfileMQJob::handle", ['response' => $response]);
Log::debug("CreatePaymentProfileMQJob::handle", ['response' => $response]);

Copilot uses AI. Check for mistakes.
$summit_id = intval($data['summit_id']);

$summit = $this->summit_repository->getById($summit_id);
$local_payment_profile =$this->payment_gateway_profile_repository->getByExternalId($id);
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after = assignment operator. Should be $local_payment_profile = $this->payment_gateway_profile_repository->getByExternalId($id); for consistent formatting.

Suggested change
$local_payment_profile =$this->payment_gateway_profile_repository->getByExternalId($id);
$local_payment_profile = $this->payment_gateway_profile_repository->getByExternalId($id);

Copilot uses AI. Check for mistakes.
@smarcet smarcet force-pushed the feature/payment-profile-synch branch from cc93a30 to a1083d3 Compare November 6, 2025 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants