Problem/Motivation

We need project_issue.module to run on PHP7+ before we can port it to D8 (or D9)

Proposed resolution

Make it compatible.

Remaining tasks

  1. Static analysis with PHPCompatibility
  2. Run automated tests.
  3. Limited local testing
  4. Test once d.o dev sites can run different versions of PHP.
  5. Fix any remaining problems.

User interface changes

None.

API changes

Hopefully none.

Data model changes

Hopefully none.

Release notes snippet

TBD.

CommentFileSizeAuthor
#2 3116822-2.patch779 bytesdww
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww created an issue. See original summary.

dww’s picture

dww
we/he/they
CreditAttribution: dww at 3281d Consulting for Drupal Association commented
Issue summary: View changes
Status: Active » Needs review
FileSize
3116822-2.patch779 bytes

Local testing is proving somewhat challenging, since I'm running into bugs with non-d.o installs. Fixing all of that is out of scope for the immediate need. Per @hestenet I'm going to wait until we can have d.o dev site sandboxes with different PHP config so I can do more thorough testing there.

So far, static analysis only finds the following:

FILE: ...pal-7.comp/web/sites/all/modules/contrib/project_issue/project_issue.module
---------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------
 2839 | WARNING | Since PHP 7.0, functions inspecting arguments, like
      |         | func_get_args(), no longer report the original value as passed
      |         | to a parameter, but will instead provide the current value.
      |         | The parameter "$vid" was used, and possibly changed (by
      |         | reference), on line 2835.
---------------------------------------------------------------------------------

Note, in local testing, this code isn't being hit. I've got an 'Issue tags' vocab created and configured (vid 2), with the following variable set:

% drush vget project_issue_taxonomy_vocabulary_issue_queue_2
project_issue_taxonomy_vocabulary_issue_queue_2: autocomplete

I've confirmed project_issue_menu_alter() is firing, but when I use the autocomplete on the issue tags field on my local issues, project_issue_issue_tag_autocomplete() isn't getting called(). So I can't actually test this with PHP 7.4 easily. But I installed the patch on https://project-drupal.dev.devdrupal.org and project_issue_issue_tag_autocomplete() is happening there and still seems to be sorting tags correctly (most used at the top). And phpcs --standard=PHPCompatibility is happy once it's applied.

  • drumm committed 427c175 on 7.x-2.x authored by dww 
    Issue #3116822 by dww: Make project_issue compatible with PHP7+
drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm at Drupal Association commented

I can also confirm this is being hit on a Drupal.org dev site, and works as expected.

Not sure how this might not be called with everything you’ve set up. Is the result of the menu altering making it into the DB?

MariaDB [drumm_drupal]> SELECT path FROM menu_router WHERE page_callback = 'project_issue_issue_tag_autocomplete';
+---------------------------------------------+
| path                                        |
+---------------------------------------------+
| admin/views/ajax/autocomplete/taxonomy/9    |
| taxonomy/autocomplete/taxonomy_vocabulary_9 |
+---------------------------------------------+
dww’s picture

dww
we/he/they
CreditAttribution: dww at 3281d Consulting for Drupal Association commented
Status: Needs review » Postponed

I tracked it down to a separate non-d.o bug in project_issue with regards to the issue_tags special casing:

#3143386: project_issue_menu_alter() assumes D6-style autocomplete paths

I think we can call this postponed until we have a PHP 7.4 d.o dev site to test on.

Thanks,
-Derek