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

Fixed #11871 Replacing (deprecated) patchwork/utf8 by polyfill/mbstring. #11872

Merged
merged 1 commit into from
Sep 27, 2022

Conversation

sunflowerbofh
Copy link
Contributor

Signed-off-by: Katharina Drexel [email protected]

Description

When doing a composer install you always get the warning
"Package patchwork/utf8 is abandoned, you should avoid using it. Use symfony/polyfill-mbstring or symfony/string instead."
(s.a. https://packagist.org/packages/patchwork/utf8)

Fixes # (issue)

-> Replaced patchwork/utf8 by symfony/polyfill-mbstring and Patchwork\Utf8::utf8_encode by Polyfill\Mbstring::mb_convert_encoding

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Cretaed an asset and put some non-ascii characters* in the notes field. Checked if evrything was readable/editable afterwards.

  • déjà σσς iıii
    DÉJÀ Σσς Iıİi
    DÉJÀ ΣΣΣ IIİI
    Déjà Σσς Iıİi
    L’École supérieure de physique et de chimie industrielles de la ville de Paris, ou ESPCI ParisTech, est une grande école d’ingénieurs fondée en 1882. Elle est située rue Vauquelin sur la montagne Sainte-Geneviève dans le cinquième arrondissement de Paris.'
    à-à-ÉÈà-à-
    请按照以下说明保留您的当前密码并更新您的帐户。
    点击这里确认验证

Test Configuration:

  • PHP version: 8.1.5
  • MySQL version 10.6
  • Webserver version Apache/2.4.54
  • OS version Debian bookworm

Checklist:

@probot-autolabeler probot-autolabeler bot added backend dependencies Pull requests that update a dependency file labels Sep 26, 2022
@snipe
Copy link
Owner

snipe commented Sep 27, 2022

Non-ASCII strings in the notes would likely not matter here. We invoke that library when converting non-ASCII database column names within custom fields. You can see https://github.com/snipe/snipe-it/blob/master/tests/Unit/CustomFieldTest.php for examples.

GitHub
A free open source IT asset/license management system - snipe-it/CustomFieldTest.php at master · snipe/snipe-it

@snipe
Copy link
Owner

snipe commented Sep 27, 2022

Unfortunately, this seems to break on PHP7.4 when I test it, which is our min requirement :(

@snipe
Copy link
Owner

snipe commented Sep 27, 2022

Aha - I think I got this working now. Thanks!

@snipe snipe merged commit 055a2f8 into snipe:develop Sep 27, 2022
@snipe
Copy link
Owner

snipe commented Sep 27, 2022

Hm, now I'm getting

[2022-09-27 16:59:12] production.ERROR: Error: Class 'Polyfill\Mbstring' not found in /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/app/Models/CustomField.php:341
Stack trace:
#0 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/storage/framework/views/76c26f48cd7db2b1357ea3f64cc32b06dba7eb32.php(149): App\Models\CustomField->convertUnicodeDbSlug()
#1 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/vendor/laravel/framework/src/Illuminate/Filesystem/Filesystem.php(107): require('/home/serverpil...')
#2 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/vendor/laravel/framework/src/Illuminate/Filesystem/Filesystem.php(108): Illuminate\Filesystem\Filesystem::Illuminate\Filesystem\{closure}()
#3 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/vendor/laravel/framework/src/Illuminate/View/Engines/PhpEngine.php(58): Illuminate\Filesystem\Filesystem->getRequire()
#4 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/vendor/livewire/livewire/src/ComponentConcerns/RendersLivewireComponents.php(69): Illuminate\View\Engines\PhpEngine->evaluatePath()
#5 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/vendor/laravel/framework/src/Illuminate/View/Engines/CompilerEngine.php(61): Livewire\CompilerEngineForIgnition->evaluatePath()
#6 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/vendor/facade/ignition/src/Views/Engines/CompilerEngine.php(37): Illuminate\View\Engines\CompilerEngine->get()
#7 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/vendor/livewire/livewire/src/ComponentConcerns/RendersLivewireComponents.php(35): Facade\Ignition\Views\Engines\CompilerEngine->get()
#8 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/vendor/laravel/framework/src/Illuminate/View/View.php(139): Livewire\CompilerEngineForIgnition->get()
#9 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/vendor/laravel/framework/src/Illuminate/View/View.php(122): Illuminate\View\View->getContents()
#10 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/vendor/laravel/framework/src/Illuminate/View/View.php(91): Illuminate\View\View->renderContents()
#11 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/vendor/laravel/framework/src/Illuminate/Http/Response.php(69): Illuminate\View\View->render()
#12 /home/serverpilot/apps/develop.snipeitapp.com/snipe-it/vendor/laravel/framework/src/Illuminate/Http/Response.php(35): Illuminate\Http\Response->setContent()

I might have to revert this one.

@sunflowerbofh
Copy link
Contributor Author

Sorry, I only tested in php8, and it worked there for some reasons. Whithout path and a second parameter I got now rid of the error also in php 7.4. Just pushed the changes, further tests would be appreciated.

@sunflowerbofh sunflowerbofh mentioned this pull request Sep 28, 2022
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants