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: PHP 8: Views Handlers: Filter: in_operator - Array to string conversion #5313

Closed
indigoxela opened this issue Oct 20, 2021 · 9 comments · Fixed by backdrop/backdrop#3799

Comments

@indigoxela
Copy link
Member

indigoxela commented Oct 20, 2021

Description of the bug

Part of #5076

Failing tests with PHP 8:

---- Views Handlers: Filter: in_operator (ViewsHandlerFilterInOperator) ----

Status    Group      Filename          Line    Function                            
---------------------------------------------------------------------------------------------------
Exception Warning    views.module      1888                                                                            
    Array to string conversion
...

Reason: str_replace() chokes on nested array in $condition['value']

Related Drupal commit and issue

https://git.drupalcode.org/project/views/-/commit/75ea48e0e6bdae213c960090a0c8aea46db0c69a

https://www.drupal.org/project/views/issues/3207982

There are additional changes in that commit.

  1. Small change in function postComment - not necessary in Backdrop, but doesn't do any harm (the actual functionality has been fixed for Backdrop in initial PHP 8 support PR.
  2. Field test enhancement, which does not belong to the in_operator fix, but to Fixed: PHP 8: Views Plugins: Cache / Views fields for Field API #5314 - without that other fix, the test would fail, so moving the changes over there
@indigoxela
Copy link
Member Author

A PR is available for review. The ViewsHandlerFilterInOperator test is green now with PHP 8.

@klonos klonos added this to the 1.20.2 milestone Oct 20, 2021
@klonos
Copy link
Member

klonos commented Oct 20, 2021

There are additional changes for code, that doesn't exist in Backdrop.

Yes they exist - just moved elsewhere in Backdrop: indigoxela/backdrop#3

@indigoxela
Copy link
Member Author

Yes they exist - just moved elsewhere in Backdrop:

Ah, there they are, thanks for the hint. 👍

I'm a bit unhappy with your unrelated changes. Especially in function view_comment_user_uid().

@indigoxela
Copy link
Member Author

indigoxela commented Oct 20, 2021

Hm, strange, a small part solves a failing test in Drupal, but that test passes in Backdrop. Need to take a closer look - with a fresh mind (not today). (CommentViewsHandlerArgumentUserUidTest)

@klonos
Copy link
Member

klonos commented Oct 20, 2021

I'm a bit unhappy with your unrelated changes

Yes, the entire file was less than 100 lines, and I felt that it could be tidied up comment-wise (comments exceeding 80 characters, missing periods at the end of comments, a comment on the same line as the thing it's describing, using C style comments which is discouraged etc.).

Sure, feel free to revert those. But I have a feeling that these coding standard violations will never get fixed if we don't gradually fix the files we touch. ...I for one would not be bothered to file a separate issue/PR for them 🤷🏼

Hm, strange, a small part solves a failing test in Drupal, but that test passes in Backdrop. Need to take a closer look - with a fresh mind (not today).

Sure. Rest up 👍🏼

@indigoxela
Copy link
Member Author

Riddles solved, issue description updated, back to "needs review".

@klonos I did a little less cleanup, only in the function that actually changes, to not bloat the PR.

@klonos
Copy link
Member

klonos commented Oct 21, 2021

...I did a little less cleanup...

I can live with that 🙂 ...RTBC 👍🏼

@indigoxela
Copy link
Member Author

The PHP version in GHA has been reset to 7.4, ready for merge.

backdrop-ci referenced this issue in backdrop/backdrop Oct 22, 2021
By @indigoxela, @klonos.

Ported from Drupal issue #3207982 by DamienMcKenna.
backdrop-ci referenced this issue in backdrop/backdrop Oct 22, 2021
By @indigoxela, @klonos.

Ported from Drupal issue #3207982 by DamienMcKenna.
@quicksketch
Copy link
Member

Excellent, thank you @indigoxela! I merged backdrop/backdrop#3799 into 1.x and 1.20.x.

@jenlampton jenlampton changed the title PHP 8: Views Handlers: Filter: in_operator - Array to string conversion Fixed: PHP 8: Views Handlers: Filter: in_operator - Array to string conversion Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants