knuckleswtf / scribe

Generate API documentation for humans from your Laravel codebase.✍
https://scribe.knuckles.wtf/laravel/
MIT License
1.59k stars 280 forks source link

[update] nullable Parameter For OpenApi3 #739

Open liverocaisson opened 9 months ago

liverocaisson commented 9 months ago

If not required field allow nullable parameter as OpenAPI suggest

shalvah commented 9 months ago

You may need to update the test fixtures.

shalvah commented 7 months ago

I think you can update this now that #755 is merged.

jfoliveira commented 1 month ago

@liverocaisson nice one! Could you merge this PR? Exporting nullable: true for the nullable fields will be very useful! 🤞

liverocaisson commented 1 month ago

Would like to merge it but test fixtures dont pass. I don't know how to write scribe tests.

jfoliveira commented 1 month ago

I don't know how to write scribe tests.

@liverocaisson I'm also not experienced with scribe code base, but I cloned your branch and made some local changes and tests seem to be passing now.

Could you apply the diffs below into your branch and update this PR?

1) File: tests/Unit/OpenAPISpecWriterTest.php

diff --git a/tests/Unit/OpenAPISpecWriterTest.php b/tests/Unit/OpenAPISpecWriterTest.php
index 41990675..7e6e77ce 100644
--- a/tests/Unit/OpenAPISpecWriterTest.php
+++ b/tests/Unit/OpenAPISpecWriterTest.php
@@ -234,6 +234,7 @@ class OpenAPISpecWriterTest extends TestCase
                 'type' => 'string',
                 'description' => 'A query param',
                 'example' => 'hahoho',
+                'nullable' => true,
             ],
         ], $results['paths']['/path1']['get']['parameters'][0]);
     }
@@ -341,6 +342,7 @@ class OpenAPISpecWriterTest extends TestCase
                                 'description' => 'String param',
                                 'example' => 'hahoho',
                                 'type' => 'string',
+                                'nullable' => true,
                             ],
                             'booleanParam' => [
                                 'description' => 'Boolean param',
@@ -361,8 +363,10 @@ class OpenAPISpecWriterTest extends TestCase
                                         'description' => 'Object param field',
                                         'example' => 119.0,
                                         'type' => 'number',
+                                        'nullable' => true,
                                     ],
                                 ],
+                                'nullable' => true,
                             ],
                         ],
                         'required' => [
@@ -384,11 +388,13 @@ class OpenAPISpecWriterTest extends TestCase
                                 'description' => 'File param',
                                 'type' => 'string',
                                 'format' => 'binary',
+                                'nullable' => true,
                             ],
                             'numberArrayParam' => [
                                 'description' => 'Number array param',
                                 'example' => [186.9],
                                 'type' => 'array',
+                                'nullable' => true,
                                 'items' => [
                                     'type' => 'number',
                                 ],
@@ -397,6 +403,7 @@ class OpenAPISpecWriterTest extends TestCase
                                 'description' => 'Object array param',
                                 'example' => [[]],

2) File: tests/Fixtures/openapi.yaml

diff --git a/tests/Fixtures/openapi.yaml b/tests/Fixtures/openapi.yaml
index 9ea3a135..e4bedbbd 100644
--- a/tests/Fixtures/openapi.yaml
+++ b/tests/Fixtures/openapi.yaml
@@ -125,6 +125,7 @@ paths:
                         type: string
                         description: 'The filters.'
                         example: consequatur
+                        nullable: true
                 -
                     in: query
                     name: url_encoded
@@ -135,6 +136,7 @@ paths:
                         type: string
                         description: 'Used for testing that URL parameters will be URL-encoded where needed.'
                         example: '+ []&='
+                        nullable: true
                 -
                     in: header
                     name: Custom-Header
@@ -192,6 +194,7 @@ paths:
                         type: string
                         description: ''
                         example: consequatur
+                        nullable: true
                 -
                     in: header
                     name: Custom-Header
@@ -283,6 +286,7 @@ paths:
                         schema:
                             type: array
                             description: Details.
+                            nullable: true
                             example:
                             - first_name: 'John'
                               last_name: 'Doe'
@@ -310,4 +314,3 @@ tags:
     -
         name: Other<F0><9F><98><8E>
         description: ''
-
liverocaisson commented 1 month ago

@jfoliveira @shalvah I have added your test but still dont pass Maybe I made a mistake or still need to add some fixtures.

jfoliveira commented 1 month ago

@liverocaisson It seems a few more changes were needed. Could you please apply the diffs below into your branch and update this PR again?

1) File: tests/Unit/OpenAPISpecWriterTest.php

diff --git a/tests/Unit/OpenAPISpecWriterTest.php b/tests/Unit/OpenAPISpecWriterTest.php
index d5a20b17..7e6e77ce 100644
--- a/tests/Unit/OpenAPISpecWriterTest.php
+++ b/tests/Unit/OpenAPISpecWriterTest.php
@@ -403,6 +403,7 @@ class OpenAPISpecWriterTest extends TestCase
                                 'description' => 'Object array param',
                                 'example' => [[]],
                                 'type' => 'array',
+                                'nullable' => true,
                                 'items' => [
                                     'type' => 'object',
                                     'required' => ['field1'],
@@ -419,6 +420,7 @@ class OpenAPISpecWriterTest extends TestCase
                                             'type' => 'string',
                                             'description' => '',
                                             'example' => "hi",
+                                            'nullable' => true,
                                         ],
                                     ],
                                 ],

2) File: tests/Fixtures/openapi.yaml

diff --git a/tests/Fixtures/openapi.yaml b/tests/Fixtures/openapi.yaml
index 4ffc86bd..e4bedbbd 100644
--- a/tests/Fixtures/openapi.yaml
+++ b/tests/Fixtures/openapi.yaml
@@ -120,23 +120,23 @@ paths:
                     name: filters
                     description: 'The filters.'
                     example: consequatur
-                    nullable: true
                     required: false
                     schema:
                         type: string
                         description: 'The filters.'
                         example: consequatur
+                        nullable: true
                 -
                     in: query
                     name: url_encoded
                     description: 'Used for testing that URL parameters will be URL-encoded where needed.'
                     example: '+ []&='
-                    nullable: true
                     required: false
                     schema:
                         type: string
                         description: 'Used for testing that URL parameters will be URL-encoded where needed.'
                         example: '+ []&='
+                        nullable: true
                 -
                     in: header
                     name: Custom-Header
@@ -314,4 +314,3 @@ tags:
     -
         name: Other😎
         description: ''
-
jfoliveira commented 5 days ago

@liverocaisson It seems a few more changes were needed

Hey @liverocaisson Could you apply those changes into your branch and update this PR?