Skip to content

Conversation

tabacitu
Copy link
Contributor

I'm submitting the PR for @pxpm - he's the one who's made all this happen 🙏

Fixes #469 . Allows Blueprint to generate models with relationships to third-party packages (or oddly-placed models).

The developer can type belongsTo: \App\Models\Cart\Entry in the YAML file, as the relationship. Notice the \ prefix - that'll tell Blueprint that it's a fully-qualified class name, so it won't process it any further - it won't prefix App or App\Models to it, it'll just use it as-is.

if ($is_model_fqn) {
$fqcn = $class ?? $column_name;
$class_name = Str::afterLast($fqcn, '\\');
}else{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
}else{
} else {

Copy link
Collaborator

Choose a reason for hiding this comment

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

@tabacitu I don't think this was submitted correctly...

@jasonmccreary
Copy link
Collaborator

Please fix the style errors and remove all code comments.

@tabacitu
Copy link
Contributor Author

Sure, I'll let @pxpm do the fixes since it's his PR and his repo, I don't have write access.

Sidenote: I started refactoring this PR for code reuse - as a start to adding FQCN support for Models, Controllers, etc. That's what I had in mind when I promised a "cleanly written, fully-tested and well-documented PR" 😅 but now... I think this PR is much better, since it's less invasive.

You can find my refactoring in https://github.com/tabacitu/blueprint/pull/2 - but I think I went on cool but wrong path, so I stopped working on it to clear my head. If you would like to add FQCN support in other places in the future and have time, give it a look, but... it's not a short read read 😅 (I do that sometimes 😞). If not, don't - I'll probably close it in a few days or weeks when I decide it was a bad idea to begin with.

Cheers!

@jasonmccreary
Copy link
Collaborator

@tabacitu, I believe the issue is you did not allow the PR to be editable by the maintainer. I'm going to go ahead and merge this. Then I'll make the requested changes.

@jasonmccreary jasonmccreary merged commit 89b290f into laravel-shift:master Apr 22, 2021
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.

[Proposal] Support fully-qualified class names in relationships
3 participants