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

Fix strstr modifier #35705

Merged
merged 2 commits into from
Apr 3, 2024
Merged

Fix strstr modifier #35705

merged 2 commits into from
Apr 3, 2024

Conversation

Hlavtox
Copy link
Contributor

@Hlavtox Hlavtox commented Mar 21, 2024

Questions Answers
Branch? 8.1.x
Description? Fixes issue PHP Deprecated: Using php-function "strstr" as a modifier is deprecated and will be removed in a future release. Use Smarty::registerPlugin to explicitly register a custom modifier.
Type? improvement
Category? FO
BC breaks? no
Deprecations? no
How to test?
UI Tests https://github.com/florine2623/testing_pr/actions/runs/8523037501
Fixed issue or discussion?
Related PRs
Sponsor company

@Hlavtox Hlavtox added this to the 8.1.6 milestone Mar 21, 2024
@Hlavtox Hlavtox requested a review from a team as a code owner March 21, 2024 07:59
@prestonBot prestonBot added 8.1.x Branch Improvement Type: Improvement labels Mar 21, 2024
config/smarty.config.inc.php Outdated Show resolved Hide resolved
@ps-jarvis ps-jarvis added the Waiting for author Status: action required, waiting for author feedback label Mar 21, 2024
Co-authored-by: Tom Zajac <SharakPL@users.noreply.github.com>
@ps-jarvis ps-jarvis added the Waiting for QA Status: action required, waiting for test feedback label Mar 21, 2024
@SharakPL SharakPL removed the Waiting for author Status: action required, waiting for author feedback label Mar 22, 2024
@florine2623
Copy link
Contributor

Hello @Hlavtox ,

Could you share the How to test steps please ? :)

Thanks!

@florine2623 florine2623 added the Waiting for author Status: action required, waiting for author feedback label Mar 25, 2024
@Hlavtox
Copy link
Contributor Author

Hlavtox commented Apr 2, 2024

@florine2623 I think that green UI tests suffice, it's the same as all of the other modifiers

@prestashop-issue-bot prestashop-issue-bot bot removed the Waiting for author Status: action required, waiting for author feedback label Apr 2, 2024
@florine2623 florine2623 self-assigned this Apr 2, 2024
Copy link
Contributor

@florine2623 florine2623 left a comment

Choose a reason for hiding this comment

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

The auto tests are broken : https://github.com/florine2623/testing_pr/actions/runs/8523037501/job/23383999023

@PrestaShop/qa-automation do you think this is related to this PR ?

Thanks!

@florine2623 florine2623 removed their assignment Apr 3, 2024
@Progi1984
Copy link
Member

The auto tests are broken : https://github.com/florine2623/testing_pr/actions/runs/8523037501/job/23383999023

@PrestaShop/qa-automation do you think this is related to this PR ?

Thanks!

No, it's not linked.

@florine2623
Copy link
Contributor

Thanks, so I can approve @Hlavtox 's PR ^^

@florine2623 florine2623 added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Apr 3, 2024
@SharakPL SharakPL merged commit 2264110 into PrestaShop:8.1.x Apr 3, 2024
38 checks passed
@SharakPL
Copy link
Contributor

SharakPL commented Apr 3, 2024

@florine2623 since I don't see this modifier used anywhere in the PS, you may want to create a test case. For example change themes/classic/templates/index.tpl like so:

obraz

and then open the front page. It should show @domain.com text right before imageslider, but without this PR it will also throw a deprecation notice

obraz

@matthieu-rolland matthieu-rolland modified the milestones: 8.1.6, 8.1.7 May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.1.x Branch Improvement Type: Improvement QA ✔️ Status: check done, code approved
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

9 participants