Add new models and migrations #35
Conversation
database/migrations/2021_02_25_194100_create_bookings_table.php
Outdated
Show resolved
Hide resolved
…:tipoff/bookings into wolfrednicolas/feature/refactor-bookings
database/migrations/2021_02_25_175500_create_booking_status_table.php
Outdated
Show resolved
Hide resolved
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| use Illuminate\Database\Migrations\Migration; | ||
| use Illuminate\Database\Schema\Blueprint; | ||
| use Illuminate\Support\Facades\Schema; | ||
|
|
||
| class CreateBookingCategoriesTable extends Migration | ||
| { | ||
| public function up() | ||
| { | ||
| Schema::create('booking_categories', function (Blueprint $table) { | ||
| $table->id(); | ||
| $table->string('label'); | ||
| $table->timestamps(); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Does this need to have its own database table?
Could we do a similar implementation to Keyword Types (TIPOFF/seo#28)?
There was a problem hiding this comment.
The reason models are used is a possibility to extend resource with configuration in the future. Booking category can define config for a group of bookings, for example category can have "requires_waiver" where one bookings need it, other ones dont.
There was a problem hiding this comment.
OK, that makes sense. Thanks
There was a problem hiding this comment.
I created issue #40 for this
…ture/models-and-migrations
Add new booking models & migrations
database/migrations/2020_05_11_100000_create_participants_table.php
Outdated
Show resolved
Hide resolved
database/migrations/2020_05_11_100000_create_participants_table.php
Outdated
Show resolved
Hide resolved
| { | ||
| Schema::create('booking_categories', function (Blueprint $table) { | ||
| $table->id(); | ||
| $table->string('label'); |
There was a problem hiding this comment.
I prefer to use name instead of label here.
There was a problem hiding this comment.
@wolfrednicolas can you rename all labels to names?
The reason I'm using label because name and label is usually different thing. $user->name is John and user->label might be John Thomas - Director of Marketing (Labels are used for text representation of resource).
There was a problem hiding this comment.
OK, I tend to prefer the terminology of title and name and making a distinction between those.
With users, I separate out first_name and last_name although the existing app uses name for first_name and name_last for last_name. We'll have to make those updates throughout the codebase in all the packages during this refactoring.
…bookings' into wolfrednicolas/feature/refactor-bookings
| $table->morphs('variation')->nullable(); | ||
| $table->morphs('experience')->nullable(); | ||
| $table->morphs('order')->nullable(); | ||
| // $table->morphs('variation')->nullable(); |
There was a problem hiding this comment.
@sl0wik how do we want to write tests for these? I'm not all that familiar with tests for these. Wouldn't these require the tipoff packages these models have been moved into? See my addtional comment below as well.
There was a problem hiding this comment.
Its still to be confimed by @drewroberts #42
We can use those rates inside variations.
There was a problem hiding this comment.
I confirm using Variations and moving in this direction.
There was a problem hiding this comment.
I created Issue #44 for Variations
| @@ -12,7 +12,7 @@ public function up() | |||
| { | |||
| Schema::create('participants', function (Blueprint $table) { | |||
| $table->id(); | |||
| $table->morphs('user')->nullable(); | |||
| // $table->morphs('user'); | |||
There was a problem hiding this comment.
This comes close to passing model tests, but since morphs create both an id and type column, the support user model for testing does not work in the scenario.
There was a problem hiding this comment.
If this is a blocker - we can restore it to user_id.
There was a problem hiding this comment.
I prefer a nullable user_id here since the User model is now in the tipoff/authorization package which is required by this package.
There was a problem hiding this comment.
CC: @sl0wik Can you make this change and then I will merge?
No description provided.