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

Update behaviour for immutable and computed properties #837

Merged
merged 17 commits into from
Sep 19, 2024

Conversation

yvo-niedrich
Copy link
Contributor

@yvo-niedrich yvo-niedrich commented Jul 17, 2024

OpenAPI Documentation

  • Display computed properties in the Create-OpenApi, if it is also annotated as ComputedDefaultValue
  • Hide immutable properties in the Update-OpenApi
  • Include GeneratedProperties (in addition to Declared) when generating the Entity OpenAPI documentation

Property Filtering

  • Strips computed fields from create operations, unless the property is also annotated as ComputedDefaultValue
  • Strips computed fields from update operations

Annotations

  • Enum can be annotated as nullable and immutable

Changes

  • Default drivers key properties (with a default value) are annotated as Computed - instead of ComputedDefault
  • Guid properties are properly validated (thanks @sean-james-eco)

Bugfixes

  • Initialise the $cookie property of a request when converting Illuminate\Request. Fixes "Request::$cookies must not be accessed before initialization" when Telescope and APP_DEBUG are enabled.
  • EloquentEntitySet::create complies with attribute.source for navigation source (thanks @sean-james-eco)
  • Prevent deprecation warnings during boot with Laravel 11.x

@yvo-niedrich
Copy link
Contributor Author

yvo-niedrich commented Jul 24, 2024

Found the docker command to boot up mongo - all test snapshots are updated now: Generated properties are listed in the OpenAPI documentation when describing an Entity.

Ready for merge (in my opinion)

@27pchrisl
Copy link
Contributor

Hi @yvo-niedrich, thanks for the work! On the computed default properties I'm not sure it's correct to indicate those can be provided during a create operation, as they are still computed by the server and not the client?

@27pchrisl
Copy link
Contributor

Ah, no you're right, "A value for this property can be provided by the client on insert and update. If no value is provided on insert, a non-static default value is generated"

@27pchrisl
Copy link
Contributor

and in that case, the entity set drivers right now are incorrectly annotating with Computed, not ComputedDefaultValue - would you mind making that change as part of this?

@yvo-niedrich
Copy link
Contributor Author

Ah, I believe I see what you mean. This one for example - right?

I'll go ahead and completely separate Computed from ComputedDefault in the types then... Will do tomorrow.

@27pchrisl
Copy link
Contributor

In

$property->addAnnotation(new ComputedDefaultValue);

and
$this->setKey((new DeclaredProperty('offset', Type::int64()))->addAnnotation(new ComputedDefaultValue));
for example, the standard drivers should be using Computed here.

@yvo-niedrich
Copy link
Contributor Author

yvo-niedrich commented Aug 7, 2024

Had some time to spare and updated the PR already 🙈

I've also included another change (again thanks to @sean-james-eco) to properly validate UUIDs. Now a 400 error is returned - instead of the DatabaseException (HTTP 500) when the Eloquent Driver tries to write an invalid value to the database.

@yvo-niedrich
Copy link
Contributor Author

Hey @27pchrisl are we still missing some of the updates you mentioned? I thought I caught them all 🤔

@27pchrisl
Copy link
Contributor

@yvo-niedrich Nope! I just hadn't got round to reading all the changes : ) I'll tag shortly...

@27pchrisl 27pchrisl merged commit b812db2 into flat3:5.x Sep 19, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants