- Issue created by @Liam Morland
- First commit to issue fork.
- last update
about 1 year ago Custom Commands Failed - @elber opened merge request.
- last update
about 1 year ago Custom Commands Failed - last update
about 1 year ago 29,490 pass, 15 fail - last update
about 1 year ago 29,490 pass, 15 fail - Status changed to Needs review
about 1 year ago 7:40pm 27 June 2023 - Status changed to Needs work
about 1 year ago 7:42pm 27 June 2023 - π¨π¦Canada Liam Morland Ontario, CA π¨π¦
The file_save_upload() function needs to continue to exist because existing code still uses it. The function should emit a deprecation notice and then call the new service. There should be a test for the deprecation notice.
Tests need to be passing before marking this "Needs review".
- π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
I was sure there was an issue for this already, but I might be confusing with π [PP-4] Unify file upload logic of REST and JSON:API Postponed
- π¨π¦Canada Liam Morland Ontario, CA π¨π¦
You may also been thinking of the other related ones.
- last update
about 1 year ago 29,493 pass, 15 fail - last update
about 1 year ago Custom Commands Failed - last update
about 1 year ago Custom Commands Failed - last update
about 1 year ago 29,560 pass, 1 fail - π§π·Brazil elber Brazil
Hi does anyone has an idea how to fix this test ?
- π¨π¦Canada Liam Morland Ontario, CA π¨π¦
The patch adds
file_save.upload
service.AutowireTest::testCoreServiceAliases()
is looking for a particular list of services. The new service needs to be added to the test. - last update
about 1 year ago 29,561 pass, 1 fail - π§π·Brazil elber Brazil
The patch adds file_save.upload service. AutowireTest::testCoreServiceAliases() is looking for a particular list of services. The new service needs to be added to the test.
Dou you have an idea how to do that?
- π¨π¦Canada Liam Morland Ontario, CA π¨π¦
Look at
AutowireTest::testCoreServiceAliases()
to see what it is testing. There will be a place where it is testing things likefile.validator
andcontent_translation.synchronizer
. The new service needs to be added. - Open on Drupal.org βEnvironment: PHP 8.2 & MySQL 8last update
about 1 year ago Not currently mergeable. - last update
about 1 year ago Custom Commands Failed - last update
about 1 year ago 26,891 pass, 745 fail - π§π·Brazil elber Brazil
Hi I'm seeing tests failed because deprecation notices should I remove the deprecation things and then remove the tests about it ?
see an example:
24x: /app/core/modules/file/file.field.inc is deprecated in drupal:10.2.0 and is removed from drupal:11.0.0. Functions have been moved to file.module. See https://www.drupal.org/node/3369330 24x in BlockContentListTest::testListing from Drupal\Tests\block_content\Functional Other deprecation notices (1) 1x: /app/core/modules/file/file.field.inc is deprecated in drupal:10.2.0 and is removed from drupal:11.0.0. Functions have been moved to file.module. See https://www.drupal.org/node/3369330 1x in BlockContentListTest::testListing from Drupal\Tests\block_content\Functional
- last update
about 1 year ago 26,889 pass, 746 fail - last update
about 1 year ago 29,566 pass - Status changed to Needs review
about 1 year ago 6:54pm 30 June 2023 - π¨π¦Canada Liam Morland Ontario, CA π¨π¦
Thanks for the patch.
file_save_upload()
should be changed so that it uses the new service and emits a deprecation notice. Look at other functions that became services to see an example.It might be deprecated in some version of Drupal 10 and removed in Drupal 11. The @deprecated comment could be written to reflect this.
- last update
about 1 year ago 29,571 pass - π§π·Brazil elber Brazil
Hi @Liam Morland I tried follow you suggestion, please can you see if that's correct?
- π¨π¦Canada Liam Morland Ontario, CA π¨π¦
You don't need the line with "move this logic to a service"; it's being done here.
You don't need the references to: https://www.drupal.org/node/2244513 β
Calling
file_save_upload()
needs to trigger a deprecation notice to appear in logs so that module maintainers now to update their code. - Status changed to Needs work
about 1 year ago 4:11pm 2 July 2023 - π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
Added the meta as a parent π [META] Modernise file upload logic Active .
I think we need to look at the logic in the call stack and see if we can make an improvement across all of these:
file_managed_file_save_upload()
_file_save_upload_from_form()
file_save_upload()
FileUploadHandler::handleFileUpload()
Let's discuss in the parent and devise a plan.
- Status changed to Closed: outdated
about 1 year ago 11:28pm 28 August 2023 - π¦πΊAustralia kim.pepper πββοΈπ¦πΊSydney, Australia
Closing this as we're looking to do all three at the same time in π Deprecate file_managed_file_save_upload(), file_save_upload() and _file_save_upload_from_form() and replace with a service Needs review