🇦🇺Australia @kim.pepper

I agree! Why is deleting code much more satisfying that writing code!? I love it. 🤣

Committed to 1.x. Thanks!

Depends when people can review it and we can merge it.

Thanks for the fix. Can you also provide a test to show the bug (fails on current HEAD ie. with the test-only pipeline) and is fixed in the MR. Thanks! 🙏🏻

Seemed like a random fail. Rebased on main and back to green.

Updated the project description to mention you need Purge URLs queuer → or your own custom code.

Done.

IMO using the interface for constants is more future proof than assuming it will always be there on a class that implements the interface.

New developments and disruptive changes should now be targeted to the

main

branch.

How about:

Merge Requests with new developments and disruptive changes should now be targeted to the

main

branch.

Made a minor tweak.

When can we expect a release with this fix?

Looks like this is ready for reviews @mohit_aghera?

They look unrelated:
core/modules/package_manager/tests/src/Kernel/PhpTufValidatorTest.php

Gin fixed this in #3560487: Gin Top Bar styling is fully gone in 11.3.x → so not sure if this is still necessary.

Possibly related issue 🐛 Top bar background color should use CSS variable Active

kim.pepper → created an issue. See original summary → .

Thanks @arjenk. Are you able to put together a MR for your suggested approach?

I've actually been doing this in issues for months and have never had pushback from the committers.

I think we should postpone this on 🐛 FileEventSubscriber::sanitizeFilename() does not check for invalid UTF-8 chars Needs review It would be great if those following this issue could review that.

Ran a test-only job to show the failure.

Ready for review.

Created 🐛 FileEventSubscriber::sanitizeFilename() does not check for invalid UTF-8 chars Needs review

kim.pepper → created an issue.

OK will take a look tomorrow.

I wonder if we should split off a new issue and consider it a bug for not handling invalid UTF-8?

I think a check at the top of FileEventSubscriber is the simplest approach.

Per #43 I added an explicit check in FileUploadHandler and changed from logging an error to throwing an exception in FileEventSubscriber::sanitizeFilename()

I feel we should have a separate check for invalid UTF-8 characters outside the whitespace replacement.

I tried checking for a NULL return value from preg_replace() and logging the error message.

Self RTBC'ing as per #4

I've been debugging but still can't figure out why change the whitespace setting causes the core/modules/file/tests/src/Functional/SaveUploadTest.php error message to go from:

"The file <em class="placeholder">x�xx.gif</em> could not be uploaded because the name is invalid."

to

The specified file <em class="placeholder">x�xx.gif</em> could not be uploaded.<ul><li>Only files with the following extensions are allowed: <em class="placeholder">jpg jpeg gif png txt doc xls pdf ppt pps odt ods odp</em>

...even though that extension is allowed. 🤔

This hasn't moved in a while so I'm creating a MR to just default to removing whitespace to see what breaks.

kim.pepper → created an issue.

Updated CR

I had another review of this. I think we should avoid adding service aliases for classes and just focus on adding aliases for interfaces if they are missing. Adding aliases for plugin managers doesn't make sense to me. We might want to consider updating classes injecting plugin managers to use PluginManagerInterface instead?

I added a couple of things I think should be changed.

Updated the list in the IS to exclude those services listed above.

@muriqui great to hear you got it working. Let's show it's fixed with the test. Leaving at NW for that.

Looks good. Did a search and couldn't find any more instances of private properties.

Woo! 🎉

Updated the CR to add more detailed explanation.

Added #3561135: Deprecate passing options array to UploadedFileConstraint → as I think this was missed due to it not being a plugin.

kim.pepper → created an issue.

No need to post the test results @eduardo morales alberti We can run test-only workflows and see the results in Gitlab.

I think we can take logger.channel.foo off the list.

OK sounds good. I updated the title and IS.

From #6

I guess I was trying to keep the scope of this issue to what is needed in the file module services. I bet there are a lot of other missing aliases. Do we want to try and find them and add them in this issue?

Rebased on 11.x and made use of the FileUsageInterface service alias.

Committed to 3.x and cherry-picked to 2.x. Thanks!

Addressed feedback.

kim.pepper → created an issue.

kim.pepper → created an issue.

Please keep your language respectful. I would remind you that you need to adhere to the code of conduct → .

It's not an unreasonable request to prove that a bug exists and that it is fixed with a test.

Thanks @oivanov. Are you able to provide a test to show this fixes the issue? e.g. the test should fail without the fix, and succeed with it.

Thanks for your patch. We need a MR to trigger the GitLab pipeline workflows.

Committed to 3.x and cherry-picked to 2.x. Thanks!

Left some feedback.

This is looking really good. Just some phpcs fails. Also I'm not 100% sure whether we need BC for the SearchParamBuilder constructor args.

kim.pepper → made their first commit to this issue’s fork.

My comment from #30 wasn't responded to:

I would prefer a way for users to add support for additional units rather than hard coding to what is in the spec.

ai_vdb_provider_opensearch → now requires the split out ai_search module 📌 Switch to drupal/ai_search project and drupal/ai 2.x Needs review

Committed to 1.x

Looks like there are specific security tests failing that expect that behaviour.

kim.pepper → made their first commit to this issue’s fork.

Committed to 1.x. Thanks!

I don't believe we need change records for removing deprecated code in major version updates.

Committed to 3.x and 2.x.

Committed to 3.x and 2.x. Thanks!

kim.pepper → made their first commit to this issue’s fork.

kim.pepper → created an issue.

kim.pepper → created an issue.

kim.pepper → created an issue.

kim.pepper → made their first commit to this issue’s fork.

I think it's fine to move it once ai_search is in its own module. It's pretty basic, and we aren't actually using the value objects anywhere yet.

Updated IS

Updated the title and IS to reflect the new approach.

Testing in PHPStorm this seems to work well.

Added the conditional return type as per #21

Added a new MR that just adds the class-string typehints to \Drupal::service().

Could we merge this docblock with the existing \Drupal::service() docblock somehow?

*  @template T of object
*   
*  @param class-string<T> $class
*
*   @return object&T
*       A service that is an instance of $class.

I would prefer a way for users to add support for additional units rather than hard coding to what is in the spec.

@gxleano you don't need to manually close issues. Fixed issues get automatically closed after 2 weeks (?).

From the discussion linked above https://github.com/design-tokens/community-group/issues/245

I think a similar approach would be a better way to address prototype/pre-spec implementations. The spec can allow parsers/transformers to ignore invalid tokens. If a vendor wants to add a unit that isn't in the spec yet, they just use it in the unit property as if it was valid and then end users can use a plugin or custom transform (perhaps written by the vendor) in the parser to handle it.

So just because it's not in the spec, doesn't mean we can't support the units we want in our implementation.

Not sure if we need to do anything with \Drupal\file\Validation\Constraint\UploadedFileConstraint ?

This got stalled most likely because it was trying to do too much.

I'm starting to split things off into smaller issues starting with 📌 Add a UploadedFilesExtractor and remove duplicate code Active

kim.pepper → created an issue.