-
Notifications
You must be signed in to change notification settings - Fork 183
Optionally expose excluded routes #752
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
base: main
Are you sure you want to change the base?
Conversation
|
Probably would be good to be able to optionally ignore hidden properties if that all access flag is enabled too. This way if, when using laravel-data for instance with conditional lazy properties that will only be included with appropriate access, they can be hidden from api docs, but optionally rendered for a user with adequate permission. |
|
Hey @willvincent Thanks for the PR! Have you considered creating 2 separate documentations with multiple API approach? If yes, why it didn't work? |
|
Yes I have multiple docs, but this is all one API routes don't differ, just some of it is going to be publicly accessible and some won't be - permissions based, not route based. In fact I want to control what's available to users based on permissions at the same routes. For example an internal user (or our own frontend app) might get 15 properties on a route, that I may only want 4 or 5 to be available to general users. Some routes will be internal only, but it's not strictly route-related. Hope that's clear, long day.. ;) |
|
The problem is complex, so I'd handle them separately: internal routes (the ones that you mark as excluded) and then internal properties. Let's handle internal routes for now as it complex enough in itself. I still think multiple API approach should work here or at least I don't see why not. Multiple API approach is not forcing you to have strictly different routes in different APIs. So for example, let's assume we have some sort of API where class SiteController
{
public function index()
{
// this route is available for general users
}
public function store()
{
// this route is available only for internal users
}
}With multiple API approach you'll have something like this: public function boot()
{
Scramble::configure(api: 'default')
->routes(function (Route $route) {
return Str::startsWith($route->uri, 'api/')
&& in_array($route->methods(), 'GET');
})
Scramble::registerApi('internal');
}In this case Would it solve the problem or I'm missing something? |
|
Not really, my use case is more involved than that.. I guess I don't understand the push back to having an option to be able to render everything, whether the majority of people would use it or not. Even if only for the artisan generate command. 🤷 |
|
I would have to explicitly define many routes, to setup yet another doc config for internal or not. And it still doesn't address my other comment (nor does my PR) about being able to set properties hidden, but then also expose those given appropriate access, or .. again, manually generating docs on the cli. The ultimate goal here is I'd like to be able to have a full and complete document of everything for my internal team. and by way of excluding routes and hiding properties, provide a curated subset of routes and request and response parameters. Ideally I'd like to not have to manage a separate doc config which would be.. substantial effort. I have several hundred routes to wrangle here. 😂 |
|
My goal here is understand your case better, not just to push back. This helps me to see if there is already an existing way to solve the problem without introducing new features. Because the problem you experience may be a signal that there is something missing in already existing set of tools. And it would be hard to me to maintain the features I don't fully understand. For example, what if there was an attribute that allows you to explicitly tell the route which API documentation it belongs to? This way to don't need to configure APIs separately, just register an internal one. class SiteController
{
public function index()
{
// this route is available in both `default` and `internal` documentation
}
#[Api('internal')]
public function store()
{
// this route is available only for `internal` documentation
}
} |
|
That could work - would be very useful to have available for properties too in request and response.. defined via validation rules, laravel-data properties, manually configured, etc. Not sure how that would work if things are defined in comments, er.. I guess we already have the I do like the annotation approach, and it's a pattern already in play, so it does make sense to lean into that even more. |
|
Awesome. Let's keep this PR open in this case, I will try how it feels with new attribute and will get back to you. |
Addresses Issue #751
Adds an
--alloption to the export command that ignoresExcludeRouteFromDocsandExcludeAllRoutesFromDocsattributesAlso ignores
ExcludeRouteFromDocsandExcludeAllRoutesFromDocsin the UI if the request has the hiddenContextscramble-all-access:One need simply set the hidden context for appropriate users' requests, thusly: