laravel-shift / blueprint

A code generation tool for Laravel developers.
MIT License
2.8k stars 270 forks source link

Table name set via meta is not honoured by foreign keys #684

Open Orteko opened 4 months ago

Orteko commented 4 months ago

Issue:

First up - thanks for the project, wish i'd found it earlier as it's going to save me a lot of grunt work!

On to the issue - as per https://github.com/laravel-shift/blueprint/pull/577 & https://github.com/laravel-shift/blueprint/issues/639 we appear to have the ability to use set a custom table name via meta when defining a model.

This works and $table is added to the model with the expected value.

However, when attempting to create a second model (Service in the included example) that references a model which has a custom table name defined, that custom table name is not used when creating the foreign key.

With the draft.yaml in this report - when running migrations a ERROR: relation "customers" does not exist (Connection: pgsql, SQL: alter table "service" add constraint "service_customer_id_foreign" foreign key ("customer_id") references "customers" ("id")) exception will be produced.

The culprit appears to be that Generators/MigrationGenerator.php @ line 318 - updating this line to include the table in the constrained() method "fixes" the issue, and both the models with custom table names and those without appear to migrate as expected.

However it then would operate identically to the if statement below it so i'm uncertain as to the underlying reason for these two if statements to be different:

diff --git a/src/Generators/MigrationGenerator.php b/src/Generators/MigrationGenerator.php
index b8c63a5..e105c5f 100644
--- a/src/Generators/MigrationGenerator.php
+++ b/src/Generators/MigrationGenerator.php
@@ -315,7 +315,7 @@ class MigrationGenerator extends AbstractClassGenerator implements Generator
                 $on_update_suffix = '->cascadeOnUpdate()';
             }
             if ($column_name === Str::singular($table) . '_' . $column) {
-                return self::INDENT . "{$prefix}->constrained(){$on_delete_suffix}{$on_update_suffix}";
+                return self::INDENT . "{$prefix}->constrained('{$table}'){$on_delete_suffix}{$on_update_suffix}";
             }
             if ($column === 'id') {
                 return self::INDENT . "{$prefix}->constrained('{$table}'){$on_delete_suffix}{$on_update_suffix}";

The 'meta' functionality is also not documented on the website as far as I can see so i'll open a separate issue at https://github.com/laravel-shift/blueprint-docs.git assuming it is expected to be supported.

draft.yaml:

models:
  Customer:
    name: unique string index
    meta:
      table: customer
  Service:
    customer_id: id foreign:customer.id
    name: unique string index
  Plan:
    name: unique string index
  Rate:
    plan_id: id foreign:plans.id
    name: unique string index

controllers:
  Customer:
    resource
  Service:
    resource
  Plan:
    resource
  Rate:
    resource
jasonmccreary commented 4 months ago

Yeah, that's getting deep. I could see fixing this for a single build run, but I wonder about future runs. Blueprint would either need to "remember" this custom table name or you would need to specify it, always. I'll have to think on that.