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

PHPORM-150 Run CI on Laravel 11 #2735

Merged
merged 4 commits into from
Feb 22, 2024
Merged

PHPORM-150 Run CI on Laravel 11 #2735

merged 4 commits into from
Feb 22, 2024

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Feb 21, 2024

Fix PHPORM-150

Checklist

  • Add tests and ensure they pass
  • Add an entry to the CHANGELOG.md file
  • Update documentation for new features

@GromNaN
Copy link
Member Author

GromNaN commented Feb 21, 2024

Static analysis found a breaking change in HasAttributes::getStorableEnumValue(), which is hard to get around to support laravel 10 and 11 at the same time.

 ------ ---------------------------------------------------------------------------------- 
  Line   Eloquent/Model.php                                                                
 ------ ---------------------------------------------------------------------------------- 
  707    Method Illuminate\Database\Eloquent\Model::getStorableEnumValue() invoked with 1  
         parameter, 2 required.                                                            
         ✏️  Eloquent/Model.php                                                            
 ------ ---------------------------------------------------------------------------------- 

@@ -704,7 +708,7 @@ protected function addCastAttributesToArray(array $attributes, array $mutatedAtt
}

if ($this->isEnumCastable($key) && (! $castValue instanceof Arrayable)) {
$castValue = $castValue !== null ? $this->getStorableEnumValue($castValue) : null;
Copy link
Member Author

Choose a reason for hiding this comment

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

The signature of this method have changed in a non-backward compatible way. So I duplicated this simple method to keep compatibility with both Laravel 10 and 11.
laravel/framework@8647dcf#diff-59d24f1a8f0dd1e51ee7dffc8778133711634e928ecef4a91b63fc3851cb1579R1188

We must keep the function addCastAttributesToArray introduced for Laravel 10 compatibility, as it provides casting of nested fields. 2ea1a7c#diff-284e164f76aeb13f424d154f9208d91fdbb0112b0a50ad8b1dfa237176f03cf2R525-R530

@@ -614,11 +614,7 @@ public function orderBy($column, $direction = 'asc')
return $this;
}

/**
* @param list{mixed, mixed}|CarbonPeriod $values
Copy link
Member Author

Choose a reason for hiding this comment

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

This signature was causing psalm error. Param type incompatible with the parent method.

@GromNaN GromNaN marked this pull request as ready for review February 21, 2024 20:36
@GromNaN GromNaN requested review from alcaeus and jmikola February 21, 2024 20:37
# Laravel 11
- php: "8.3"
mongodb: "7.0"
mode: "dev"
Copy link
Member Author

Choose a reason for hiding this comment

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

Once Laravel 11 will be released, this will no longer be necessary. But only low-deps job will run Laravel 10.

Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

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

Looks good, I have a minor nit but considering the build passes it's not too relevant.

.github/workflows/build-ci.yml Show resolved Hide resolved
@GromNaN GromNaN merged commit 9b72148 into mongodb:4.2 Feb 22, 2024
15 checks passed
@GromNaN GromNaN deleted the PHPORM-150 branch February 22, 2024 10:04
@GromNaN GromNaN mentioned this pull request Mar 5, 2024
3 tasks
alcaeus added a commit that referenced this pull request Mar 13, 2024
* 4.2:
  PHPORM-139 Implement `Model::createOrFirst()` using `findOneAndUpdate` operation (#2742)
  Test Laravel 10 and 11 (#2746)
  PHPORM-150 Run CI on Laravel 11 (#2735)
  PHPORM-152 Fix tests for Carbon 3 (#2733)
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.

2 participants