Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is the by-resource option functional? #169

Closed
Wizofgoz opened this issue Apr 24, 2018 · 8 comments
Closed

Is the by-resource option functional? #169

Wizofgoz opened this issue Apr 24, 2018 · 8 comments
Labels

Comments

@Wizofgoz
Copy link

I'm trying to generate my resources via artisan with the by-resource option set to false but it is creating the same class names in the same namespaces and throws an exception. I would think it would be expecting the class name to be the same as the underlying model but wanted to be sure (i.e. User model => User adapter).

I went looking in the code to determine what class names would be expected but couldn't find any places that use that option other than the artisan generators.

@Wizofgoz
Copy link
Author

Correction: I was able to determine that the class names should be the same as the underlying model, so the only problem is that the generator does not set the correct class name when using
php artisan make:json-api:resource resourceName

@lindyhopchris
Copy link
Member

Hi! This should be working so thanks for reporting. I'll look into it shortly. What version are you using?

@lindyhopchris
Copy link
Member

Quick note: tested on the 1.0.0-alpha.1 branch and it's working on that branch. Need to check on 0.12

@Wizofgoz
Copy link
Author

ok, I am on 0.12 but it's good to hear that it's working on 1.0.0-alpha.1

@Wizofgoz
Copy link
Author

@lindyhopchris are you sure it's working on 1.0.0-alpha.1? I just migrated to that branch and it's still happening.

When using php artisan make:json-api:resource with the following in the json-api config file:
'by-resource' => false, 'use-eloquent' => true,

The correct file names are generated but the underlying class names are incorrect (Adapters/User.php houses 'Adapter' class instead of 'User')

@lindyhopchris
Copy link
Member

Ah, ok I might have misinterpreted what you meant! I thought you meant the file name was wrong rather than the class name. I'll double check (will be tomorrow) and fix.

As a side note, it hasn't been a waste of time upgrading to the alpha.1 branch as I'll be tagging that early next week.

@Wizofgoz
Copy link
Author

That's awesome! I really like the changes you made for 1.0, keep up the good work!

lindyhopchris added a commit that referenced this issue Apr 27, 2018
When generating a file when the `by-resource` option was set to
false, the wrong class name was used in the generated file.
This commit adds tests and the fix.

Fixes #169
@lindyhopchris
Copy link
Member

Thanks for reporting, I don't think this has ever worked but it is now and is tested.

It does create the problem that there's an App\Post import statement in the Eloquent adapter, which collides with class Post for the adapter class name, but not a lot I can do about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants