Skip to content

Comments

Release v1.2.1#127

Merged
LukeTowers merged 43 commits into1.2from
develop
Oct 20, 2022
Merged

Release v1.2.1#127
LukeTowers merged 43 commits into1.2from
develop

Conversation

@LukeTowers
Copy link
Member

No description provided.

LukeTowers and others added 30 commits July 19, 2022 17:33
Without this morphedByMany relationships use the incorrect class name when building queries.
- Add missing model.relation.before/after events
- New HasSortableRelations trait

Co-authored-by: Tobias Kündig <tobias@offline.swiss>
Co-authored-by: Marc Jauvin <marc.jauvin@gmail.com>
Regenerate resizer fixtures.
This is required to allow converting images to other formats (e.g. webp). Otherwise the resulting file will have the correct extension but the wrong image format.

Fixes #9 & #74
Uses the https://github.com/wintercms/laravel-config-writer repo to provide configuration modification. The original classes in Storm have been stubbed so that they continue to work as documented.
Laravel's "Illuminate\Http\Concerns\InteractsWithInput::all()" method provides all values (query and request) as well as files. The override prevented files from being included.
This reinstates the behaviour originally present in fideveloper/trustedproxy where setting ** as the value for app.trustedProxies would allow all proxies vs * which would only allow the most recent one in a chain of proxies (as determined by $_SERVER['REMOTE_ADDR']). See fideloper/TrustedProxy@6018dfb for when & why it was originally added.

The '**' wildcard was removed in v4 of that package (fideloper/TrustedProxy@1d09591) with no explanation and was never added back in when Laravel merged it into the core in laravel/framework#38295.

This causes problems for environments where you have multiple proxies in a chain (i.e. Amazon CloudFront in front of Amazon ELB). These problems are documented in fideloper/TrustedProxy#115 & fideloper/TrustedProxy#107, and spawned fideloper/TrustedProxy#142 & https://github.com/ge-tracker/laravel-vapor-trusted-proxies to resolve them.

Ultimately, this commit serves to reintroduce the original behaviour of fideveloper/trustproxies v3 and make it so that you can use `**` as the value for app.trustProxies in order to get the correct client IP address when running Winter on Laravel Vapor.
When adding dynamically created properties to models, you have two choices:

1. You can either do it through direct assignment which eventually passes through Laravel's attribute system and stores the dynamic property in the $attributes property, thus flagging it for insertion into the database like a regular property; or
2. You can add it through `addDynamicProperty()` which currently ultimately ends up doing the same thing as 1 except that it also gets added to the list of dynamic properties on the model through the ExtendableTrait.

The key difference in choosing to use dynamic properties instead of model attributes usually comes down to whether or not you want your newly added property to be stored in the database. Most commonly, you would choose to use addDynamicProperty() when adding a logical property that's used for performing logic on the model (i.e. adding a ModelBehavior to the model with extendClassWith() that requires properties to be present in order to configure itself).

This assumption is present in the PurgeableBehavior itself currently (see https://github.com/wintercms/storm/blob/c972c1f50cce9c5c150278facb86e1caf9dd69ed/src/Database/Behaviors/Purgeable.php#L34-L36) which automatically adds any existing defined dynamic properties to the purgeable array during the process of booting the PurgeableBehavior when it is added to a class (a key part of the original design considerations: octobercms/library#324 (comment)). 

In addition to that, the Winter.Translate plugin has to go out of its way to explicitly add the Purgeable behavior whenever it adds it's own TranslateableModel behavior (see https://github.com/wintercms/wn-translate-plugin/blob/344711704f8ec0556295a02714cb2a6f48f9dd27/Plugin.php#L79-L81). 

It has also popped up in issues (octobercms/october#3433) before as a source of developer confusion. 

Given all of the above information, it makes sense to have the Model class itself automatically add the PurgeableBehavior if not already present as soon as a single dynamic property is defined so that all dynamic properties can be automatically added to the list of purgeable properties as soon as they are defined.

In the future, it might make sense to just make the Purgeable trait itself included by default in the base model in order to not have to dynamically & automatically add the Purgeable behavior under these conditions, but I leave that as a task for later.
… core model

Follow up to a2253ef, @bennothommo pointed out that it made more sense to just include the Purgeable trait on the base model itself.
Port of ee91a88 from the 1.1 branch to 1.2.
Adds support for creating a file model from a file on the disk returned by File->getDisk() via the fromStorage() method.

Useful for processing file uploads to remote disks where the file is uploaded directly to the remote disk instead of first touching the application server.
…omStorage()

The file path provided could be a UUID string inside of a temporary directory so we need to allow for the file_name and content_type to be explicitly set.
Type hint requires that a value is always returned.
Enables easily performing cleanup tasks on CLI commands through a cross platform interface by implementing the trait and a handleCleanup() method.

Also reorgs the base command logic into traits as applicable.
Includes testing of the chained proxies introduced in 411695b, and adjusts the scenarios to more closely test according to how the TrustedProxy library tested.

Fixes wintercms/winter#637
Fix for Event::listen('event-name', 'ClassName@method') in L9 when ClassName is not fully qualified but has been bound to the App with $this->app->bind('ClassName', \Path\To\ClassName::class);
jaxwilko and others added 13 commits September 21, 2022 10:37
The `EXISTS` sql statement will return true at the first existing value where the `COUNT` will uselessly continue to loop through the whole table which is inaccurate for this use case.
Fixes wintercms/winter#714

Allows systems without GLOB_BRACE support (ie. Solaris, Alpine Linux) to still correctly include hidden files in a Zip by emulating the brace search using multiple glob passes.
Co-authored-by: Luke Towers <github@luketowers.ca>
…ator. (#122)

This fixes the issue with the arrays not being merged properly with auto-incremented keys. See #120 (comment)
This reverses the order for checking mail configs by searching for the new format first, then checking for the old format.

Laravel checks old format first, then new format, which works fine for them, but doesn't work for us as we also populate settings via the Backend, which populates the settings in the new format.

Fixes wintercms/winter#626
@LukeTowers LukeTowers merged commit 6ec99ea into 1.2 Oct 20, 2022
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.

7 participants