Skip to content

Comments

Refactor/rates#175

Open
afonic wants to merge 5 commits intomainfrom
refactor/rates
Open

Refactor/rates#175
afonic wants to merge 5 commits intomainfrom
refactor/rates

Conversation

@afonic
Copy link
Contributor

@afonic afonic commented Feb 10, 2026

No description provided.

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 implements a major refactor replacing the property-based system with a comprehensive Rate model. It introduces independent and relative pricing, shared and independent availability pools, and removes the complex connected availability system.

Changes:

  • Adds resrv_rates table with support for relative pricing (e.g., -20%, +€20 from base) and shared availability
  • Migrates property column to rate_id across availabilities, reservations, and fixed pricing tables
  • Removes the event-driven connected availability system and its 5 sync types
  • Updates all Livewire components and Blade views from property/advanced terminology to rate-based

Reviewed changes

Copilot reviewed 57 out of 60 changed files in this pull request and generated no comments.

Show a summary per file
File Description
database/migrations/2026_03_01_000001_create_resrv_rates_table.php Creates rates table with pricing, availability, and restriction columns
database/migrations/2026_03_01_000002_add_rate_id_to_existing_tables.php Adds rate_id to availabilities, reservations, and fixed pricing
src/Models/Rate.php New Rate model with pricing calculations and availability logic
src/Http/Controllers/RateCpController.php CRUD controller for rate management in admin CP
src/Repositories/AvailabilityRepository.php Adds rate-aware queries and shared availability logic
src/Traits/HandlesPricing.php Applies relative rate pricing modifiers per day
src/Livewire/AvailabilitySearch.php Replaces advanced/property with rates UI
tests/Rate/RateCpTest.php Tests for rate CRUD operations and validation
tests/Rate/RateSharedAvailabilityTest.php Tests for shared availability pool behavior
resources/js/components/RatesList.vue Admin UI for managing rates per entry
PLAN.md Architecture documentation for rate system
TASKS.md Implementation task breakdown
Comments suppressed due to low confidence (4)

tests/TestCase.php:1

  • The Collection::make() result is not assigned to a variable but is discarded. While the .save() persists it, consider assigning it for consistency with line 203 where the collection is captured in $collection.
    tests/TestCase.php:1
  • The Collection::make() result is not assigned to a variable but is discarded. While the .save() persists it, consider assigning it for consistency with line 203 where the collection is captured in $collection.
    src/Traits/HandlesAvailabilityDates.php:1
  • The setAdvanced() method should be deprecated or removed as the system migrates from property-based to rate-based. The setRate() method has been added, but keeping both creates confusion about which to use.
    src/Livewire/Forms/AvailabilityData.php:1
  • The toResrvArray() method returns both rate_id and advanced keys. During the migration period this may be intentional for backward compatibility, but the advanced key should be removed once the migration is complete to avoid confusion.

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

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

Copilot reviewed 84 out of 89 changed files in this pull request and generated 3 comments.


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

Comment on lines +24 to +30
/**
* The data type of the primary key ID.
* Set to string for PostgreSQL compatibility with dynamic_pricing_assignments table.
*
* @var string
*/
protected $keyType = 'string';
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

The primary key type is set to 'string' in the model, but the database migration creates it as bigint unsigned via $table->id(). This type mismatch could cause issues with Eloquent operations expecting string IDs when the database actually stores integers. The comment mentions PostgreSQL compatibility with dynamic_pricing_assignments, but this should be resolved at the database schema level, not by misrepresenting the key type in the model.

Copilot uses AI. Check for mistakes.
Comment on lines +26 to 28
'ref' => 'required|string|max:10',
'hash' => 'required|string|size:64',
]);
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

The validation rule for 'ref' accepts strings up to 10 characters, but there's no validation ensuring the reference follows the expected format. The error message refers to "Invalid reservation parameters" which is very generic. Consider adding a regex pattern to validate the reference format if it follows a specific structure.

Copilot uses AI. Check for mistakes.
Comment on lines +40 to +42
if (! hash_equals($expectedHash, request()->get('hash'))) {
abort(404);
}
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

The hash verification uses timing-safe comparison with hash_equals, but if the hash doesn't match, the code returns a generic 404 error instead of 401 Unauthorized or 403 Forbidden. This makes it indistinguishable from a genuinely missing reservation, which could confuse legitimate users who have a typo in their hash versus attackers probing the system.

Copilot uses AI. Check for mistakes.
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.

1 participant